fix(router): RouterOutlet loads component twice in a race condition

Closes angular#7497
mhevery · Mar 11, 2016 · 33fafd9 · 33fafd9
1 parent 8fbf418
commit 33fafd9
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 26 deletions.
diff --git a/modules/angular2/src/core/linker/dynamic_component_loader.ts b/modules/angular2/src/core/linker/dynamic_component_loader.ts
@@ -59,7 +59,7 @@ export abstract class ComponentRef {
    *
    * TODO(i): rename to destroy to be consistent with AppViewManager and ViewContainerRef
    */
-  abstract dispose();
+  abstract dispose(): void;
 }
 
 export class ComponentRef_ extends ComponentRef {
@@ -84,7 +84,7 @@ export class ComponentRef_ extends ComponentRef {
    */
   get hostComponentType(): Type { return this.componentType; }
 
-  dispose() { this._dispose(); }
+  dispose(): void { this._dispose(); }
 }
 
 /**

diff --git a/modules/angular2/src/router/directives/router_outlet.ts b/modules/angular2/src/router/directives/router_outlet.ts
@@ -34,7 +34,7 @@ let _resolveToTrue = PromiseWrapper.resolve(true);
 @Directive({selector: 'router-outlet'})
 export class RouterOutlet implements OnDestroy {
   name: string = null;
-  private _componentRef: ComponentRef = null;
+  private _componentRef: Promise<ComponentRef> = null;
   private _currentInstruction: ComponentInstruction = null;
 
   constructor(private _elementRef: ElementRef, private _loader: DynamicComponentLoader,
@@ -62,14 +62,17 @@ export class RouterOutlet implements OnDestroy {
       provide(RouteParams, {useValue: new RouteParams(nextInstruction.params)}),
       provide(routerMod.Router, {useValue: childRouter})
     ]);
-    return this._loader.loadNextToLocation(componentType, this._elementRef, providers)
-        .then((componentRef) => {
-          this._componentRef = componentRef;
-          if (hasLifecycleHook(hookMod.routerOnActivate, componentType)) {
-            return (<OnActivate>this._componentRef.instance)
-                .routerOnActivate(nextInstruction, previousInstruction);
-          }
-        });
+    this._componentRef =
+        this._loader.loadNextToLocation(componentType, this._elementRef, providers);
+    return this._componentRef.then((componentRef) => {
+      if (hasLifecycleHook(hookMod.routerOnActivate, componentType)) {
+        return this._componentRef.then(
+            (ref: ComponentRef) =>
+                (<OnActivate>ref.instance).routerOnActivate(nextInstruction, previousInstruction));
+      } else {
+        return componentRef;
+      }
+    });
   }
 
   /**
@@ -86,12 +89,14 @@ export class RouterOutlet implements OnDestroy {
     // a new one.
     if (isBlank(this._componentRef)) {
       return this.activate(nextInstruction);
+    } else {
+      return PromiseWrapper.resolve(
+          hasLifecycleHook(hookMod.routerOnReuse, this._currentInstruction.componentType) ?
+              this._componentRef.then(
+                  (ref: ComponentRef) =>
+                      (<OnReuse>ref.instance).routerOnReuse(nextInstruction, previousInstruction)) :
+              true);
     }
-    return PromiseWrapper.resolve(
-        hasLifecycleHook(hookMod.routerOnReuse, this._currentInstruction.componentType) ?
-            (<OnReuse>this._componentRef.instance)
-                .routerOnReuse(nextInstruction, previousInstruction) :
-            true);
   }
 
   /**
@@ -102,14 +107,16 @@ export class RouterOutlet implements OnDestroy {
     var next = _resolveToTrue;
     if (isPresent(this._componentRef) && isPresent(this._currentInstruction) &&
         hasLifecycleHook(hookMod.routerOnDeactivate, this._currentInstruction.componentType)) {
-      next = <Promise<boolean>>PromiseWrapper.resolve(
-          (<OnDeactivate>this._componentRef.instance)
-              .routerOnDeactivate(nextInstruction, this._currentInstruction));
+      next = this._componentRef.then(
+          (ref: ComponentRef) =>
+              (<OnDeactivate>ref.instance)
+                  .routerOnDeactivate(nextInstruction, this._currentInstruction));
     }
     return next.then((_) => {
       if (isPresent(this._componentRef)) {
-        this._componentRef.dispose();
+        var onDispose = this._componentRef.then((ref: ComponentRef) => ref.dispose());
         this._componentRef = null;
+        return onDispose;
       }
     });
   }
@@ -127,11 +134,13 @@ export class RouterOutlet implements OnDestroy {
       return _resolveToTrue;
     }
     if (hasLifecycleHook(hookMod.routerCanDeactivate, this._currentInstruction.componentType)) {
-      return <Promise<boolean>>PromiseWrapper.resolve(
-          (<CanDeactivate>this._componentRef.instance)
-              .routerCanDeactivate(nextInstruction, this._currentInstruction));
+      return this._componentRef.then(
+          (ref: ComponentRef) =>
+              (<CanDeactivate>ref.instance)
+                  .routerCanDeactivate(nextInstruction, this._currentInstruction));
+    } else {
+      return _resolveToTrue;
     }
-    return _resolveToTrue;
   }
 
   /**
@@ -151,8 +160,9 @@ export class RouterOutlet implements OnDestroy {
         this._currentInstruction.componentType != nextInstruction.componentType) {
       result = false;
     } else if (hasLifecycleHook(hookMod.routerCanReuse, this._currentInstruction.componentType)) {
-      result = (<CanReuse>this._componentRef.instance)
-                   .routerCanReuse(nextInstruction, this._currentInstruction);
+      result = this._componentRef.then(
+          (ref: ComponentRef) =>
+              (<CanReuse>ref.instance).routerCanReuse(nextInstruction, this._currentInstruction));
     } else {
       result = nextInstruction == this._currentInstruction ||
                (isPresent(nextInstruction.params) && isPresent(this._currentInstruction.params) &&