fix(router): fix load interaction of navigation and preload strategies (

#40389)

Fix router to ensure that a route module is only loaded once especially
in relation to the use of preload strategies with delayed or partial
loading.

Add test to check the interaction of PreloadingStrategy and normal
router navigation under differing scenarios.
Checking:
 * Prevention of duplicate loading of modules.
   related to #26557
 * Prevention of duplicate RouteConfigLoad(Start|End) events
   related to #22842
 * Ensuring preload strategy remains active for submodules if needed
   The selected preload strategy should still decide when to load submodules
 * Possibility of memory leak with unfinished preload subscription
   related to #26557
 * Ensure that the stored loader promise is cleared so that subsequent
   load will try the fetch again.
 * Add error handle error from loadChildren
 * Ensure we handle error from with NgModule create

Fixes #26557 #22842 #26557

PR Close #40389

goldens/size-tracking/aio-payloads.json

@@ -12,7 +12,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 3033,
-        "main-es2015": 447514,
+        "main-es2015": 448036,
         "polyfills-es2015": 52493
       }
     }
@@ -21,9 +21,9 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 3153,
-        "main-es2015": 432078,
+        "main-es2015": 432647,
         "polyfills-es2015": 52493
       }
     }
   }
-}
+}

goldens/size-tracking/integration-payloads.json

@@ -39,7 +39,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 2285,
-        "main-es2015": 241202,
+        "main-es2015": 241843,
         "polyfills-es2015": 36709,
         "5-es2015": 745
       }
@@ -66,4 +66,4 @@
       }
     }
   }
-}
+}

packages/router/src/apply_redirects.ts

