fix(router): restore internal URL on popstate when browserUrl is used#67557
fix(router): restore internal URL on popstate when browserUrl is used#67557SkyZeroZx wants to merge 1 commit intoangular:mainfrom
browserUrl is used#67557Conversation
| }) | ||
| class RootCmp {} | ||
|
|
||
| describe('browserUrl with popstate navigation', () => { |
There was a problem hiding this comment.
I'm wondering if we should put it in integration (I haven't found any tests related to browserUrl) or if it would be fine here, even though I use integration helpers.
There was a problem hiding this comment.
Putting this directly in packages/router/test/integration/integration.spec.ts seems good to me
| } else { | ||
| const newState = { | ||
| ...state, | ||
| ...this.routerUrlState(navigation), |
There was a problem hiding this comment.
This should be in generateNgRouterState
| : 'push'; | ||
| const state = { | ||
| ...transition.extras.state, | ||
| ...this.routerUrlState(transition), |
There was a problem hiding this comment.
This is now duplicated twice here. Let's make a similar generateNgRouterState like in history state manager
| } | ||
| const rawUrl = | ||
| finalUrl !== undefined ? this.urlHandlingStrategy.merge(finalUrl!, initialUrl) : initialUrl; | ||
| return {ɵrouterUrl: this.urlSerializer.serialize(rawUrl)}; |
There was a problem hiding this comment.
I don't believe this is correct. The point of this is to preserve the internal URL used for matching routes. This is finalUrl only
| }) | ||
| class RootCmp {} | ||
|
|
||
| describe('browserUrl with popstate navigation', () => { |
There was a problem hiding this comment.
Putting this directly in packages/router/test/integration/integration.spec.ts seems good to me
| const location = TestBed.inject(Location); | ||
| const router = TestBed.inject(Router); | ||
|
|
||
| await createRoot(router, RootCmp); |
There was a problem hiding this comment.
Use RouterTestingHarness instead
Fixed an issue where back/forward (`popstate`) navigation attempted to match the displayed `browserUrl` instead of the internal route, which could result in `NG04002: Cannot match any routes`. Fixes angular#67549
066ae0a to
0a0c674
Compare
Fixed an issue where back/forward (
popstate) navigation attempted to match the displayedbrowserUrlinstead of the internal route, which could result inNG04002: Cannot match any routes.Fixes #67549
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information