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
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions aio/content/guide/router-tutorial-toh.md
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
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) ||
atscott marked this conversation as resolved.
Show resolved Hide 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
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