@@ -280,11 +280,12 @@ class ApplyRedirects {
       segments: UrlSegment[], outlet: string): Observable<UrlSegmentGroup> {
     if (route.path === '**') {
       if (route.loadChildren) {
-        return this.configLoader.load(ngModule.injector, route)
-            .pipe(map((cfg: LoadedRouterConfig) => {
-              route._loadedConfig = cfg;
-              return new UrlSegmentGroup(segments, {});
-            }));
+        const loaded$ = route._loadedConfig ? of(route._loadedConfig) :
+                                              this.configLoader.load(ngModule.injector, route);
+        return loaded$.pipe(map((cfg: LoadedRouterConfig) => {
+          route._loadedConfig = cfg;
+          return new UrlSegmentGroup(segments, {});
+        }));
       }
 
       return of(new UrlSegmentGroup(segments, {}));

packages/router/src/config.ts

@@ -483,6 +483,11 @@ export interface Route {
    * @internal
    */
   _loadedConfig?: LoadedRouterConfig;
+  /**
+   * Filled for routes with `loadChildren` during load
+   * @internal
+   */
+  _loader$?: Observable<LoadedRouterConfig>;
 }
 
 export class LoadedRouterConfig {

packages/router/src/router_config_loader.ts

@@ -7,8 +7,8 @@
  */
 
 import {Compiler, InjectFlags, InjectionToken, Injector, NgModuleFactory, NgModuleFactoryLoader} from '@angular/core';
-import {from, Observable, of} from 'rxjs';
-import {map, mergeMap} from 'rxjs/operators';
+import {ConnectableObservable, from, Observable, of, Subject} from 'rxjs';
+import {catchError, map, mergeMap, refCount, tap} from 'rxjs/operators';
 
 import {LoadChildren, LoadedRouterConfig, Route} from './config';
 import {flatten, wrapIntoObservable} from './utils/collection';
@@ -28,27 +28,39 @@ export class RouterConfigLoader {
       private onLoadEndListener?: (r: Route) => void) {}
 
   load(parentInjector: Injector, route: Route): Observable<LoadedRouterConfig> {
+    if (route._loader$) {
+      return route._loader$;
+    }
+
     if (this.onLoadStartListener) {
       this.onLoadStartListener(route);
     }
-
     const moduleFactory$ = this.loadModuleFactory(route.loadChildren!);
-
-    return moduleFactory$.pipe(map((factory: NgModuleFactory<any>) => {
-      if (this.onLoadEndListener) {
-        this.onLoadEndListener(route);
-      }
-
-      const module = factory.create(parentInjector);
-
-      // When loading a module that doesn't provide `RouterModule.forChild()` preloader will get
-      // stuck in an infinite loop. The child module's Injector will look to its parent `Injector`
-      // when it doesn't find any ROUTES so it will return routes for it's parent module instead.
-      return new LoadedRouterConfig(
-          flatten(module.injector.get(ROUTES, undefined, InjectFlags.Self | InjectFlags.Optional))
-              .map(standardizeConfig),
-          module);
-    }));
+    const loadRunner = moduleFactory$.pipe(
+        map((factory: NgModuleFactory<any>) => {
+          if (this.onLoadEndListener) {
+            this.onLoadEndListener(route);
+          }
+          const module = factory.create(parentInjector);
+          // When loading a module that doesn't provide `RouterModule.forChild()` preloader
+          // will get stuck in an infinite loop. The child module's Injector will look to
+          // its parent `Injector` when it doesn't find any ROUTES so it will return routes
+          // for it's parent module instead.
+          return new LoadedRouterConfig(
+              flatten(
+                  module.injector.get(ROUTES, undefined, InjectFlags.Self | InjectFlags.Optional))
+                  .map(standardizeConfig),
+              module);
+        }),
+        catchError((err) => {
+          route._loader$ = undefined;
+          throw err;
+        }),
+    );
+    // Use custom ConnectableObservable as share in runners pipe increasing the bundle size too much
+    route._loader$ = new ConnectableObservable(loadRunner, () => new Subject<LoadedRouterConfig>())
+                         .pipe(refCount());
+    return route._loader$;
   }
 
   private loadModuleFactory(loadChildren: LoadChildren): Observable<NgModuleFactory<any>> {

packages/router/src/router_preloader.ts

@@ -126,7 +126,8 @@ export class RouterPreloader implements OnDestroy {
 
   private preloadConfig(ngModule: NgModuleRef<any>, route: Route): Observable<void> {
     return this.preloadingStrategy.preload(route, () => {
-      const loaded$ = this.loader.load(ngModule.injector, route);
+      const loaded$ = route._loadedConfig ? of(route._loadedConfig) :
+                                            this.loader.load(ngModule.injector, route);
       return loaded$.pipe(mergeMap((config: LoadedRouterConfig) => {
         route._loadedConfig = config;
         return this.processRoutes(config.module, config.routes);

packages/router/test/apply_redirects.spec.ts

@@ -514,6 +514,45 @@ describe('applyRedirects', () => {
          expect(loaded).toEqual(['root', 'aux']);
        }));
 
+    it('should not try to load any matching configuration if previous load completed',
+       fakeAsync(() => {
+         const loadedConfig =
+             new LoadedRouterConfig([{path: 'a', component: ComponentA}], testModule);
+         let loadCalls = 0;
+         let loaded: string[] = [];
+         const loader = {
+           load: (injector: any, p: Route) => {
+             loadCalls++;
+             return of(loadedConfig)
+                 .pipe(
+                     delay(100 * loadCalls),
+                     tap(() => loaded.push(p.loadChildren! as string)),
+                 );
+           }
+         };
+
+         const config: Routes = [
+           {path: '**', loadChildren: 'children'},
+         ];
+
+         applyRedirects(testModule.injector, <any>loader, serializer, tree('xyz/a'), config)
+             .subscribe();
+         expect(loadCalls).toBe(1);
+         tick(50);
+         expect(loaded).toEqual([]);
+         applyRedirects(testModule.injector, <any>loader, serializer, tree('xyz/b'), config)
+             .subscribe();
+         tick(50);
+         expect(loaded).toEqual(['children']);
+         expect(loadCalls).toBe(2);
+         tick(200);
+         applyRedirects(testModule.injector, <any>loader, serializer, tree('xyz/c'), config)
+             .subscribe();
+         tick(50);
+         expect(loadCalls).toBe(2);
+         tick(300);
+       }));
+
     it('loads only the first match when two Routes with the same outlet have the same path', () => {
       const loadedConfig = new LoadedRouterConfig([{path: '', component: ComponentA}], testModule);
       let loadCalls = 0;