-
Notifications
You must be signed in to change notification settings - Fork 11.9k
fix(@angular/ssr): ensure server-side navigation triggers a redirect #31497
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
dbdf556 to
0d20150
Compare
0d20150 to
53180a8
Compare
cf9f7db to
d9dc146
Compare
d9dc146 to
27ffba9
Compare
| const pendingTasks = inject(PendingTasks); | ||
|
|
||
| pendingTasks.run(() => | ||
| router.navigate([], { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we maybe do this in a guard instead? As discussed, I don’t love that this causes the navigation to not complete successfully.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to keep this spec case as similar to the reported issue as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, I get that. Can you double check that this actually does also happen (as in it doesn’t work as a redirect) for navigations in guards? By far, that’s more common so at the very least, you’d want to cover that case too. This test is good on its own, it’s just that I wish it already worked correctly with the old code and didn’t need this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without this change, within a guard the navigation change does not work as a redirect.
27ffba9 to
305aaa1
Compare
4bea198 to
336a6c6
Compare
When a navigation occurs on the server-side, such as using `router.navigate`, and the final URL is different from the initial URL that was requested, the server should respond with a redirect. Previously, the initial URL was being read from `router.lastSuccessfulNavigation.initialUrl`, which could be incorrect in scenarios involving server-side navigations, causing the comparison with the final URL to fail and preventing the redirect. This change ensures that the initial URL requested by the browser is used for the comparison, correctly triggering a redirect when necessary. Closes angular#31482
336a6c6 to
9c3a689
Compare
When a navigation occurs on the server-side, such as using
router.navigate, and the final URL is different from the initial URL that was requested, the server should respond with a redirect.Previously, the initial URL was being read from
router.lastSuccessfulNavigation.initialUrl, which could be incorrect in scenarios involving server-side navigations, causing the comparison with the final URL to fail and preventing the redirect.This change ensures that the initial URL requested by the browser is used for the comparison, correctly triggering a redirect when necessary.
Closes #31482