Skip to content

Conversation

@alan-agius4
Copy link
Collaborator

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

@alan-agius4 alan-agius4 added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Oct 16, 2025
@alan-agius4 alan-agius4 requested a review from hybrist October 16, 2025 08:41
@alan-agius4 alan-agius4 force-pushed the navigation-initial-url branch from dbdf556 to 0d20150 Compare October 16, 2025 09:18
@alan-agius4 alan-agius4 force-pushed the navigation-initial-url branch from 0d20150 to 53180a8 Compare October 16, 2025 09:32
@alan-agius4 alan-agius4 reopened this Oct 16, 2025
@alan-agius4 alan-agius4 force-pushed the navigation-initial-url branch 4 times, most recently from cf9f7db to d9dc146 Compare October 16, 2025 13:20
@alan-agius4 alan-agius4 requested review from atscott and removed request for hybrist October 16, 2025 13:20
@alan-agius4 alan-agius4 force-pushed the navigation-initial-url branch from d9dc146 to 27ffba9 Compare October 16, 2025 13:21
const pendingTasks = inject(PendingTasks);

pendingTasks.run(() =>
router.navigate([], {
Copy link
Contributor

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.

Copy link
Collaborator Author

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.

Copy link
Contributor

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

Copy link
Collaborator Author

@alan-agius4 alan-agius4 Oct 16, 2025

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.

@alan-agius4 alan-agius4 force-pushed the navigation-initial-url branch from 27ffba9 to 305aaa1 Compare October 16, 2025 13:51
@alan-agius4 alan-agius4 requested a review from atscott October 16, 2025 13:51
@alan-agius4 alan-agius4 force-pushed the navigation-initial-url branch 2 times, most recently from 4bea198 to 336a6c6 Compare October 16, 2025 13:54
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
@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release and removed action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Oct 16, 2025
@alan-agius4 alan-agius4 merged commit 58dcfd1 into angular:main Oct 16, 2025
32 of 33 checks passed
@alan-agius4 alan-agius4 deleted the navigation-initial-url branch October 16, 2025 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker area: @angular/ssr target: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Server side router navigation with changes to query parameters not sending a redirect response

2 participants