diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index ce42fcc3451..dec8ec5a667 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -1526,6 +1526,9 @@ { "name": "isArrayLike" }, + { + "name": "isBrowserTriggeredNavigation" + }, { "name": "isCommandWithOutlets" }, diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts index db34072a358..f1641ac8588 100644 --- a/packages/router/src/router.ts +++ b/packages/router/src/router.ts @@ -636,6 +636,12 @@ export class Router { (this.onSameUrlNavigation === 'reload' ? true : urlTransition) && this.urlHandlingStrategy.shouldProcessUrl(t.rawUrl); + // If the source of the navigation is from a browser event, the URL is + // already updated. We already need to sync the internal state. + if (isBrowserTriggeredNavigation(t.source)) { + this.browserUrlTree = t.rawUrl; + } + if (processCurrentUrl) { return of(t).pipe( // Fire NavigationStart event @@ -954,7 +960,12 @@ export class Router { this.urlHandlingStrategy.merge(e.url, this.rawUrlTree); const extras = { skipLocationChange: t.extras.skipLocationChange, - replaceUrl: this.urlUpdateStrategy === 'eager' + // The URL is already updated at this point if we have 'eager' URL + // updates or if the navigation was triggered by the browser (back + // button, URL bar, etc). We want to replace that item in history if + // the navigation is rejected. + replaceUrl: this.urlUpdateStrategy === 'eager' || + isBrowserTriggeredNavigation(t.source) }; this.scheduleNavigation( @@ -1385,8 +1396,8 @@ export class Router { const lastNavigation = this.getTransition(); // We don't want to skip duplicate successful navs if they're imperative because // onSameUrlNavigation could be 'reload' (so the duplicate is intended). - const browserNavPrecededByRouterNav = - source !== 'imperative' && lastNavigation?.source === 'imperative'; + const browserNavPrecededByRouterNav = isBrowserTriggeredNavigation(source) && lastNavigation && + !isBrowserTriggeredNavigation(lastNavigation.source); const lastNavigationSucceeded = this.lastSuccessfulId === lastNavigation.id; // If the last navigation succeeded or is in flight, we can use the rawUrl as the comparison. // However, if it failed, we should compare to the final result (urlAfterRedirects). @@ -1549,3 +1560,7 @@ function validateCommands(commands: string[]): void { } } } + +function isBrowserTriggeredNavigation(source: 'imperative'|'popstate'|'hashchange') { + return source !== 'imperative'; +} diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts index b3bc41553b9..3e9210b418a 100644 --- a/packages/router/test/integration.spec.ts +++ b/packages/router/test/integration.spec.ts @@ -1230,7 +1230,11 @@ describe('Integration', () => { location.simulateHashChange('/blocked'); advance(fixture); - expectEvents(recordedEvents, [[NavigationStart, '/blocked']]); + expectEvents(recordedEvents, [ + [NavigationStart, '/blocked'], + [NavigationStart, '/simple'], + [NavigationEnd, '/simple'], + ]); })); }); diff --git a/packages/router/test/regression_integration.spec.ts b/packages/router/test/regression_integration.spec.ts index 371431b9e95..9df54ddeae1 100644 --- a/packages/router/test/regression_integration.spec.ts +++ b/packages/router/test/regression_integration.spec.ts @@ -266,14 +266,9 @@ describe('Integration', () => { location.simulateHashChange('/one'); advance(fixture); - const BASE_ERROR_MESSAGE = - 'This asserts current behavior, which is incorrect. When #28561 is fixed, it should be: '; - - expect(location.path()).toEqual('/one', BASE_ERROR_MESSAGE + '/'); - const urlChanges = ['replace: /', 'hash: /one']; - expect(location.urlChanges) - .toEqual( - urlChanges, BASE_ERROR_MESSAGE + JSON.stringify(urlChanges.concat('replace: /'))); + expect(location.path()).toEqual('/'); + const urlChanges = ['replace: /', 'hash: /one', 'replace: /']; + expect(location.urlChanges).toEqual(urlChanges); })); }); });