docs(router): Deprecate canLoad guards in favor of canMatch#48180
Closed
atscott wants to merge 1 commit intoangular:mainfrom
Closed
docs(router): Deprecate canLoad guards in favor of canMatch#48180atscott wants to merge 1 commit intoangular:mainfrom
atscott wants to merge 1 commit intoangular:mainfrom
Conversation
1c92a39 to
58a246c
Compare
78922ca to
605ae27
Compare
bob-watson
suggested changes
Nov 23, 2022
Contributor
bob-watson
left a comment
There was a problem hiding this comment.
Looks good, overall.
I just had some suggestions to tighten up the language some.
As mentioned in angular#46021, `canMatch` guards can replace `canLoad`. There are slight differences between the two but the purpose of preventing user access to feature modules is still achievable. There are several reasons keeping `CanLoad` around is detrimental to the API surface: * Lazy loading should not be an architectural feature of an application. It's an optimization you do for code size. That is, there should not be an architectural feature in the router to directly specifically control whether to lazy load something or not based on conditions such as authentication. This slightly different from the `canMatch` guard: the guard controls whether you can use the route at all and as a side-effect, whether we download the code. `CanLoad` only specified whether the code should be downloaded so `canMatch` is more powerful and more appropriate. * The naming of `CanLoad` will be potentially misunderstood for the `loadComponent` feature. Because it applies to `loadChildren`, it feels reasonable to think that it will also apply to `loadComponent`. This isn’t the case: since we don't need to load the component until right before activation, we defer the loading until all guards/resolvers have run. * Unnecessary API surface bloat where two features (CanMatch and CanLoad) do essentially the same thing. This affects code size for supporting two nearly identical features as well as the learning and teaching journey for them both. * `CanLoad` guards have the downside of _only_ being run once to prevent loading child routes. Once that passes and children are loaded, the guard never runs again. As a result, developers need to always provide _both_ canLoad and a canActivate in case the answer to the guard flips back to `false`. This is not the case for `canMatch`, which will run on every navigation. DEPRECATED: CanLoad guards in the Router are deprecated. Use CanMatch instead.
f6a7f5c to
8048111
Compare
bob-watson
approved these changes
Nov 23, 2022
Contributor
bob-watson
left a comment
There was a problem hiding this comment.
LGTM
Reviewed-for: global-docs-approvers
jessicajaniuk
approved these changes
Nov 23, 2022
Contributor
jessicajaniuk
left a comment
There was a problem hiding this comment.
reviewed-for: public-api
pkozlowski-opensource
approved these changes
Nov 28, 2022
Member
pkozlowski-opensource
left a comment
There was a problem hiding this comment.
LGTM
Reviewed-for: public-api
Contributor
Author
|
This PR was merged into the repository by commit 228e992. |
|
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. |
trekladyone
pushed a commit
to trekladyone/angular
that referenced
this pull request
Feb 1, 2023
…48180) As mentioned in angular#46021, `canMatch` guards can replace `canLoad`. There are slight differences between the two but the purpose of preventing user access to feature modules is still achievable. There are several reasons keeping `CanLoad` around is detrimental to the API surface: * Lazy loading should not be an architectural feature of an application. It's an optimization you do for code size. That is, there should not be an architectural feature in the router to directly specifically control whether to lazy load something or not based on conditions such as authentication. This slightly different from the `canMatch` guard: the guard controls whether you can use the route at all and as a side-effect, whether we download the code. `CanLoad` only specified whether the code should be downloaded so `canMatch` is more powerful and more appropriate. * The naming of `CanLoad` will be potentially misunderstood for the `loadComponent` feature. Because it applies to `loadChildren`, it feels reasonable to think that it will also apply to `loadComponent`. This isn’t the case: since we don't need to load the component until right before activation, we defer the loading until all guards/resolvers have run. * Unnecessary API surface bloat where two features (CanMatch and CanLoad) do essentially the same thing. This affects code size for supporting two nearly identical features as well as the learning and teaching journey for them both. * `CanLoad` guards have the downside of _only_ being run once to prevent loading child routes. Once that passes and children are loaded, the guard never runs again. As a result, developers need to always provide _both_ canLoad and a canActivate in case the answer to the guard flips back to `false`. This is not the case for `canMatch`, which will run on every navigation. DEPRECATED: CanLoad guards in the Router are deprecated. Use CanMatch instead. PR Close angular#48180
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.
4 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.
As mentioned in #46021,
canMatchguards can replacecanLoad. There are slight differences between the two but the purpose of preventing user access to feature modules is still achievable. There are several reasons keepingCanLoadaround is detrimental to the API surface:Lazy loading should not be an architectural feature of an application. It's an optimization you do for code size. That is, there should not be an architectural feature in the router to directly specifically control whether to lazy load something or not based on conditions such as authentication. This is slightly different from the
canMatchguard: the guard controls whether you can use the route at all and as a side-effect, whether we download the code.CanLoadonly specified whether the code should be downloaded socanMatchis more powerful and more appropriate.The naming of
CanLoadwill be potentially misunderstood for theloadComponentfeature. Because it applies toloadChildren, it feels reasonable to think that it will also apply toloadComponent. This isn’t the case: since we don't need to load the component until right before activation, we defer the loading until all guards/resolvers have run.Unnecessary API surface bloat where two features (CanMatch and CanLoad) do essentially the same thing. This affects code size for supporting two nearly identical features as well as the learning and teaching journey for them both.
CanLoadguards have the downside of only being run once to preventloading child routes. Once that passes and children are loaded, the
guard never runs again. As a result, developers need to always provide
both canLoad and a canActivate in case the answer to the guard flips
back to
false. This is not the case forcanMatch, which will runon every navigation.