-
Notifications
You must be signed in to change notification settings - Fork 26.3k
feat(router): Cascading route support (4.3.x) #18163
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.
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
Conversation
Added cascading routes capability to the router, added new test cases. Previous behavior resulted in dead routes (e.g. direct navigation to a route with canActivate=>false resulted in blank page). Additionally this feature improvement provides conditional routing based on params and other information that could be used to dynamically enable/disable a route while ensuring the user always falls through to a valid route. One existing test was modified which needed to verify depth-first-order on matching. recognize() can sequence all matches now. However, the records will still be returned by recognize() with the same depth-first-order. Passes all latest tests and is backwards compatible.
@matthewerwin Why are there 3 copies of this PR? |
@Toxicable there were differences due to a number of things that happened to Angular codebase since the 4.0.x version -- this PR has been around for a long time. Since I'm unsure where it makes most sense for the Angular team to merge (4.3.x would be closest to 5.x, or we can issue a PR against master). The software change is backwards compatible so I don't know if that'll be a factor related to where it should be merged. I'm happy to delete whatever PR's don't make sense and issue against whatever branch it does...just trying to make sure that it can be managed as desired by Angular team and I'll make sure any future PR's are in alignment with the decision on how this is merged. |
I don't mean sound pedantic here, but like I said before, it cannot be merged into those older versions. Even though it's a old PR, what version it goes into is based on when it's merged not when the PR was made. Even though it's backwards compatible that's only because of semantic versioning which you would be breaking by merging this into 4.0, 4.1, 4.2 or 4.3 therefore the only option is v5, unless there is a 4.4 which there shouldn't be. |
Let's just move forward with this version since it's the one that's synced to the more recent version of Angular. @matthewerwin You can go ahead and do the compare against |
@matthewerwin Ignore above comment. Let's go back to #16416 as the main PR since that has the most history. Please rebase that one against master and submit the compare against master. Closing this PR. |
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. |
See #16416
This is rebased for merging into 4.3.x
@jasonaden FYI includes your code review request for changes on the original 4.0.x pull request.