Skip to content

Conversation

@atscott
Copy link
Contributor

@atscott atscott commented Feb 22, 2023

No description provided.

@ngbot ngbot bot modified the milestone: Backlog Feb 22, 2023
@atscott atscott force-pushed the combineRecognizeAndApplyRedirects branch 3 times, most recently from 9f3d0c4 to e53bc2a Compare February 22, 2023 22:04
@atscott atscott marked this pull request as ready for review February 27, 2023 22:08
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

@atscott I've looked through the change and I'd like to ask if we can pair to view it together, so I can better understand the context. Thank you.

@pullapprove pullapprove bot requested a review from AndrewKushnir February 28, 2023 01:56
@atscott atscott force-pushed the combineRecognizeAndApplyRedirects branch 2 times, most recently from 4ba3d6d to 627d211 Compare March 1, 2023 17:03
@atscott atscott force-pushed the combineRecognizeAndApplyRedirects branch from 627d211 to f49bbd3 Compare March 27, 2023 21:33
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

👍

@pullapprove pullapprove bot requested a review from AndrewKushnir March 27, 2023 22:09
Copy link
Contributor

@AndrewKushnir AndrewKushnir 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: size-tracking

@atscott atscott removed the request for review from dylhunn March 27, 2023 22:46
@atscott atscott added action: global presubmit The PR is in need of a google3 global presubmit 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 and removed action: global presubmit The PR is in need of a google3 global presubmit labels Mar 27, 2023
@atscott
Copy link
Contributor Author

atscott commented Mar 28, 2023

@atscott
Copy link
Contributor Author

atscott commented Mar 28, 2023

This PR was merged into the repository by commit 1600687.

@atscott atscott closed this in 1600687 Mar 28, 2023
atscott added a commit to atscott/angular that referenced this pull request Mar 29, 2023
atscott added a commit that referenced this pull request Mar 29, 2023
@atscott atscott reopened this Mar 29, 2023
atscott added 2 commits March 29, 2023 13:45
When navigating in the Router, the current approach does the redirects
and the creation of the `RouterStateSnapshot` in two separate steps
(applyRedirects and recognize). These two steps duplicate the route
matching logic, resulting in user code on routes being executing twice
(custom `UrlMatcher` and `canMatch` guards). This also duplicates the
complex matching logic in two places, which increases the bundle size
and maintenance burden.

This commit combines the `applyRedirects` and `recognize` steps into a
single matching algorithm.

fixes angular#26081
@atscott atscott force-pushed the combineRecognizeAndApplyRedirects branch from f49bbd3 to ea1a12d Compare March 29, 2023 20:45
@atscott
Copy link
Contributor Author

atscott commented Mar 29, 2023

Re-merging. I've confirmed this was an issue with the internal route configuration.

@atscott
Copy link
Contributor Author

atscott commented Mar 29, 2023

This PR was merged into the repository by commit 3c7e637.

@atscott atscott closed this in 3c7e637 Mar 29, 2023
@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 Apr 29, 2023
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 merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note router: config matching/activation/validation 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