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

atscott · AndrewKushnir · commit d8cf78ba5e6e · 2022-08-02T09:38:28.000-07:00
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
diff --git a/aio/content/guide/router-tutorial-toh.md b/aio/content/guide/router-tutorial-toh.md
@@ -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.
 
diff --git a/packages/router/src/router_preloader.ts b/packages/router/src/router_preloader.ts
@@ -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) ||
           (route.loadComponent && !route._loadedComponent)) {
         res.push(this.preloadConfig(injectorForCurrentRoute, route));
       } else if (route.children || route._loadedRoutes) {
diff --git a/packages/router/test/router_preloader.spec.ts b/packages/router/test/router_preloader.spec.ts
@@ -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}])]
@@ -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(() => {});
 
@@ -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', () => {
