Skip to content
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

fix(router): Do not call preload method when not necessary #47007

Closed
wants to merge 1 commit into from

Conversation

atscott
Copy link
Contributor

@atscott atscott commented Aug 1, 2022

In Angular 14, we introduced the loadComponent API for a Route to
allow lazy loading of a routed component in addition to the existing
loadChildren which allows lazy loading of child routes. As a result,
the preload method of the PreloadingStrategy needs to sometimes be
called even when there is a canLoad guard on the Route. CanLoad
guards block loading of child routes but do not block loading of the
component.

This change updates the conditional checks in the internal preloader to
skip calling the PreloadingStrategy.preload when there is only a
loadChildren callback with a canLoad guard an no loadComponent.
In this case, the callback passed to the preload method is already
effectively a no-op so it's not necessary to call it at all.

resolves #47003

@atscott atscott added area: router target: patch This PR is targeted for the next patch release labels Aug 1, 2022
@ngbot ngbot bot modified the milestone: Backlog Aug 1, 2022
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

👍

packages/router/src/router_preloader.ts Show resolved Hide resolved
In Angular 14, we introduced the `loadComponent` API for a `Route` to
allow lazy loading of a routed component in addition to the existing
`loadChildren` which allows lazy loading of child routes. As a result,
the `preload` method of the `PreloadingStrategy` needs to sometimes be
called even when there is a `canLoad` guard on the `Route`. `CanLoad`
guards block loading of child routes but _do not_ block loading of the
component.

This change updates the conditional checks in the internal preloader to
skip calling the `PreloadingStrategy.preload` when there is only a
`loadChildren` callback with a `canLoad` guard an no `loadComponent`.
In this case, the callback passed to the `preload` method is already
effectively a no-op so it's not necessary to call it at all.

resolves angular#47003
@atscott atscott added the action: merge The PR is ready for merge by the caretaker label Aug 1, 2022
@ngbot
Copy link

ngbot bot commented Aug 1, 2022

I see that you just added the action: merge label, but the following checks are still failing:
    failure status "google3" is failing

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken main, please try rebasing to main and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

@AndrewKushnir
Copy link
Contributor

This PR was merged into the repository by commit d8cf78b.

AndrewKushnir pushed a commit that referenced this pull request Aug 2, 2022
In Angular 14, we introduced the `loadComponent` API for a `Route` to
allow lazy loading of a routed component in addition to the existing
`loadChildren` which allows lazy loading of child routes. As a result,
the `preload` method of the `PreloadingStrategy` needs to sometimes be
called even when there is a `canLoad` guard on the `Route`. `CanLoad`
guards block loading of child routes but _do not_ block loading of the
component.

This change updates the conditional checks in the internal preloader to
skip calling the `PreloadingStrategy.preload` when there is only a
`loadChildren` callback with a `canLoad` guard an no `loadComponent`.
In this case, the callback passed to the `preload` method is already
effectively a no-op so it's not necessary to call it at all.

resolves #47003

PR Close #47007
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Aug 9, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@angular/animations](https://github.com/angular/angular) | dependencies | patch | [`14.1.0` -> `14.1.1`](https://renovatebot.com/diffs/npm/@angular%2fanimations/14.1.0/14.1.1) |
| [@angular/common](https://github.com/angular/angular) | dependencies | patch | [`14.1.0` -> `14.1.1`](https://renovatebot.com/diffs/npm/@angular%2fcommon/14.1.0/14.1.1) |
| [@angular/compiler](https://github.com/angular/angular) | dependencies | patch | [`14.1.0` -> `14.1.1`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/14.1.0/14.1.1) |
| [@angular/compiler-cli](https://github.com/angular/angular) | devDependencies | patch | [`14.1.0` -> `14.1.1`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/14.1.0/14.1.1) |
| [@angular/core](https://github.com/angular/angular) | dependencies | patch | [`14.1.0` -> `14.1.1`](https://renovatebot.com/diffs/npm/@angular%2fcore/14.1.0/14.1.1) |
| [@angular/forms](https://github.com/angular/angular) | dependencies | patch | [`14.1.0` -> `14.1.1`](https://renovatebot.com/diffs/npm/@angular%2fforms/14.1.0/14.1.1) |
| [@angular/platform-browser](https://github.com/angular/angular) | dependencies | patch | [`14.1.0` -> `14.1.1`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/14.1.0/14.1.1) |
| [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | patch | [`14.1.0` -> `14.1.1`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/14.1.0/14.1.1) |

---

### Release Notes

<details>
<summary>angular/angular</summary>

### [`v14.1.1`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#&#8203;1411-2022-08-03)

[Compare Source](angular/angular@14.1.0...14.1.1)

##### core

| Commit | Type | Description |
| -- | -- | -- |
| [3606917732](angular/angular@3606917) | fix | improve the missing control flow directive message ([#&#8203;46903](angular/angular#46903)) |

##### router

| Commit | Type | Description |
| -- | -- | -- |
| [79825d3f10](angular/angular@79825d3) | fix | Do not call preload method when not necessary ([#&#8203;47007](angular/angular#47007)) |
| [05f3f7445a](angular/angular@05f3f74) | fix | Use correct return type for provideRoutes function ([#&#8203;46941](angular/angular#46941)) |

#### Special Thanks

Alan Agius, Andrew Kushnir, Andrew Quinn, Andrew Scott, Aristeidis Bampakos, Asaf M, Bob Watson, Cédric Exbrayat, Durairaj Subramaniam, George Kalpakas, Ivaylo Kirov, J Rob Gant, Kristiyan Kostadinov, Marek Hám, Paul Gschwendtner, Roman Matusevich and Simona Cotin

<!-- CHANGELOG SPLIT MARKER -->

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4xNDEuMCIsInVwZGF0ZWRJblZlciI6IjMyLjE0MS4wIn0=-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1490
Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
@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 Sep 2, 2022
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 area: router target: patch This PR is targeted for the next patch release
Projects
None yet
2 participants