From 9299073e72f911103f1e9baaf1e7293fff5ad1a1 Mon Sep 17 00:00:00 2001 From: vsavkin Date: Sat, 28 Jan 2017 12:41:12 -0500 Subject: [PATCH] fix(router): do not finish bootstrap until all the routes are resolved Fixes #12162 --- modules/@angular/router/src/router.ts | 57 ++++++-- modules/@angular/router/src/router_module.ts | 79 +++++++++-- .../@angular/router/test/bootstrap.spec.ts | 128 ++++++++++++++++++ .../router/test/router_module.spec.ts | 91 ------------- modules/@angular/router/upgrade.ts | 46 +++---- 5 files changed, 258 insertions(+), 143 deletions(-) create mode 100644 modules/@angular/router/test/bootstrap.spec.ts delete mode 100644 modules/@angular/router/test/router_module.spec.ts diff --git a/modules/@angular/router/src/router.ts b/modules/@angular/router/src/router.ts index f9088ded9a47d4..0782e16e9da1a6 100644 --- a/modules/@angular/router/src/router.ts +++ b/modules/@angular/router/src/router.ts @@ -278,6 +278,18 @@ type NavigationParams = { source: NavigationSource, }; +/** + * @internal + */ +export type RouterHook = (snapshot: RouterStateSnapshot) => Observable; + +/** + * @internal + */ +function defaultRouterHook(snapshot: RouterStateSnapshot): Observable { + return of (snapshot); +} + /** * Does not detach any subtrees. Reuses routes as long as their route config is the same. @@ -320,11 +332,23 @@ export class Router { */ errorHandler: ErrorHandler = defaultErrorHandler; + + /** * Indicates if at least one navigation happened. */ navigated: boolean = false; + /** + * Used by RouterModule and experimental libraries. This allows us to + * pause the navigation either before preactivation or after it. + * @internal + */ + hooks: {beforePreactivation: RouterHook, afterPreactivation: RouterHook} = { + beforePreactivation: defaultRouterHook, + afterPreactivation: defaultRouterHook + }; + /** * Extracts and merges URLs. Used for AngularJS to Angular migrations. */ @@ -679,17 +703,23 @@ export class Router { urlAndSnapshot$ = of ({appliedUrl: url, snapshot: precreatedState}); } + const beforePreactivationDone$ = mergeMap.call(urlAndSnapshot$, (p: any) => { + return map.call( + this.hooks.beforePreactivation(p.snapshot), + (newSnapshot: any) => ({appliedUrl: p.appliedUrl, snapshot: newSnapshot})); + }); // run preactivation: guards and data resolvers let preActivation: PreActivation; - const preactivationTraverse$ = map.call(urlAndSnapshot$, ({appliedUrl, snapshot}: any) => { - preActivation = - new PreActivation(snapshot, this.currentRouterState.snapshot, this.injector); - preActivation.traverse(this.outletMap); - return {appliedUrl, snapshot}; - }); + const preactivationTraverse$ = + map.call(beforePreactivationDone$, ({appliedUrl, snapshot}: any) => { + preActivation = + new PreActivation(snapshot, this.currentRouterState.snapshot, this.injector); + preActivation.traverse(this.outletMap); + return {appliedUrl, snapshot}; + }); - const preactivationCheckGuards = + const preactivationCheckGuards$ = mergeMap.call(preactivationTraverse$, ({appliedUrl, snapshot}: any) => { if (this.navigationId !== id) return of (false); @@ -698,7 +728,7 @@ export class Router { }); }); - const preactivationResolveData$ = mergeMap.call(preactivationCheckGuards, (p: any) => { + const preactivationResolveData$ = mergeMap.call(preactivationCheckGuards$, (p: any) => { if (this.navigationId !== id) return of (false); if (p.shouldActivate) { @@ -708,11 +738,20 @@ export class Router { } }); + const preactivationDone$ = mergeMap.call(preactivationResolveData$, (p: any) => { + return map.call( + this.hooks.afterPreactivation(p.snapshot), (newSnapshot: any) => ({ + appliedUrl: p.appliedUrl, + snapshot: newSnapshot, + shouldActivate: p.shouldActivate + })); + }); + // create router state // this operation has side effects => route state is being affected const routerState$ = - map.call(preactivationResolveData$, ({appliedUrl, snapshot, shouldActivate}: any) => { + map.call(preactivationDone$, ({appliedUrl, snapshot, shouldActivate}: any) => { if (shouldActivate) { const state = createRouterState(this.routeReuseStrategy, snapshot, this.currentRouterState); diff --git a/modules/@angular/router/src/router_module.ts b/modules/@angular/router/src/router_module.ts index 2f3a7624733efa..aad3c39b0107f3 100644 --- a/modules/@angular/router/src/router_module.ts +++ b/modules/@angular/router/src/router_module.ts @@ -7,7 +7,8 @@ */ import {APP_BASE_HREF, HashLocationStrategy, Location, LocationStrategy, PathLocationStrategy, PlatformLocation} from '@angular/common'; -import {ANALYZE_FOR_ENTRY_COMPONENTS, APP_BOOTSTRAP_LISTENER, ApplicationRef, Compiler, ComponentRef, Inject, InjectionToken, Injector, ModuleWithProviders, NgModule, NgModuleFactoryLoader, NgProbeToken, Optional, Provider, SkipSelf, SystemJsNgModuleLoader} from '@angular/core'; +import {ANALYZE_FOR_ENTRY_COMPONENTS, APP_BOOTSTRAP_LISTENER, APP_INITIALIZER, ApplicationRef, Compiler, ComponentRef, Inject, Injectable, InjectionToken, Injector, ModuleWithProviders, NgModule, NgModuleFactoryLoader, NgProbeToken, Optional, Provider, SkipSelf, SystemJsNgModuleLoader} from '@angular/core'; +import {Subject} from 'rxjs'; import {Route, Routes} from './config'; import {RouterLink, RouterLinkWithHref} from './directives/router_link'; @@ -21,6 +22,7 @@ import {RouterOutletMap} from './router_outlet_map'; import {NoPreloading, PreloadAllModules, PreloadingStrategy, RouterPreloader} from './router_preloader'; import {ActivatedRoute} from './router_state'; import {UrlHandlingStrategy} from './url_handling_strategy'; +import {RouterStateSnapshot} from './router_state'; import {DefaultUrlSerializer, UrlSerializer} from './url_tree'; import {flatten} from './utils/collection'; @@ -278,22 +280,70 @@ export function rootRoute(router: Router): ActivatedRoute { return router.routerState.root; } -export function initialRouterNavigation( - router: Router, ref: ApplicationRef, preloader: RouterPreloader, opts: ExtraOptions) { - return (bootstrappedComponentRef: ComponentRef) => { +/** + * To initialize the router properly we need to do in two steps: + * + * We need to start the navigation in a APP_INITIALIZER to block the bootstrap if + * a resolver or a guards executes asynchronously. Second, we need to actually run + * activation in a BOOTSTRAP_LISTENER. We utilize an experimental afterPreactivation + * hook provided by the router to do that. + * + * The router navigation starts, reaches the point when preactivation is done, and then + * pauses. It waits for the hook to be resolved. We then resolve it only in a bootstrap listener. + */ +@Injectable() +export class RouterInitializer { + private initSnapshot: RouterStateSnapshot; + private resultOfPreactivationDone = new Subject(); - if (bootstrappedComponentRef !== ref.components[0]) { - return; - } + constructor(private injector: Injector) {} + + appInitializer(): Promise { + let resolve: any = null; + const res = new Promise(r => resolve = r); + + const router = this.injector.get(Router); + + router.hooks.afterPreactivation = (s) => { + this.initSnapshot = s; + resolve(true); + return this.resultOfPreactivationDone; + }; + + const opts = this.injector.get(ROUTER_CONFIGURATION); - router.resetRootComponentType(ref.componentTypes[0]); - preloader.setUpPreloading(); if (opts.initialNavigation === false) { router.setUpLocationChangeListener(); } else { router.initialNavigation(); } - }; + + return res; + } + + bootstrapListener(bootstrappedComponentRef: ComponentRef): void { + const ref = this.injector.get(ApplicationRef); + if (bootstrappedComponentRef !== ref.components[0]) { + return; + } + + const preloader = this.injector.get(RouterPreloader); + preloader.setUpPreloading(); + + const router = this.injector.get(Router); + router.resetRootComponentType(ref.componentTypes[0]); + + this.resultOfPreactivationDone.next(this.initSnapshot); + this.resultOfPreactivationDone.complete(); + } +} + +export function getAppInitializer(r: RouterInitializer) { + return r.appInitializer.bind(r); +} + +export function getBootstrapListener(r: RouterInitializer) { + return r.bootstrapListener.bind(r); } /** @@ -306,11 +356,14 @@ export const ROUTER_INITIALIZER = export function provideRouterInitializer() { return [ + RouterInitializer, { - provide: ROUTER_INITIALIZER, - useFactory: initialRouterNavigation, - deps: [Router, ApplicationRef, RouterPreloader, ROUTER_CONFIGURATION] + provide: APP_INITIALIZER, + multi: true, + useFactory: getAppInitializer, + deps: [RouterInitializer] }, + {provide: ROUTER_INITIALIZER, useFactory: getBootstrapListener, deps: [RouterInitializer]}, {provide: APP_BOOTSTRAP_LISTENER, multi: true, useExisting: ROUTER_INITIALIZER}, ]; } diff --git a/modules/@angular/router/test/bootstrap.spec.ts b/modules/@angular/router/test/bootstrap.spec.ts new file mode 100644 index 00000000000000..65ab118b715746 --- /dev/null +++ b/modules/@angular/router/test/bootstrap.spec.ts @@ -0,0 +1,128 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {APP_BASE_HREF} from '@angular/common'; +import {ApplicationRef, CUSTOM_ELEMENTS_SCHEMA, Component, NgModule, destroyPlatform} from '@angular/core'; +import {BrowserModule, DOCUMENT} from '@angular/platform-browser'; +import {platformBrowserDynamic} from '@angular/platform-browser-dynamic'; +import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter'; +import {Resolve, Router, RouterModule} from '@angular/router'; + +describe('bootstrap should wait for resolvers to complete', () => { + + @Component({selector: 'test-app', template: 'root '}) + class RootCmp { + } + + @Component({selector: 'test-app2', template: 'root '}) + class SecondRootCmp { + } + + @Component({selector: 'test', template: 'test'}) + class TestCmp { + } + + class TestResolver implements Resolve { + resolve() { + let resolve: any = null; + const res = new Promise(r => resolve = r); + setTimeout(() => resolve('test-data'), 0); + return res; + } + } + + let testProviders: any[] = null; + + beforeEach(() => { + destroyPlatform(); + const fakeDoc = getDOM().createHtmlDocument(); + const el1 = getDOM().createElement('test-app', fakeDoc); + const el2 = getDOM().createElement('test-app2', fakeDoc); + getDOM().appendChild(fakeDoc.body, el1); + getDOM().appendChild(fakeDoc.body, el2); + testProviders = + [{provide: DOCUMENT, useValue: fakeDoc}, {provide: APP_BASE_HREF, useValue: ''}]; + }); + + it('should work', (done) => { + @NgModule({ + imports: [ + BrowserModule, + RouterModule.forRoot( + [{path: '**', component: TestCmp, resolve: {test: TestResolver}}], {useHash: true}) + ], + declarations: [SecondRootCmp, RootCmp, TestCmp], + bootstrap: [RootCmp], + providers: [...testProviders, TestResolver], + schemas: [CUSTOM_ELEMENTS_SCHEMA] + }) + class TestModule { + } + + platformBrowserDynamic([]).bootstrapModule(TestModule).then(res => { + const router = res.injector.get(Router); + const data = router.routerState.snapshot.root.firstChild.data; + expect(data['test']).toEqual('test-data'); + done(); + }); + }); + + it('should not init router navigation listeners if a non root component is bootstrapped', + (done) => { + @NgModule({ + imports: [BrowserModule, RouterModule.forRoot([], {useHash: true})], + declarations: [SecondRootCmp, RootCmp], + entryComponents: [SecondRootCmp], + bootstrap: [RootCmp], + providers: testProviders, + schemas: [CUSTOM_ELEMENTS_SCHEMA] + }) + class TestModule { + } + + platformBrowserDynamic([]).bootstrapModule(TestModule).then(res => { + const router = res.injector.get(Router); + spyOn(router, 'resetRootComponentType').and.callThrough(); + + const appRef: ApplicationRef = res.injector.get(ApplicationRef); + appRef.bootstrap(SecondRootCmp); + + expect(router.resetRootComponentType).not.toHaveBeenCalled(); + + done(); + }); + }); + + it('should reinit router navigation listeners if a previously bootstrapped root component is destroyed', + (done) => { + @NgModule({ + imports: [BrowserModule, RouterModule.forRoot([], {useHash: true})], + declarations: [SecondRootCmp, RootCmp], + entryComponents: [SecondRootCmp], + bootstrap: [RootCmp], + providers: testProviders, + schemas: [CUSTOM_ELEMENTS_SCHEMA] + }) + class TestModule { + } + + platformBrowserDynamic([]).bootstrapModule(TestModule).then(res => { + const router = res.injector.get(Router); + spyOn(router, 'resetRootComponentType').and.callThrough(); + + const appRef: ApplicationRef = res.injector.get(ApplicationRef); + appRef.components[0].onDestroy(() => { + appRef.bootstrap(SecondRootCmp); + expect(router.resetRootComponentType).toHaveBeenCalled(); + done(); + }); + + appRef.components[0].destroy(); + }); + }); +}); diff --git a/modules/@angular/router/test/router_module.spec.ts b/modules/@angular/router/test/router_module.spec.ts deleted file mode 100644 index 379a3a35d7fd3d..00000000000000 --- a/modules/@angular/router/test/router_module.spec.ts +++ /dev/null @@ -1,91 +0,0 @@ -/** - * @license - * Copyright Google Inc. All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ -import {APP_BASE_HREF} from '@angular/common'; -import {ApplicationRef, Component, NgModule} from '@angular/core'; -import {TestBed, inject} from '@angular/core/testing'; -import {DOCUMENT} from '@angular/platform-browser'; -import {Router, RouterModule, Routes} from '@angular/router'; - - -@Component({selector: 'app-root', template: ''}) -export class AppRootComponent { -} - -@Component({selector: 'bootstrappable-component', template: ''}) -export class BootstrappableComponent { -} - -export const appRoutes: Routes = [{path: '**', redirectTo: ''}]; - - -@NgModule({ - imports: [RouterModule.forRoot(appRoutes)], - declarations: [AppRootComponent, BootstrappableComponent], - entryComponents: [AppRootComponent, BootstrappableComponent], - providers: [{provide: APP_BASE_HREF, useValue: '/'}] -}) -export class RouterInitTestModule { -} - - -describe('RouterModule', () => { - describe('RouterInitializer', () => { - - beforeEach(() => { TestBed.configureTestingModule({imports: [RouterInitTestModule]}); }); - - beforeEach(inject([DOCUMENT], function(doc: HTMLDocument) { - - const elRootApp = doc.createElement('app-root'); - doc.body.appendChild(elRootApp); - - const elBootComp = doc.createElement('bootstrappable-component'); - doc.body.appendChild(elBootComp); - - })); - it('should not init router navigation listeners if a non root component is bootstrapped', - () => { - - const appRef: ApplicationRef = TestBed.get(ApplicationRef); - const r: Router = TestBed.get(Router); - - const spy = spyOn(r, 'resetRootComponentType').and.callThrough(); - - appRef.bootstrap(AppRootComponent); - expect(r.resetRootComponentType).toHaveBeenCalled(); - - spy.calls.reset(); - - appRef.bootstrap(BootstrappableComponent); - expect(r.resetRootComponentType).not.toHaveBeenCalled(); - }); - it('should reinit router navigation listeners if a previously bootstrapped root component is destroyed', - (done) => { - - const appRef: ApplicationRef = TestBed.get(ApplicationRef); - const r: Router = TestBed.get(Router); - - const spy = spyOn(r, 'resetRootComponentType').and.callThrough(); - - const compRef = appRef.bootstrap(AppRootComponent); - expect(r.resetRootComponentType).toHaveBeenCalled(); - - spy.calls.reset(); - - compRef.onDestroy(() => { - - appRef.bootstrap(BootstrappableComponent); - expect(r.resetRootComponentType).toHaveBeenCalled(); - - done(); - }); - - compRef.destroy(); - }); - }); - -}); diff --git a/modules/@angular/router/upgrade.ts b/modules/@angular/router/upgrade.ts index 17f5bdd1e94a3e..723d8fa28cb5a7 100644 --- a/modules/@angular/router/upgrade.ts +++ b/modules/@angular/router/upgrade.ts @@ -6,8 +6,8 @@ * found in the LICENSE file at https://angular.io/license */ -import {APP_BOOTSTRAP_LISTENER, ApplicationRef, ComponentRef, InjectionToken} from '@angular/core'; -import {ExtraOptions, ROUTER_CONFIGURATION, ROUTER_INITIALIZER, Router, RouterPreloader} from '@angular/router'; +import {APP_BOOTSTRAP_LISTENER, ComponentRef, InjectionToken} from '@angular/core'; +import {Router} from '@angular/router'; import {UpgradeModule} from '@angular/upgrade/static'; @@ -36,38 +36,17 @@ import {UpgradeModule} from '@angular/upgrade/static'; * @experimental */ export const RouterUpgradeInitializer = { - provide: ROUTER_INITIALIZER, - useFactory: initialRouterNavigation, - deps: [UpgradeModule, ApplicationRef, RouterPreloader, ROUTER_CONFIGURATION] + provide: APP_BOOTSTRAP_LISTENER, + multi: true, + useFactory: locationSyncBootstrapListener, + deps: [UpgradeModule] }; /** * @internal */ -export function initialRouterNavigation( - ngUpgrade: UpgradeModule, ref: ApplicationRef, preloader: RouterPreloader, - opts: ExtraOptions): Function { - return () => { - if (!ngUpgrade.$injector) { - throw new Error(` - RouterUpgradeInitializer can be used only after UpgradeModule.bootstrap has been called. - Remove RouterUpgradeInitializer and call setUpLocationSync after UpgradeModule.bootstrap. - `); - } - - const router = ngUpgrade.injector.get(Router); - const ref = ngUpgrade.injector.get(ApplicationRef); - - (router as any).resetRootComponentType(ref.componentTypes[0]); - preloader.setUpPreloading(); - if (opts.initialNavigation === false) { - router.setUpLocationChangeListener(); - } else { - router.initialNavigation(); - } - - setUpLocationSync(ngUpgrade); - }; +export function locationSyncBootstrapListener(ngUpgrade: UpgradeModule) { + return () => { setUpLocationSync(ngUpgrade); }; } /** @@ -78,7 +57,14 @@ export function initialRouterNavigation( * * @experimental */ -export function setUpLocationSync(ngUpgrade: UpgradeModule): void { +export function setUpLocationSync(ngUpgrade: UpgradeModule) { + if (!ngUpgrade.$injector) { + throw new Error(` + RouterUpgradeInitializer can be used only after UpgradeModule.bootstrap has been called. + Remove RouterUpgradeInitializer and call setUpLocationSync after UpgradeModule.bootstrap. + `); + } + const router: Router = ngUpgrade.injector.get(Router); const url = document.createElement('a');