Skip to content

Conversation

@atscott
Copy link
Contributor

@atscott atscott commented May 3, 2022

Note: This change is breaking but I would like to consider introducing the function to create a UrlTree from an ActivatedRouteSnapshot as a feature in a minor version. This PR is being created to run global presubmits to stress test this function in place of the existing createUrlTree used by the router.

atscott added 7 commits May 3, 2022 15:03
BREAKING CHANGE: The Router is now able to generate a correct URL from
any given `ActivatedRoute`. Previously, if the `ActivatedRoute` provided
as `relativeTo` did not appear in the current URL tree, the `Router`
would effectively ignore the `relativeTo` and just use the current
state. Navigations which intend to keep the path the same, but update
the URL should not specify a `relativeTo` (`router.createUrlTree([],
{queryParams: newParams})`).
@atscott
Copy link
Contributor Author

atscott commented May 3, 2022

presubmit

@atscott
Copy link
Contributor Author

atscott commented May 4, 2022

TGP looks good. The only failures were 2 teams accessing the private _urlSegment which was removed in this PR.

Next steps will be to clean up the create_url_tree file to preserve the old way of creating URLs as the default and expose a new function for the future replacement.

@atscott atscott closed this May 4, 2022
atscott added a commit to atscott/angular that referenced this pull request May 17, 2022
…eSnapshot`

This exposes a new function from the router public API that allows
developers to create a `UrlTree` from _any_ `ActivatedRouteSnapshot`.
The current Router APIs only support creating a `UrlTree` from an
`ActivatedRoute` which is both active _and_ materially appears in the
`UrlTree` (it cannot be an empty path named outlet). This is because the
implementation of the current way of creating a `UrlTree` attempts to
look up the `UrlSegment` of the `ActivatedRoute` in the currently active
`UrlTree` of the router. When this doesn't work, the `UrlTree` creation
fails.

Note that this API does not replace the current one. That would actually be a
breaking change but should be done at some point in the future (v15). That is,
`router.navigate` should call this new function. At that point, we can
remove `_lastPathIndex`, `_urlSegment`, and `_correctedPathIndex` from
the `ActivatedRoute`, along with all of the logic associated with
determining what those should be. In addition, this would unblock a fix
for angular#26081 because the `applyRedirects` and `recognize` operations
could be combined into one.  Overall, this would simplify logic in the router
and reduce code size. It also exposes core routing capabilities as a helper function
which were previously private API, which is a necessary step towards angular#42953.

As a stress test for this new function, it _was_ swapped in as the
default for `UrlTree` creation in angular#45859 and tested internally. The
results indicate that this function behaves correctly.

resolves angular#42191 (Tested directly)
resolves angular#38276 (The test with a guard covers this case)
resolves angular#22763 (Tested directly)
alxhub pushed a commit that referenced this pull request May 24, 2022
…eSnapshot` (#45877)

This exposes a new function from the router public API that allows
developers to create a `UrlTree` from _any_ `ActivatedRouteSnapshot`.
The current Router APIs only support creating a `UrlTree` from an
`ActivatedRoute` which is both active _and_ materially appears in the
`UrlTree` (it cannot be an empty path named outlet). This is because the
implementation of the current way of creating a `UrlTree` attempts to
look up the `UrlSegment` of the `ActivatedRoute` in the currently active
`UrlTree` of the router. When this doesn't work, the `UrlTree` creation
fails.

Note that this API does not replace the current one. That would actually be a
breaking change but should be done at some point in the future (v15). That is,
`router.navigate` should call this new function. At that point, we can
remove `_lastPathIndex`, `_urlSegment`, and `_correctedPathIndex` from
the `ActivatedRoute`, along with all of the logic associated with
determining what those should be. In addition, this would unblock a fix
for #26081 because the `applyRedirects` and `recognize` operations
could be combined into one.  Overall, this would simplify logic in the router
and reduce code size. It also exposes core routing capabilities as a helper function
which were previously private API, which is a necessary step towards #42953.

As a stress test for this new function, it _was_ swapped in as the
default for `UrlTree` creation in #45859 and tested internally. The
results indicate that this function behaves correctly.

resolves #42191 (Tested directly)
resolves #38276 (The test with a guard covers this case)
resolves #22763 (Tested directly)

PR Close #45877
@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 Jun 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant