Skip to content

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

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.

Sign up for GitHub

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
atscott wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions aio/content/guide/router-tutorial-toh.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2382,12 +2382,12 @@ Currently, the `AdminModule` does not preload because `CanLoad` is blocking it.

<a id="preload-canload"></a>

#### `CanLoad` blocks preload
#### `CanLoad` blocks preload of children

The `PreloadAllModules` strategy does not load feature areas protected by a [CanLoad](#can-load-guard) guard.

You added a `CanLoad` guard to the route in the `AdminModule` a few steps back to block loading of that module until the user is authorized.
That `CanLoad` guard takes precedence over the preload strategy.
That `CanLoad` guard takes precedence over the preload strategy for loading children routes.

If you want to preload a module as well as guard against unauthorized access, remove the `canLoad()` guard method and rely on the [canActivate()](#can-activate-guard) guard alone.

Expand Down
10 changes: 9 additions & 1 deletion packages/router/src/router_preloader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,15 @@ export class RouterPreloader implements OnDestroy {
const injectorForCurrentRoute = route._injector ?? injector;
const injectorForChildren = route._loadedInjector ?? injectorForCurrentRoute;

if ((route.loadChildren && !route._loadedRoutes) ||
// Note that `canLoad` is only checked as a condition that prevents `loadChildren` and not
// `loadComponent`. `canLoad` guards only block loading of child routes by design. This
// happens as a consequence of needing to descend into children for route matching immediately
// while component loading is deferred until route activation. Because `canLoad` guards can
// have side effects, we cannot execute them here so we instead skip preloading altogether
// when present. Lastly, it remains to be decided whether `canLoad` should behave this way
// at all. Code splitting and lazy loading is separate from client-side authorization checks
// and should not be used as a security measure to prevent loading of code.
if ((route.loadChildren && !route._loadedRoutes && route.canLoad === undefined) ||
Comment thread
atscott marked this conversation as resolved.
Resolved
(route.loadComponent && !route._loadedComponent)) {
res.push(this.preloadConfig(injectorForCurrentRoute, route));
} else if (route.children || route._loadedRoutes) {
Expand Down
15 changes: 13 additions & 2 deletions packages/router/test/router_preloader.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ describe('RouterPreloader', () => {
});
});

describe('should not load configurations with canLoad guard', () => {
describe('configurations with canLoad guard', () => {
@NgModule({
declarations: [LazyLoadedCmp],
imports: [RouterModule.forChild([{path: 'LoadedModule1', component: LazyLoadedCmp}])]
Expand All @@ -56,7 +56,7 @@ describe('RouterPreloader', () => {
});


it('should work',
it('should not load children',
fakeAsync(inject([RouterPreloader, Router], (preloader: RouterPreloader, router: Router) => {
preloader.preload().subscribe(() => {});

Expand All @@ -65,6 +65,17 @@ describe('RouterPreloader', () => {
const c = router.config;
expect((c[0] as any)._loadedConfig).not.toBeDefined();
})));

it('should not call the preloading method because children will not be loaded anyways',
fakeAsync(() => {
const preloader = TestBed.inject(RouterPreloader);
const preloadingStrategy = TestBed.inject(PreloadingStrategy);
spyOn(preloadingStrategy, 'preload').and.callThrough();
preloader.preload().subscribe(() => {});

tick();
expect(preloadingStrategy.preload).not.toHaveBeenCalled();
}));
});

describe('should preload configurations', () => {
Expand Down