From aef353c143ea4e31d76f00ae91efe49eecc3a321 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 (#48066) The way to complete the `Subject` in a way that is able to be read on the subject properties itself is to call `unsubscribe`: https://github.com/ReactiveX/rxjs/blob/afac3d574323333572987e043adcd0f8d4cff546/src/internal/Subject.ts#L101-L104 This sets the `closed` property to `true` whereas `complete` does not. fixes #48052 PR Close #48066 --- packages/router/src/provide_router.ts | 6 ++- packages/router/test/bootstrap.spec.ts | 51 +++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/packages/router/src/provide_router.ts b/packages/router/src/provide_router.ts index 0bd7e14e238d4..2ffdbde9bc4e0 100644 --- a/packages/router/src/provide_router.ts +++ b/packages/router/src/provide_router.ts @@ -184,8 +184,10 @@ export function getBootstrapListener() { injector.get(ROUTER_PRELOADER, null, InjectFlags.Optional)?.setUpPreloading(); injector.get(ROUTER_SCROLLER, null, InjectFlags.Optional)?.init(); router.resetRootComponentType(ref.componentTypes[0]); - bootstrapDone.next(); - bootstrapDone.complete(); + if (!bootstrapDone.closed) { + bootstrapDone.next(); + bootstrapDone.unsubscribe(); + } }; } diff --git a/packages/router/test/bootstrap.spec.ts b/packages/router/test/bootstrap.spec.ts index 59a0977611c39..f677aa64fcf6c 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 {