From d8cf78ba5e6e5bf48e6c3f2b5f2b590fa4598bb3 Mon Sep 17 00:00:00 2001
From: Andrew Scott <atscott@google.com>
Date: Mon, 1 Aug 2022 09:25:35 -0700
Subject: [PATCH] fix(router): Do not call preload method when not necessary
 (#47007)

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
---
 aio/content/guide/router-tutorial-toh.md      |  4 ++--
 packages/router/src/router_preloader.ts       | 10 +++++++++-
 packages/router/test/router_preloader.spec.ts | 15 +++++++++++++--
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/aio/content/guide/router-tutorial-toh.md b/aio/content/guide/router-tutorial-toh.md
index 698a88e0eb6..29fc7f2e926 100644
--- 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
index 749dc32ccde..272e77a1e32 100644
--- 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
index 9e4d838ca9f..e735a188f20 100644
--- 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', () => {