From 2f581ffc88466138675b3fa13b8c44d328439e68 Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Thu, 10 Mar 2016 20:42:32 -0800 Subject: [PATCH] fix(router): RouterOutlet loads component twice in a race condition Closes #7497 Closes #7545 --- .../core/linker/dynamic_component_loader.ts | 4 +- .../src/router/directives/router_outlet.ts | 58 +++++++++++-------- tools/public_api_guard/public_api_spec.ts | 2 +- 3 files changed, 37 insertions(+), 27 deletions(-) diff --git a/modules/angular2/src/core/linker/dynamic_component_loader.ts b/modules/angular2/src/core/linker/dynamic_component_loader.ts index a005f6c894..3a03fdf155 100644 --- 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 index 7c066991ab..bf156ec0ef 100644 --- 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 = 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 (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) => + (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) => + (ref.instance).routerOnReuse(nextInstruction, previousInstruction)) : + true); } - return PromiseWrapper.resolve( - hasLifecycleHook(hookMod.routerOnReuse, this._currentInstruction.componentType) ? - (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 = >PromiseWrapper.resolve( - (this._componentRef.instance) - .routerOnDeactivate(nextInstruction, this._currentInstruction)); + next = this._componentRef.then( + (ref: ComponentRef) => + (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 >PromiseWrapper.resolve( - (this._componentRef.instance) - .routerCanDeactivate(nextInstruction, this._currentInstruction)); + return this._componentRef.then( + (ref: ComponentRef) => + (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 = (this._componentRef.instance) - .routerCanReuse(nextInstruction, this._currentInstruction); + result = this._componentRef.then( + (ref: ComponentRef) => + (ref.instance).routerCanReuse(nextInstruction, this._currentInstruction)); } else { result = nextInstruction == this._currentInstruction || (isPresent(nextInstruction.params) && isPresent(this._currentInstruction.params) && diff --git a/tools/public_api_guard/public_api_spec.ts b/tools/public_api_guard/public_api_spec.ts index 1de862efad..d5eb25c315 100644 --- a/tools/public_api_guard/public_api_spec.ts +++ b/tools/public_api_guard/public_api_spec.ts @@ -106,7 +106,7 @@ const CORE = [ 'ComponentMetadata.viewProviders:any[]', 'ComponentRef', 'ComponentRef.componentType:Type', - 'ComponentRef.dispose():any', + 'ComponentRef.dispose():void', 'ComponentRef.hostComponent:any', 'ComponentRef.hostView:HostViewRef', 'ComponentRef.injector:Injector',