Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@atscott
Copy link
Contributor

@atscott atscott commented Aug 4, 2025

This is effectively a revert of 72e6a94.
Debugging the recognize stage is considerably easier with async/await
stacks compared to rxjs. This also improves maintainability and is a
better 1:1 with server-side logic that has been implemented to match
and can be more easily kept in sync.

BREAKING CHANGE: Router navigations may take several additional
microtasks to complete. Tests have been found to often be highly
dependent on the exact timing of navigation completions with respect to
the microtask queue. The most common fix for tests is to ensure all
navigations have been completed before making assertions. On rare
occasions, this can also affect production applications. This can be
caused by multiple subscriptions to router state throughout the application,
both of which trigger navigations that happened to not conflict with the
previous timing.

@atscott atscott added the target: major This PR is targeted for the next major release label Aug 4, 2025
@angular-robot angular-robot bot added detected: breaking change PR contains a commit with a breaking change area: router labels Aug 4, 2025
@ngbot ngbot bot added this to the Backlog milestone Aug 4, 2025
@atscott atscott force-pushed the recognizetopromise2 branch 6 times, most recently from 33addc6 to 006cab6 Compare August 7, 2025 18:01
@atscott atscott force-pushed the recognizetopromise2 branch from 006cab6 to ae4e101 Compare August 13, 2025 16:54
@atscott atscott force-pushed the recognizetopromise2 branch 3 times, most recently from 3a9a217 to d52fcfa Compare August 27, 2025 20:18
@atscott
Copy link
Contributor Author

atscott commented Aug 27, 2025

caretaker note: this will need patch cl/800152294 to temporarily disable the change in g3

@atscott atscott marked this pull request as ready for review August 27, 2025 20:35
@atscott atscott force-pushed the recognizetopromise2 branch from d52fcfa to 26b84bb Compare August 27, 2025 20:37
This is effectively a revert of angular@72e6a94.
Debugging the recognize stage is considerably easier with async/await
stacks compared to rxjs. This also improves maintainability and is a
better 1:1 with server-side logic that has been implemented to match
and can be more easily kept in sync.

This also ensures that the recognize step is always async, whereas it
can sometimes be synchronous with rxjs.

BREAKING CHANGE: Router navigations may take several additional
microtasks to complete. Tests have been found to often be highly
dependent on the exact timing of navigation completions with respect to
the microtask queue. The most common fix for tests is to ensure all
navigations have been completed before making assertions. On rare
occasions, this can also affect production applications. This can be
caused by multiple subscriptions to router state throughout the application,
both of which trigger navigations that happened to not conflict with the
previous timing.
@atscott atscott force-pushed the recognizetopromise2 branch from 26b84bb to 0658b79 Compare August 28, 2025 17:05
To aid in hitting external breaking change deadlines without pressure
of fixing everything in g3 first, add an internal opt out flag.

This also adds a privately exported provider to revert to the old
rxjs-based behavior, which can be synchronous, until any issues that
come up are addressed.
@atscott atscott force-pushed the recognizetopromise2 branch from 0658b79 to 87ed780 Compare August 28, 2025 19:06
@atscott atscott added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels Aug 29, 2025
@mmalerba
Copy link
Contributor

This PR was merged into the repository. The changes were merged into the following branches:

@mmalerba mmalerba closed this in 5b53535 Aug 29, 2025
mmalerba pushed a commit that referenced this pull request Aug 29, 2025
#62994)

To aid in hitting external breaking change deadlines without pressure
of fixing everything in g3 first, add an internal opt out flag.

This also adds a privately exported provider to revert to the old
rxjs-based behavior, which can be synchronous, until any issues that
come up are addressed.

PR Close #62994
atscott added a commit to atscott/angular that referenced this pull request Aug 29, 2025
This fixes a bug introduced in angular#62994 where `toPromise` was used. This
doesn't work in all situations here because some observables don't
complete. This only affected the redirect path, since the others are
already behind other rxjs code which takes the first value from the user
guards.

Note: This is marked as a refactor rather than fix because the commit above is not in
any release yet.
atscott added a commit to atscott/angular that referenced this pull request Aug 29, 2025
This fixes a bug introduced in angular#62994 where `toPromise` was used. This
doesn't work in all situations here because some observables don't
complete. This only affected the redirect path, since the others are
already behind other rxjs code which takes the first value from the user
guards.

Note: This is marked as a refactor rather than fix because the commit above is not in
any release yet.
atscott added a commit to atscott/angular that referenced this pull request Aug 29, 2025
This fixes a bug introduced in angular#62994 where `toPromise` was used. This
doesn't work in all situations here because some observables don't
complete. This only affected the redirect path, since the others are
already behind other rxjs code which takes the first value from the user
guards.

Note: This is marked as a refactor rather than fix because the commit above is not in
any release yet.
atscott added a commit to atscott/angular that referenced this pull request Aug 29, 2025
This fixes a bug introduced in angular#62994 where `toPromise` was used. This
doesn't work in all situations here because some observables don't
complete. This only affected the redirect path, since the others are
already behind other rxjs code which takes the first value from the user
guards.

Note: This is marked as a refactor rather than fix because the commit above is not in
any release yet.
mmalerba pushed a commit that referenced this pull request Aug 29, 2025
This fixes a bug introduced in #62994 where `toPromise` was used. This
doesn't work in all situations here because some observables don't
complete. This only affected the redirect path, since the others are
already behind other rxjs code which takes the first value from the user
guards.

Note: This is marked as a refactor rather than fix because the commit above is not in
any release yet.

PR Close #63485
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 30, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: router detected: breaking change PR contains a commit with a breaking change 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

Development

Successfully merging this pull request may close these issues.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.