Skip to content

feat(router): convert lastSuccessfulNavigation to signal #63057

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

feat(router): convert lastSuccessfulNavigation to signal #63057

wants to merge 1 commit into from

Conversation

JeanMeche
Copy link
Member

This commit also include an ng update migration to ensure lastSuccessfulNavigation is invoked.

BREAKING CHANGE: lastSuccessfulNavigation is now a signal and needs to be invoked

@JeanMeche JeanMeche added this to the v21 Candidate milestone Aug 7, 2025
@ngbot ngbot bot removed this from the v21 Candidate milestone Aug 7, 2025
@JeanMeche JeanMeche added the target: major This PR is targeted for the next major release label Aug 7, 2025
@JeanMeche JeanMeche added this to the v21 Candidate milestone Aug 7, 2025
@ngbot ngbot bot removed this from the v21 Candidate milestone Aug 7, 2025
@angular-robot angular-robot bot added detected: breaking change PR contains a commit with a breaking change detected: feature PR contains a feature commit area: router labels Aug 7, 2025
@ngbot ngbot bot added this to the Backlog milestone Aug 7, 2025
@JeanMeche JeanMeche modified the milestones: Backlog, v21 Candidate Aug 7, 2025
@JeanMeche JeanMeche force-pushed the lastSuccessfulNavigation branch 2 times, most recently from 3046187 to 30373f0 Compare August 11, 2025 23:20
@JeanMeche
Copy link
Member Author

The patch CL to submit alongside this PR: https://critique.corp.google.com/cl/793706017

@JeanMeche JeanMeche added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Aug 11, 2025
@JeanMeche JeanMeche marked this pull request as ready for review August 11, 2025 23:45
@pullapprove pullapprove bot requested a review from AndrewKushnir August 11, 2025 23:45
@JeanMeche JeanMeche requested review from atscott and removed request for AndrewKushnir August 11, 2025 23:46
@pullapprove pullapprove bot requested a review from AndrewKushnir August 11, 2025 23:46
@JeanMeche JeanMeche removed the request for review from AndrewKushnir August 11, 2025 23:47
@atscott atscott added the action: merge The PR is ready for merge by the caretaker label Aug 11, 2025
atscott
atscott approved these changes Aug 12, 2025
View reviewed changes
Copy link
Contributor

@atscott atscott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed-for: public-api

@pullapprove pullapprove bot requested a review from crisbeto August 12, 2025 00:05
@JeanMeche
feat(router): convert lastSuccessfulNavigation to signal
9117590 
This commit also include an `ng update` migration to ensure `lastSuccessfulNavigation` is invoked.

BREAKING CHANGE: `lastSuccessfulNavigation` is now a signal and needs to be invoked
@JeanMeche JeanMeche force-pushed the lastSuccessfulNavigation branch from 30373f0 to 9117590 Compare August 12, 2025 01:03
@JeanMeche JeanMeche mentioned this pull request Aug 12, 2025
Closed
crisbeto
crisbeto approved these changes Aug 13, 2025
View reviewed changes
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-for: public-api

@atscott
Copy link
Contributor

atscott commented Aug 13, 2025

This PR was merged into the repository by commit 4e0fc81.

The changes were merged into the following branches: main

@atscott atscott closed this in 4e0fc81 Aug 13, 2025
@JeanMeche JeanMeche deleted the lastSuccessfulNavigation branch August 29, 2025 00:43
@IgorSedov
Copy link

@JeanMeche I noticed a change in the behavior of lastSuccessfulNavigation.

On a direct page visit (the initial navigation), the lastSuccessfulNavigation() signal is populated with a Navigation object (id: 1). In the previous versions Angular (for example, 20.2), lastSuccessfulNavigation would have been null initially.

Is this new behavior intentional and an expected part of the new signal-based API? I couldn't find info of this specific behavior in the PR, so I wanted to confirm.

Thank you for the clarification.

@atscott
Copy link
Contributor

atscott commented Sep 3, 2025

@IgorSedov Do you have a reproduction? There were no changes to the timing of the updates in this CL

@IgorSedov
Copy link

@atscott My bad. Thanks for the quick response. I found the problem on my end.

I tested in different configurations (SSR, Zoneless, v20/v21) and everything is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@atscott atscott atscott approved these changes

@crisbeto crisbeto crisbeto approved these changes

@AndrewKushnir AndrewKushnir Awaiting requested review from AndrewKushnir

@kirjs kirjs Awaiting requested review from kirjs

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker area: router breaking changes detected: breaking change PR contains a commit with a breaking change detected: feature PR contains a feature commit merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: major This PR is targeted for the next major release
Projects
None yet
Milestone
v21 Candidate
Development

Successfully merging this pull request may close these issues.
4 participants
@JeanMeche @atscott @IgorSedov @crisbeto