From 912afc4fc7331ebed821db2ebe640aff37c69cba Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Tue, 15 Nov 2022 08:49:18 -0800 Subject: [PATCH] fix(router): Ensure renavigating in component init works with enabledBlocking The correct property to check if the subject has completed is `isStopped` rather than `isClosed`. This issue fixes returning the subject again when it has already emitted (and won't emit again). fixes #48052 --- packages/router/src/provide_router.ts | 2 +- packages/router/test/bootstrap.spec.ts | 51 +++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/packages/router/src/provide_router.ts b/packages/router/src/provide_router.ts index 2c96a85d13fcd6..48e463799e6883 100644 --- a/packages/router/src/provide_router.ts +++ b/packages/router/src/provide_router.ts @@ -366,7 +366,7 @@ export function withEnabledBlockingInitialNavigation(): EnabledBlockingInitialNa resolve(true); // only the initial navigation should be delayed until bootstrapping is done. if (!initNavigation) { - return bootstrapDone.closed ? of(void 0) : bootstrapDone; + return bootstrapDone.isStopped ? of(void 0) : bootstrapDone; // subsequent navigations should not be delayed } else { return of(void 0); diff --git a/packages/router/test/bootstrap.spec.ts b/packages/router/test/bootstrap.spec.ts index 111ae7099358d6..6127ef76d40667 100644 --- a/packages/router/test/bootstrap.spec.ts +++ b/packages/router/test/bootstrap.spec.ts @@ -11,7 +11,7 @@ import {ApplicationRef, Component, CUSTOM_ELEMENTS_SCHEMA, destroyPlatform, Inje import {inject} from '@angular/core/testing'; import {BrowserModule} from '@angular/platform-browser'; import {platformBrowserDynamic} from '@angular/platform-browser-dynamic'; -import {NavigationEnd, provideRouter, Resolve, Router, RouterModule, withEnabledBlockingInitialNavigation} from '@angular/router'; +import {NavigationEnd, provideRouter, Resolve, Router, RouterModule, RouterOutlet, withEnabledBlockingInitialNavigation} from '@angular/router'; // This is needed, because all files under `packages/` are compiled together as part of the // [legacy-unit-tests-saucelabs][1] CI job, including the `lib.webworker.d.ts` typings brought in by @@ -112,6 +112,55 @@ describe('bootstrap', () => { }); }); + it('should finish navigation when initial navigation is enabledBlocking and component renavigates on render', + async () => { + @Component({ + template: '', + standalone: true, + }) + class Renavigate { + constructor(router: Router) { + router.navigateByUrl('/other'); + } + } + @Component({ + template: '', + standalone: true, + }) + class BlankCmp { + } + + let resolveFn: () => void; + const navigationEndPromise = new Promise(r => { + resolveFn = r; + }); + + @NgModule({ + imports: [BrowserModule, RouterOutlet], + declarations: [RootCmp], + bootstrap: [RootCmp], + providers: [ + {provide: LocationStrategy, useClass: HashLocationStrategy}, + provideRouter( + [{path: '', component: Renavigate}, {path: 'other', component: BlankCmp}], + withEnabledBlockingInitialNavigation()) + ], + }) + class TestModule { + constructor(router: Router) { + router.events.subscribe(e => { + if (e instanceof NavigationEnd) { + resolveFn(); + expect(router.url).toEqual('/other'); + } + }); + } + } + + await Promise.all( + [platformBrowserDynamic([]).bootstrapModule(TestModule), navigationEndPromise]); + }); + it('should wait for redirect when initialNavigation = enabledBlocking', async () => { @Injectable({providedIn: 'root'}) class Redirect {