Combine recognize and apply redirects#49163
Closed
atscott wants to merge 2 commits intoangular:mainfrom
Closed
Conversation
atscott
added
area: router
target: major
atscott
force-pushed
the
combineRecognizeAndApplyRedirects
branch
3 times, most recently
from
9f3d0c4 to
e53bc2a
Compare
Contributor
AndrewKushnir
left a comment
AndrewKushnir
left a comment
There was a problem hiding this comment.
@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.
atscott
force-pushed
the
combineRecognizeAndApplyRedirects
branch
2 times, most recently
from
4ba3d6d to
627d211
Compare
atscott
force-pushed
the
combineRecognizeAndApplyRedirects
branch
from
627d211 to
f49bbd3
Compare
AndrewKushnir
approved these changes
Mar 27, 2023
Contributor
AndrewKushnir
left a comment
AndrewKushnir
left a comment
There was a problem hiding this comment.
Reviewed-for: size-tracking
atscott
added
action: global presubmit
Contributor
Author
|
This PR was merged into the repository by commit 1600687. |
atscott
added a commit
to atscott/angular
that referenced
this pull request
Mar 29, 2023
…gating (angular#49163)" This reverts commit 1600687.
atscott
added a commit
that referenced
this pull request
Mar 29, 2023
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
force-pushed
the
combineRecognizeAndApplyRedirects
branch
from
f49bbd3 to
ea1a12d
Compare
Contributor
Author
|
Re-merging. I've confirmed this was an issue with the internal route configuration. |
Contributor
Author
|
This PR was merged into the repository by commit 3c7e637. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.