fix(router): ensure named outlets with empty path parents are matched#40029
Closed
atscott wants to merge 9 commits intoangular:masterfrom
Closed
fix(router): ensure named outlets with empty path parents are matched#40029atscott wants to merge 9 commits intoangular:masterfrom
atscott wants to merge 9 commits intoangular:masterfrom
Conversation
mhevery
reviewed
Dec 9, 2020
mhevery
reviewed
Dec 9, 2020
Contributor
mhevery
left a comment
There was a problem hiding this comment.
Thank you for all of the new comments. It is a good start! Please keep comenting.
zarend
pushed a commit
to zarend/angular
that referenced
this pull request
Dec 10, 2020
b05d434 to
44cefbc
Compare
e767ddb to
44486ef
Compare
Contributor
Author
When stepping through the `recognize` algorithm, it is much easier to follow when using a simple `for...of` rather than the helper `mapChildrenIntoArray` with the passed closure. The only special thing that `mapChildrenIntoArray` does is ensure the primary route appears first. This change will have no affect on the result because `processChildren` later calls `sortActivatedRouteSnapshots`, which does the same thing.
…o match This commit updates the `recognize` algorithm to return `null` when a segment does not match a given config rather than throwing an error. This makes the code much easier to follow because the "no match" result has to be explicitly handled rather than catching the error in very specific places.
9c71295 to
b4e1b64
Compare
Contributor
Author
14 tasks
josephperrott
pushed a commit
that referenced
this pull request
Jan 5, 2021
…o match (#40029) This commit updates the `recognize` algorithm to return `null` when a segment does not match a given config rather than throwing an error. This makes the code much easier to follow because the "no match" result has to be explicitly handled rather than catching the error in very specific places. PR Close #40029
josephperrott
pushed a commit
that referenced
this pull request
Jan 5, 2021
To make the tests suite easier to follow, `Recognize#apply` can be made into a synchronous function rather than one that return an `Observable`. Also, as a chore, remove as many `any` types as possible. PR Close #40029
josephperrott
pushed a commit
that referenced
this pull request
Jan 5, 2021
…zed (#40029) This commit updates the `recognize` algorithm to work with named outlets which have empty path parents. For example, given the following config ``` const routes = [ { path: '', children: [ {path: 'a', outlet: 'aux', component: AuxComponent} ]} ]; ``` The url `/(aux:a)` should match this config. In order to do so, we need to allow the children of `UrlSegmentGroup`s to match a `Route` config for a different outlet (in this example, the `primary`) when it's an empty path. This should also *only* happen if we were unable to find a match for the outlet in the level above. That is, the matching strategy is to find the first `Route` in the list which _matches the given outlet_. If we are unable to do that, then we allow empty paths from other outlets to match and try to find some child there whose outlet matches our segment. PR Close #40029
josephperrott
pushed a commit
that referenced
this pull request
Jan 5, 2021
The `applyRedirects` and `recognize` algorithms have the same overall goal: match a `UrlTree` with the application's `Routes` config. There are a few key functions in these algorithms which can be shared rather than duplicated between the two. This also makes it easier to see how the two are similar and where they diverge. PR Close #40029
josephperrott
pushed a commit
that referenced
this pull request
Jan 5, 2021
…th parents (#40029) There are two parts to this commit: 1. Revert the changes from #38379. This change had an incomplete view of how things worked and also diverged the implementations of `applyRedirects` and `recognize` even more. 2. Apply the fixes from the `recognize` algorithm to ensure that named outlets with empty path parents can be matched. This change also passes all the tests that were added in #38379 with the added benefit of being a more complete fix that stays in-line with the `recognize` algorithm. This was made possible by using the same approach for `split` by always creating segments for empty path matches (previously, this was only done in `applyRedirects` if there was a `redirectTo` value). At the end of the expansions, we need to squash all empty segments so that serializing the final `UrlTree` returns the same result as before. Fixes #39952 Fixes #10726 Closes #30410 PR Close #40029
atscott
added a commit
to atscott/angular
that referenced
this pull request
Jan 5, 2021
…per (angular#40029) When stepping through the `recognize` algorithm, it is much easier to follow when using a simple `for...of` rather than the helper `mapChildrenIntoArray` with the passed closure. The only special thing that `mapChildrenIntoArray` does is ensure the primary route appears first. This change will have no affect on the result because `processChildren` later calls `sortActivatedRouteSnapshots`, which does the same thing. PR Close angular#40029
atscott
added a commit
to atscott/angular
that referenced
this pull request
Jan 5, 2021
…o match (angular#40029) This commit updates the `recognize` algorithm to return `null` when a segment does not match a given config rather than throwing an error. This makes the code much easier to follow because the "no match" result has to be explicitly handled rather than catching the error in very specific places. PR Close angular#40029
atscott
added a commit
to atscott/angular
that referenced
this pull request
Jan 5, 2021
To make the tests suite easier to follow, `Recognize#apply` can be made into a synchronous function rather than one that return an `Observable`. Also, as a chore, remove as many `any` types as possible. PR Close angular#40029
atscott
added a commit
to atscott/angular
that referenced
this pull request
Jan 5, 2021
…zed (angular#40029) This commit updates the `recognize` algorithm to work with named outlets which have empty path parents. For example, given the following config ``` const routes = [ { path: '', children: [ {path: 'a', outlet: 'aux', component: AuxComponent} ]} ]; ``` The url `/(aux:a)` should match this config. In order to do so, we need to allow the children of `UrlSegmentGroup`s to match a `Route` config for a different outlet (in this example, the `primary`) when it's an empty path. This should also *only* happen if we were unable to find a match for the outlet in the level above. That is, the matching strategy is to find the first `Route` in the list which _matches the given outlet_. If we are unable to do that, then we allow empty paths from other outlets to match and try to find some child there whose outlet matches our segment. PR Close angular#40029
atscott
added a commit
to atscott/angular
that referenced
this pull request
Jan 5, 2021
…ar#40029) The `applyRedirects` and `recognize` algorithms have the same overall goal: match a `UrlTree` with the application's `Routes` config. There are a few key functions in these algorithms which can be shared rather than duplicated between the two. This also makes it easier to see how the two are similar and where they diverge. PR Close angular#40029
atscott
added a commit
to atscott/angular
that referenced
this pull request
Jan 5, 2021
…th parents (angular#40029) There are two parts to this commit: 1. Revert the changes from angular#38379. This change had an incomplete view of how things worked and also diverged the implementations of `applyRedirects` and `recognize` even more. 2. Apply the fixes from the `recognize` algorithm to ensure that named outlets with empty path parents can be matched. This change also passes all the tests that were added in angular#38379 with the added benefit of being a more complete fix that stays in-line with the `recognize` algorithm. This was made possible by using the same approach for `split` by always creating segments for empty path matches (previously, this was only done in `applyRedirects` if there was a `redirectTo` value). At the end of the expansions, we need to squash all empty segments so that serializing the final `UrlTree` returns the same result as before. Fixes angular#39952 Fixes angular#10726 Closes angular#30410 PR Close angular#40029
josephperrott
pushed a commit
that referenced
this pull request
Jan 5, 2021
…per (#40029) (#40315) When stepping through the `recognize` algorithm, it is much easier to follow when using a simple `for...of` rather than the helper `mapChildrenIntoArray` with the passed closure. The only special thing that `mapChildrenIntoArray` does is ensure the primary route appears first. This change will have no affect on the result because `processChildren` later calls `sortActivatedRouteSnapshots`, which does the same thing. PR Close #40029 PR Close #40315
josephperrott
pushed a commit
that referenced
this pull request
Jan 5, 2021
…o match (#40029) (#40315) This commit updates the `recognize` algorithm to return `null` when a segment does not match a given config rather than throwing an error. This makes the code much easier to follow because the "no match" result has to be explicitly handled rather than catching the error in very specific places. PR Close #40029 PR Close #40315
josephperrott
pushed a commit
that referenced
this pull request
Jan 5, 2021
…zed (#40029) (#40315) This commit updates the `recognize` algorithm to work with named outlets which have empty path parents. For example, given the following config ``` const routes = [ { path: '', children: [ {path: 'a', outlet: 'aux', component: AuxComponent} ]} ]; ``` The url `/(aux:a)` should match this config. In order to do so, we need to allow the children of `UrlSegmentGroup`s to match a `Route` config for a different outlet (in this example, the `primary`) when it's an empty path. This should also *only* happen if we were unable to find a match for the outlet in the level above. That is, the matching strategy is to find the first `Route` in the list which _matches the given outlet_. If we are unable to do that, then we allow empty paths from other outlets to match and try to find some child there whose outlet matches our segment. PR Close #40029 PR Close #40315
josephperrott
pushed a commit
that referenced
this pull request
Jan 5, 2021
… (#40315) The `applyRedirects` and `recognize` algorithms have the same overall goal: match a `UrlTree` with the application's `Routes` config. There are a few key functions in these algorithms which can be shared rather than duplicated between the two. This also makes it easier to see how the two are similar and where they diverge. PR Close #40029 PR Close #40315
josephperrott
pushed a commit
that referenced
this pull request
Jan 5, 2021
…th parents (#40029) (#40315) There are two parts to this commit: 1. Revert the changes from #38379. This change had an incomplete view of how things worked and also diverged the implementations of `applyRedirects` and `recognize` even more. 2. Apply the fixes from the `recognize` algorithm to ensure that named outlets with empty path parents can be matched. This change also passes all the tests that were added in #38379 with the added benefit of being a more complete fix that stays in-line with the `recognize` algorithm. This was made possible by using the same approach for `split` by always creating segments for empty path matches (previously, this was only done in `applyRedirects` if there was a `redirectTo` value). At the end of the expansions, we need to squash all empty segments so that serializing the final `UrlTree` returns the same result as before. Fixes #39952 Fixes #10726 Closes #30410 PR Close #40029 PR Close #40315
|
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.
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.
see individual commits