diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts index 5f68622214e..3e1dc26a5f6 100644 --- a/packages/router/src/router.ts +++ b/packages/router/src/router.ts @@ -329,7 +329,7 @@ export class Router { this.navigationTransitions.setupNavigations(this).subscribe( t => { this.lastSuccessfulId = t.id; - this.currentPageId = t.targetPageId; + this.currentPageId = this.browserPageId ?? 0; }, e => { this.console.warn(`Unhandled Navigation Error: ${e}`); @@ -733,13 +733,9 @@ export class Router { if (restoredState && restoredState.ɵrouterPageId) { targetPageId = restoredState.ɵrouterPageId; } else { - // If we're replacing the URL or doing a silent navigation, we do not want to increment the - // page id because we aren't pushing a new entry to history. - if (extras.replaceUrl || extras.skipLocationChange) { - targetPageId = this.browserPageId ?? 0; - } else { - targetPageId = (this.browserPageId ?? 0) + 1; - } + // Otherwise, targetPageId should be the next number in the event of a `pushState` + // navigation. + targetPageId = (this.browserPageId ?? 0) + 1; } } else { // This is unused when `canceledNavigationResolution` is not computed. @@ -779,13 +775,20 @@ export class Router { /** @internal */ setBrowserUrl(url: UrlTree, transition: NavigationTransition) { const path = this.urlSerializer.serialize(url); - const state = { - ...transition.extras.state, - ...this.generateNgRouterState(transition.id, transition.targetPageId) - }; if (this.location.isCurrentPathEqualTo(path) || !!transition.extras.replaceUrl) { + // replacements do not update the target page + const currentBrowserPageId = + this.canceledNavigationResolution === 'computed' ? this.browserPageId : undefined; + const state = { + ...transition.extras.state, + ...this.generateNgRouterState(transition.id, currentBrowserPageId) + }; this.location.replaceState(path, '', state); } else { + const state = { + ...transition.extras.state, + ...this.generateNgRouterState(transition.id, transition.targetPageId) + }; this.location.go(path, '', state); } } @@ -797,16 +800,9 @@ export class Router { */ restoreHistory(transition: NavigationTransition, restoringFromCaughtError = false) { if (this.canceledNavigationResolution === 'computed') { - const targetPagePosition = this.currentPageId - transition.targetPageId; - // The navigator change the location before triggered the browser event, - // so we need to go back to the current url if the navigation is canceled. - // Also, when navigation gets cancelled while using url update strategy eager, then we need to - // go back. Because, when `urlUpdateStrategy` is `eager`; `setBrowserUrl` method is called - // before any verification. - const browserUrlUpdateOccurred = - (transition.source === 'popstate' || this.urlUpdateStrategy === 'eager' || - this.currentUrlTree === this.getCurrentNavigation()?.finalUrl); - if (browserUrlUpdateOccurred && targetPagePosition !== 0) { + const currentBrowserPageId = this.browserPageId ?? this.currentPageId; + const targetPagePosition = this.currentPageId - currentBrowserPageId; + if (targetPagePosition !== 0) { this.location.historyGo(targetPagePosition); } else if ( this.currentUrlTree === this.getCurrentNavigation()?.finalUrl && diff --git a/packages/router/test/computed_state_restoration.spec.ts b/packages/router/test/computed_state_restoration.spec.ts index cbd8fedeceb..3831113242d 100644 --- a/packages/router/test/computed_state_restoration.spec.ts +++ b/packages/router/test/computed_state_restoration.spec.ts @@ -357,6 +357,55 @@ describe('`restoredState#ɵrouterPageId`', () => { const location = TestBed.inject(Location); const router = TestBed.inject(Router); router.urlUpdateStrategy = 'eager'; + let allowNavigation = true; + router.resetConfig([ + {path: 'initial', children: []}, + {path: 'redirectFrom', redirectTo: 'redirectTo'}, + {path: 'redirectTo', children: [], canActivate: [() => allowNavigation]}, + ]); + + // already at '2' from the `beforeEach` navigations + expect(location.getState()).toEqual(jasmine.objectContaining({ɵrouterPageId: 2})); + router.navigateByUrl('/initial'); + advance(fixture); + expect(location.path()).toEqual('/initial'); + expect(location.getState()).toEqual(jasmine.objectContaining({ɵrouterPageId: 3})); + + TestBed.inject(MyCanActivateGuard).redirectTo = null; + + router.navigateByUrl('redirectTo'); + advance(fixture); + expect(location.path()).toEqual('/redirectTo'); + expect(location.getState()).toEqual(jasmine.objectContaining({ɵrouterPageId: 4})); + + // Navigate to different URL but get redirected to same URL should result in same page id + router.navigateByUrl('redirectFrom'); + advance(fixture); + expect(location.path()).toEqual('/redirectTo'); + expect(location.getState()).toEqual(jasmine.objectContaining({ɵrouterPageId: 4})); + + // Back and forward should have page IDs 1 apart + location.back(); + advance(fixture); + expect(location.path()).toEqual('/initial'); + expect(location.getState()).toEqual(jasmine.objectContaining({ɵrouterPageId: 3})); + location.forward(); + advance(fixture); + expect(location.path()).toEqual('/redirectTo'); + expect(location.getState()).toEqual(jasmine.objectContaining({ɵrouterPageId: 4})); + + // Rejected navigation after redirect to same URL should have the same page ID + allowNavigation = false; + router.navigateByUrl('redirectFrom'); + advance(fixture); + expect(location.path()).toEqual('/redirectTo'); + expect(location.getState()).toEqual(jasmine.objectContaining({ɵrouterPageId: 4})); + })); + + it('urlUpdateStrategy="eager", redirectTo with same url, and guard reject', fakeAsync(() => { + const location = TestBed.inject(Location); + const router = TestBed.inject(Router); + router.urlUpdateStrategy = 'eager'; TestBed.inject(MyCanActivateGuard).redirectTo = router.createUrlTree(['unguarded']); router.navigateByUrl('/third');