fix(common): Update Location to get a normalized URL valid in case a represented URL starts with the substring equals APP_BASE_HREF#48394
Conversation
atscott
left a comment
There was a problem hiding this comment.
Awesome, thanks for taking this one on too :)
pkozlowski-opensource
left a comment
There was a problem hiding this comment.
Thnx for the fix!
I've left a small nit / cleanup comment, feel free to address or ignore.
|
I see that you just added the If you want your PR to be merged, it has to pass all the CI checks. If you can't get the PR to a green state due to flakes or broken |
…a represented URL starts with the substring equals `APP_BASE_HREF` ```ts @NgModule({ imports: [RouterModule.forRoot([{path: '/enigma', component: EnigmaComponent}])], providers: [{provide: APP_BASE_HREF, useValue: '/en'}] }) export class AppModule {} ``` Navigating to `/enigma` will redirect to `/en/igma` not to `/en/enigma` as it expects Fixes: angular#45744
1303d28 to
ae0efb6
Compare
|
caretaker note: as far as I can tell the failing G3 check is pre-existing and not related to this change |
|
This PR was merged into the repository by commit d87285c. |
…a represented URL starts with the substring equals `APP_BASE_HREF` (#48394) ```ts @NgModule({ imports: [RouterModule.forRoot([{path: '/enigma', component: EnigmaComponent}])], providers: [{provide: APP_BASE_HREF, useValue: '/en'}] }) export class AppModule {} ``` Navigating to `/enigma` will redirect to `/en/igma` not to `/en/enigma` as it expects Fixes: #45744 PR Close #48394
|
@constantant This caused a breakage in Google3 and had to be reverted in #48461. |
|
@jessicajaniuk did you see the comment from @pkozlowski-opensource ? |
|
@constantant That was pre-merge. Post-merge we got direct reports of multiple failures breaking tests and builds from those teams. So it actually isn't irrelevant. Reverting this fixed their broken test cases. Apps were unable to reach certain paths after this change landed. I'm doing a bit of investigating as to why. |
|
@constantant OK, after a little bit of investigating, we've figured out the cause. When the URL has query params, matrix params, or fragments at the end, the check on line 304 in your PR fails to match up and moves to the second part, which duplicates the APP_BASE_HREF in the url. So what happens is something like this:
path: This would happen with query params, like above, or fragments, like |
|
@jessicajaniuk I will take care of it. Thanks for explanation. |
|
@jessicajaniuk I have opened another PR: #48489 |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
…a represented URL starts with the substring equals `APP_BASE_HREF` (angular#48394) ```ts @NgModule({ imports: [RouterModule.forRoot([{path: '/enigma', component: EnigmaComponent}])], providers: [{provide: APP_BASE_HREF, useValue: '/en'}] }) export class AppModule {} ``` Navigating to `/enigma` will redirect to `/en/igma` not to `/en/enigma` as it expects Fixes: angular#45744 PR Close angular#48394
Update
Locationto get a normalized URL valid in case a represented URL starts with the substring equalsAPP_BASE_HREFNavigating to
/enigmawill redirect to/en/igmanot to/en/enigmaas it expectsFixes: #45744
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: 45744
What is the new behavior?
Does this PR introduce a breaking change?
Other information