From c4becca0e4238640461c43567a6d3e16b8c5d3f3 Mon Sep 17 00:00:00 2001 From: Adam Date: Sun, 7 Jun 2020 18:01:11 -0500 Subject: [PATCH] feat(router): add new initialNavigation options to replace legacy (#37480) As of Angular v4, four of the options for `ExtraOptions#initialNavigation` have been deprecated. We intend to remove them in v11. The final state for these options is: `enabledBlocking`, `enabledNonBlocking`, and `disabled`. We plan to remove and deprecate the remaining option in the next two major releases. New options: - `enabledNonBlocking`: same as legacy_enabled - `enabledBlocking`: same as enabled BREAKING CHANGE: * The `initialNavigation` property for the options in `RouterModule.forRoot` no longer supports `legacy_disabled`, `legacy_enabled`, `true`, or `false` as valid values. `legacy_enabled` (the old default) is instead `enabledNonBlocking` * `enabled` is deprecated as a valid value for the `RouterModule.forRoot` `initialNavigation` option. `enabledBlocking` has been introduced to replace it PR Close #37480 --- goldens/public-api/router/router.d.ts | 2 +- .../size-tracking/integration-payloads.json | 2 +- packages/router/src/router_module.ts | 71 ++++------- packages/router/test/bootstrap.spec.ts | 113 +++++++++++++----- 4 files changed, 104 insertions(+), 84 deletions(-) diff --git a/goldens/public-api/router/router.d.ts b/goldens/public-api/router/router.d.ts index be72c3083c..8734cd842a 100644 --- a/goldens/public-api/router/router.d.ts +++ b/goldens/public-api/router/router.d.ts @@ -156,7 +156,7 @@ export declare class GuardsCheckStart extends RouterEvent { toString(): string; } -export declare type InitialNavigation = true | false | 'enabled' | 'disabled' | 'legacy_enabled' | 'legacy_disabled'; +export declare type InitialNavigation = 'disabled' | 'enabled' | 'enabledBlocking' | 'enabledNonBlocking'; export declare type LoadChildren = LoadChildrenCallback | DeprecatedLoadChildren; diff --git a/goldens/size-tracking/integration-payloads.json b/goldens/size-tracking/integration-payloads.json index a71c2411f5..b632516078 100644 --- a/goldens/size-tracking/integration-payloads.json +++ b/goldens/size-tracking/integration-payloads.json @@ -49,7 +49,7 @@ "master": { "uncompressed": { "runtime-es2015": 2289, - "main-es2015": 218329, + "main-es2015": 217827, "polyfills-es2015": 36723, "5-es2015": 781 } diff --git a/packages/router/src/router_module.ts b/packages/router/src/router_module.ts index bf2d216f63..46ad352138 100644 --- a/packages/router/src/router_module.ts +++ b/packages/router/src/router_module.ts @@ -222,7 +222,10 @@ export function provideRoutes(routes: Routes): any { * Allowed values in an `ExtraOptions` object that configure * when the router performs the initial navigation operation. * - * * 'enabled' - The initial navigation starts before the root component is created. + * * 'enabledNonBlocking' - (default) The initial navigation starts after the + * root component has been created. The bootstrap is not blocked on the completion of the initial + * navigation. + * * 'enabledBlocking' - The initial navigation starts before the root component is created. * The bootstrap is blocked until the initial navigation is complete. This value is required * for [server-side rendering](guide/universal) to work. * * 'disabled' - The initial navigation is not performed. The location listener is set up before @@ -230,24 +233,16 @@ export function provideRoutes(routes: Routes): any { * more control over when the router starts its initial navigation due to some complex * initialization logic. * - * The following values have been [deprecated](guide/releases#deprecation-practices) since v4, + * The following values have been [deprecated](guide/releases#deprecation-practices) since v11, * and should not be used for new applications. * - * * 'legacy_enabled'- (Default, for compatibility.) The initial navigation starts after the root - * component has been created. The bootstrap is not blocked until the initial navigation is - * complete. - * * 'legacy_disabled'- The initial navigation is not performed. The location listener is set up - * after the root component gets created. - * * `true` - same as 'legacy_enabled'. - * * `false` - same as 'legacy_disabled'. - * - * The 'legacy_enabled' and 'legacy_disabled' should not be used for new applications. + * * 'enabled' - This option is 1:1 replaceable with `enabledNonBlocking`. * * @see `forRoot()` * * @publicApi */ -export type InitialNavigation = true|false|'enabled'|'disabled'|'legacy_enabled'|'legacy_disabled'; +export type InitialNavigation = 'disabled'|'enabled'|'enabledBlocking'|'enabledNonBlocking'; /** * A set of configuration options for a router module, provided in the @@ -272,24 +267,15 @@ export interface ExtraOptions { useHash?: boolean; /** - * One of `enabled` or `disabled`. - * When set to `enabled`, the initial navigation starts before the root component is created. - * The bootstrap is blocked until the initial navigation is complete. This value is required for - * [server-side rendering](guide/universal) to work. - * When set to `disabled`, the initial navigation is not performed. - * The location listener is set up before the root component gets created. - * Use if there is a reason to have more control over when the router + * One of `enabled`, `enabledBlocking`, `enabledNonBlocking` or `disabled`. + * When set to `enabled` or `enabledBlocking`, the initial navigation starts before the root + * component is created. The bootstrap is blocked until the initial navigation is complete. This + * value is required for [server-side rendering](guide/universal) to work. When set to + * `enabledNonBlocking`, the initial navigation starts after the root component has been created. + * The bootstrap is not blocked on the completion of the initial navigation. When set to + * `disabled`, the initial navigation is not performed. The location listener is set up before the + * root component gets created. Use if there is a reason to have more control over when the router * starts its initial navigation due to some complex initialization logic. - * - * Legacy values are deprecated since v4 and should not be used for new applications: - * - * * `legacy_enabled` - Default for compatibility. - * The initial navigation starts after the root component has been created, - * but the bootstrap is not blocked until the initial navigation is complete. - * * `legacy_disabled` - The initial navigation is not performed. - * The location listener is set up after the root component gets created. - * * `true` - same as `legacy_enabled`. - * * `false` - same as `legacy_disabled`. */ initialNavigation?: InitialNavigation; @@ -519,14 +505,12 @@ export class RouterInitializer { const router = this.injector.get(Router); const opts = this.injector.get(ROUTER_CONFIGURATION); - if (this.isLegacyDisabled(opts) || this.isLegacyEnabled(opts)) { - resolve(true); - - } else if (opts.initialNavigation === 'disabled') { + if (opts.initialNavigation === 'disabled') { router.setUpLocationChangeListener(); resolve(true); - - } else if (opts.initialNavigation === 'enabled') { + } else if ( + // TODO: enabled is deprecated as of v11, can be removed in v13 + opts.initialNavigation === 'enabled' || opts.initialNavigation === 'enabledBlocking') { router.hooks.afterPreactivation = () => { // only the initial navigation should be delayed if (!this.initNavigation) { @@ -540,9 +524,8 @@ export class RouterInitializer { } }; router.initialNavigation(); - } else { - throw new Error(`Invalid initialNavigation options: '${opts.initialNavigation}'`); + resolve(true); } return res; @@ -560,10 +543,9 @@ export class RouterInitializer { return; } - if (this.isLegacyEnabled(opts)) { + // Default case + if (opts.initialNavigation === 'enabledNonBlocking' || opts.initialNavigation === undefined) { router.initialNavigation(); - } else if (this.isLegacyDisabled(opts)) { - router.setUpLocationChangeListener(); } preloader.setUpPreloading(); @@ -572,15 +554,6 @@ export class RouterInitializer { this.resultOfPreactivationDone.next(null!); this.resultOfPreactivationDone.complete(); } - - private isLegacyEnabled(opts: ExtraOptions): boolean { - return opts.initialNavigation === 'legacy_enabled' || opts.initialNavigation === true || - opts.initialNavigation === undefined; - } - - private isLegacyDisabled(opts: ExtraOptions): boolean { - return opts.initialNavigation === 'legacy_disabled' || opts.initialNavigation === false; - } } export function getAppInitializer(r: RouterInitializer) { diff --git a/packages/router/test/bootstrap.spec.ts b/packages/router/test/bootstrap.spec.ts index 6f8a5a87c4..f3ee2c51df 100644 --- a/packages/router/test/bootstrap.spec.ts +++ b/packages/router/test/bootstrap.spec.ts @@ -95,7 +95,44 @@ describe('bootstrap', () => { }); }); - it('should NOT wait for resolvers to complete when initialNavigation = legacy_enabled', + it('should wait for resolvers to complete when initialNavigation = enabledBlocking', (done) => { + @Component({selector: 'test', template: 'test'}) + class TestCmpEnabled { + } + + @NgModule({ + imports: [ + BrowserModule, + RouterModule.forRoot( + [{path: '**', component: TestCmpEnabled, resolve: {test: TestResolver}}], + {useHash: true, initialNavigation: 'enabledBlocking'}) + ], + declarations: [RootCmp, TestCmpEnabled], + bootstrap: [RootCmp], + providers: [...testProviders, TestResolver], + schemas: [CUSTOM_ELEMENTS_SCHEMA] + }) + class TestModule { + constructor(router: Router) { + log.push('TestModule'); + router.events.subscribe(e => log.push(e.constructor.name)); + } + } + + 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'); + expect(log).toEqual([ + 'TestModule', 'NavigationStart', 'RoutesRecognized', 'GuardsCheckStart', + 'ChildActivationStart', 'ActivationStart', 'GuardsCheckEnd', 'ResolveStart', 'ResolveEnd', + 'RootCmp', 'ActivationEnd', 'ChildActivationEnd', 'NavigationEnd', 'Scroll' + ]); + done(); + }); + }); + + it('should NOT wait for resolvers to complete when initialNavigation = enabledNonBlocking', (done) => { @Component({selector: 'test', template: 'test'}) class TestCmpLegacyEnabled { @@ -106,7 +143,7 @@ describe('bootstrap', () => { BrowserModule, RouterModule.forRoot( [{path: '**', component: TestCmpLegacyEnabled, resolve: {test: TestResolver}}], - {useHash: true, initialNavigation: 'legacy_enabled'}) + {useHash: true, initialNavigation: 'enabledNonBlocking'}) ], declarations: [RootCmp, TestCmpLegacyEnabled], bootstrap: [RootCmp], @@ -137,6 +174,47 @@ describe('bootstrap', () => { }); }); + it('should NOT wait for resolvers to complete when initialNavigation is not set', (done) => { + @Component({selector: 'test', template: 'test'}) + class TestCmpLegacyEnabled { + } + + @NgModule({ + imports: [ + BrowserModule, + RouterModule.forRoot( + [{path: '**', component: TestCmpLegacyEnabled, resolve: {test: TestResolver}}], + {useHash: true}) + ], + declarations: [RootCmp, TestCmpLegacyEnabled], + bootstrap: [RootCmp], + providers: [...testProviders, TestResolver], + schemas: [CUSTOM_ELEMENTS_SCHEMA] + }) + class TestModule { + constructor(router: Router) { + log.push('TestModule'); + router.events.subscribe(e => log.push(e.constructor.name)); + } + } + + platformBrowserDynamic([]).bootstrapModule(TestModule).then(res => { + const router: Router = res.injector.get(Router); + expect(router.routerState.snapshot.root.firstChild).toBeNull(); + // ResolveEnd has not been emitted yet because bootstrap returned too early + expect(log).toEqual([ + 'TestModule', 'RootCmp', 'NavigationStart', 'RoutesRecognized', 'GuardsCheckStart', + 'ChildActivationStart', 'ActivationStart', 'GuardsCheckEnd', 'ResolveStart' + ]); + + router.events.subscribe((e) => { + if (e instanceof NavigationEnd) { + done(); + } + }); + }); + }); + it('should not run navigation when initialNavigation = disabled', (done) => { @Component({selector: 'test', template: 'test'}) class TestCmpDiabled { @@ -168,37 +246,6 @@ describe('bootstrap', () => { }); }); - it('should not run navigation when initialNavigation = legacy_disabled', (done) => { - @Component({selector: 'test', template: 'test'}) - class TestCmpLegacyDisabled { - } - - @NgModule({ - imports: [ - BrowserModule, - RouterModule.forRoot( - [{path: '**', component: TestCmpLegacyDisabled, resolve: {test: TestResolver}}], - {useHash: true, initialNavigation: 'legacy_disabled'}) - ], - declarations: [RootCmp, TestCmpLegacyDisabled], - bootstrap: [RootCmp], - providers: [...testProviders, TestResolver], - schemas: [CUSTOM_ELEMENTS_SCHEMA] - }) - class TestModule { - constructor(router: Router) { - log.push('TestModule'); - router.events.subscribe(e => log.push(e.constructor.name)); - } - } - - platformBrowserDynamic([]).bootstrapModule(TestModule).then(res => { - const router = res.injector.get(Router); - expect(log).toEqual(['TestModule', 'RootCmp']); - done(); - }); - }); - it('should not init router navigation listeners if a non root component is bootstrapped', (done) => { @NgModule({