Skip to content

docs(router): Deprecate canLoad guards in favor of canMatch #48180

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

Closed
wants to merge 1 commit into from

Conversation

atscott
Copy link
Contributor

@atscott atscott commented Nov 22, 2022

As mentioned in #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 is 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.

@atscott atscott added the target: minor This PR is targeted for the next minor release label Nov 22, 2022
@angular-robot angular-robot bot added detected: deprecation PR contains a commit with a deprecation area: docs labels Nov 22, 2022
@ngbot ngbot bot added this to the Backlog milestone Nov 22, 2022
@mgechev mgechev requested a review from bob-watson November 22, 2022 20:45
@atscott atscott force-pushed the deprecateCanLoad branch 7 times, most recently from 78922ca to 605ae27 Compare November 22, 2022 22:36
Copy link
Contributor

@bob-watson bob-watson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Copy link
Contributor

@bob-watson bob-watson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Reviewed-for: global-docs-approvers

Copy link
Contributor

@jessicajaniuk jessicajaniuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed-for: public-api

Copy link
Member

@pkozlowski-opensource pkozlowski-opensource left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Reviewed-for: public-api

@atscott atscott added the action: merge The PR is ready for merge by the caretaker label Nov 28, 2022
@atscott atscott removed the request for review from alxhub November 28, 2022 17:03
@atscott
Copy link
Contributor Author

atscott commented Nov 28, 2022

This PR was merged into the repository by commit 228e992.

@atscott atscott closed this in 228e992 Nov 28, 2022
@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 Dec 29, 2022
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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker detected: deprecation PR contains a commit with a deprecation target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants