Revert "fix(router): fix load interaction of navigation and preload strategies (#40389)" (#40806)

This reverts commit e9a19a6152.

PR Close #40806

goldens/size-tracking/aio-payloads.json

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

goldens/size-tracking/integration-payloads.json

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

packages/router/src/apply_redirects.ts

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

packages/router/src/config.ts

@@ -483,11 +483,6 @@ 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 {ConnectableObservable, from, Observable, of, Subject} from 'rxjs';
+import {from, Observable, of} from 'rxjs';
-import {catchError, map, mergeMap, refCount, tap} from 'rxjs/operators';
+import {map, mergeMap} from 'rxjs/operators';
 
 import {LoadChildren, LoadedRouterConfig, Route} from './config';
 import {flatten, wrapIntoObservable} from './utils/collection';
@@ -28,39 +28,27 @@ 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!);
-    const loadRunner = moduleFactory$.pipe(
+
-        map((factory: NgModuleFactory<any>) => {
+    return moduleFactory$.pipe(map((factory: NgModuleFactory<any>) => {
-          if (this.onLoadEndListener) {
+      if (this.onLoadEndListener) {
-            this.onLoadEndListener(route);
+        this.onLoadEndListener(route);
-          }
+      }
-          const module = factory.create(parentInjector);
+
-          // When loading a module that doesn't provide `RouterModule.forChild()` preloader
+      const module = factory.create(parentInjector);
-          // 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
+      // When loading a module that doesn't provide `RouterModule.forChild()` preloader will get
-          // for it's parent module instead.
+      // stuck in an infinite loop. The child module's Injector will look to its parent `Injector`
-          return new LoadedRouterConfig(
+      // when it doesn't find any ROUTES so it will return routes for it's parent module instead.
-              flatten(
+      return new LoadedRouterConfig(
-                  module.injector.get(ROUTES, undefined, InjectFlags.Self | InjectFlags.Optional))
+          flatten(module.injector.get(ROUTES, undefined, InjectFlags.Self | InjectFlags.Optional))
-                  .map(standardizeConfig),
+              .map(standardizeConfig),
-              module);
+          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,8 +126,7 @@ export class RouterPreloader implements OnDestroy {
 
   private preloadConfig(ngModule: NgModuleRef<any>, route: Route): Observable<void> {
     return this.preloadingStrategy.preload(route, () => {
-      const loaded$ = route._loadedConfig ? of(route._loadedConfig) :
+      const loaded$ = this.loader.load(ngModule.injector, route);
-                                            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,45 +514,6 @@ 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;

packages/router/test/router_preloader.spec.ts

@@ -6,18 +6,14 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Compiler, Component, Injector, NgModule, NgModuleFactory, NgModuleFactoryLoader, NgModuleRef, Type} from '@angular/core';
+import {Compiler, Component, NgModule, NgModuleFactoryLoader, NgModuleRef} from '@angular/core';
-import {resolveComponentResources} from '@angular/core/src/metadata/resource_loading';
 import {fakeAsync, inject, TestBed, tick} from '@angular/core/testing';
 import {PreloadAllModules, PreloadingStrategy, RouterPreloader} from '@angular/router';
-import {BehaviorSubject, Observable, of, throwError} from 'rxjs';
-import {catchError, delay, filter, switchMap, take} from 'rxjs/operators';
 
 import {Route, RouteConfigLoadEnd, RouteConfigLoadStart, Router, RouterModule} from '../index';
 import {LoadedRouterConfig} from '../src/config';
 import {RouterTestingModule, SpyNgModuleFactoryLoader} from '../testing';
 
-
 describe('RouterPreloader', () => {
   @Component({template: ''})
   class LazyLoadedCmp {
@@ -200,311 +196,6 @@ describe('RouterPreloader', () => {
            })));
   });
 
-  describe('should support preloading strategies', () => {
-    let delayLoadUnPaused: BehaviorSubject<string[]>;
-    let delayLoadObserver$: Observable<string[]>;
-    let events: Array<RouteConfigLoadStart|RouteConfigLoadEnd>;
-
-    const subLoadChildrenSpy = jasmine.createSpy('submodule');
-    const lazyLoadChildrenSpy = jasmine.createSpy('lazymodule');
-
-    const mockPreloaderFactory = (): PreloadingStrategy => {
-      class DelayedPreLoad implements PreloadingStrategy {
-        preload(route: Route, fn: () => Observable<any>): Observable<any> {
-          const routeName =
-              route.loadChildren ? (route.loadChildren as jasmine.Spy).and.identity : 'noChildren';
-          return delayLoadObserver$.pipe(
-              filter(unpauseList => unpauseList.indexOf(routeName) !== -1),
-              take(1),
-              switchMap(() => {
-                return fn().pipe(catchError(() => of(null)));
-              }),
-          );
-        }
-      }
-      return new DelayedPreLoad();
-    };
-
-    @NgModule({
-      declarations: [LazyLoadedCmp],
-    })
-    class SharedModule {
-    }
-
-    @NgModule({
-      imports: [
-        SharedModule, RouterModule.forChild([
-          {path: 'LoadedModule1', component: LazyLoadedCmp},
-          {path: 'sub', loadChildren: subLoadChildrenSpy}
-        ])
-      ]
-    })
-    class LoadedModule1 {
-    }
-
-    @NgModule({
-      imports:
-          [SharedModule, RouterModule.forChild([{path: 'LoadedModule2', component: LazyLoadedCmp}])]
-    })
-    class LoadedModule2 {
-    }
-
-    beforeEach(() => {
-      delayLoadUnPaused = new BehaviorSubject<string[]>([]);
-      delayLoadObserver$ = delayLoadUnPaused.asObservable();
-      subLoadChildrenSpy.calls.reset();
-      lazyLoadChildrenSpy.calls.reset();
-      TestBed.configureTestingModule({
-        imports:
-            [RouterTestingModule.withRoutes([{path: 'lazy', loadChildren: lazyLoadChildrenSpy}])],
-        providers: [{provide: PreloadingStrategy, useFactory: mockPreloaderFactory}]
-      });
-      events = [];
-    });
-
-    it('without reloading loaded modules', fakeAsync(() => {
-         const preloader = TestBed.inject(RouterPreloader);
-         const router = TestBed.inject(Router);
-         router.events.subscribe(e => {
-           if (e instanceof RouteConfigLoadEnd || e instanceof RouteConfigLoadStart) {
-             events.push(e);
-           }
-         });
-         lazyLoadChildrenSpy.and.returnValue(of(LoadedModule1));
-
-         // App start activation of preloader
-         preloader.preload().subscribe((x) => {});
-         tick();
-         expect(lazyLoadChildrenSpy).toHaveBeenCalledTimes(0);
-
-         // Initial navigation cause route load
-         router.navigateByUrl('/lazy/LoadedModule1');
-         tick();
-         expect(lazyLoadChildrenSpy).toHaveBeenCalledTimes(1);
-
-         // Secondary load or navigation should use same loaded object (
-         //   ie this is a noop as the module should already be loaded)
-         delayLoadUnPaused.next(['lazymodule']);
-         tick();
-         expect(lazyLoadChildrenSpy).toHaveBeenCalledTimes(1);
-         expect(subLoadChildrenSpy).toHaveBeenCalledTimes(0);
-         expect(events.map(e => e.toString())).toEqual([
-           'RouteConfigLoadStart(path: lazy)', 'RouteConfigLoadEnd(path: lazy)'
-         ]);
-       }));
-
-    it('and cope with the loader throwing exceptions during module load but allow retry',
-       fakeAsync(() => {
-         const preloader = TestBed.inject(RouterPreloader);
-         const router = TestBed.inject(Router);
-         router.events.subscribe(e => {
-           if (e instanceof RouteConfigLoadEnd || e instanceof RouteConfigLoadStart) {
-             events.push(e);
-           }
-         });
-
-         lazyLoadChildrenSpy.and.returnValue(
-             throwError('Error: Fake module load error (expectedreload)'));
-         preloader.preload().subscribe((x) => {});
-         tick();
-         expect(lazyLoadChildrenSpy).toHaveBeenCalledTimes(0);
-
-         delayLoadUnPaused.next(['lazymodule']);
-         tick();
-         expect(lazyLoadChildrenSpy).toHaveBeenCalledTimes(1);
-
-         lazyLoadChildrenSpy.and.returnValue(of(LoadedModule1));
-         router.navigateByUrl('/lazy/LoadedModule1').catch(() => {
-           fail('navigation should not throw');
-         });
-         tick();
-         expect(lazyLoadChildrenSpy).toHaveBeenCalledTimes(2);
-         expect(subLoadChildrenSpy).toHaveBeenCalledTimes(0);
-
-         expect(events.map(e => e.toString())).toEqual([
-           'RouteConfigLoadStart(path: lazy)', 'RouteConfigLoadStart(path: lazy)',
-           'RouteConfigLoadEnd(path: lazy)'
-         ]);
-       }));
-
-    it('and cope with the loader throwing exceptions but allow retry', fakeAsync(() => {
-         const preloader = TestBed.inject(RouterPreloader);
-         const router = TestBed.inject(Router);
-         router.events.subscribe(e => {
-           if (e instanceof RouteConfigLoadEnd || e instanceof RouteConfigLoadStart) {
-             events.push(e);
-           }
-         });
-
-         lazyLoadChildrenSpy.and.returnValue(
-             throwError('Error: Fake module load error (expectedreload)'));
-         preloader.preload().subscribe((x) => {});
-         tick();
-         expect(lazyLoadChildrenSpy).toHaveBeenCalledTimes(0);
-
-         router.navigateByUrl('/lazy/LoadedModule1').catch((reason) => {
-           expect(reason).toEqual('Error: Fake module load error (expectedreload)');
-         });
-         tick();
-
-         expect(lazyLoadChildrenSpy).toHaveBeenCalledTimes(1);
-         lazyLoadChildrenSpy.and.returnValue(of(LoadedModule1));
-         router.navigateByUrl('/lazy/LoadedModule1').catch(() => {
-           fail('navigation should not throw');
-         });
-         tick();
-
-         expect(lazyLoadChildrenSpy).toHaveBeenCalledTimes(2);
-         expect(subLoadChildrenSpy).toHaveBeenCalledTimes(0);
-         expect(events.map(e => e.toString())).toEqual([
-           'RouteConfigLoadStart(path: lazy)', 'RouteConfigLoadStart(path: lazy)',
-           'RouteConfigLoadEnd(path: lazy)'
-         ]);
-       }));
-
-    it('without autoloading loading submodules', fakeAsync(() => {
-         const preloader = TestBed.inject(RouterPreloader);
-         const router = TestBed.inject(Router);
-         router.events.subscribe(e => {
-           if (e instanceof RouteConfigLoadEnd || e instanceof RouteConfigLoadStart) {
-             events.push(e);
-           }
-         });
-
-         lazyLoadChildrenSpy.and.returnValue(of(LoadedModule1));
-         subLoadChildrenSpy.and.returnValue(of(LoadedModule2));
-
-         preloader.preload().subscribe((x) => {});
-         tick();
-         router.navigateByUrl('/lazy/LoadedModule1');
-         tick();
-         expect(lazyLoadChildrenSpy).toHaveBeenCalledTimes(1);
-         expect(subLoadChildrenSpy).toHaveBeenCalledTimes(0);
-         expect(events.map(e => e.toString())).toEqual([
-           'RouteConfigLoadStart(path: lazy)', 'RouteConfigLoadEnd(path: lazy)'
-         ]);
-
-         // Release submodule to check it does in fact load
-         delayLoadUnPaused.next(['lazymodule', 'submodule']);
-         tick();
-         expect(lazyLoadChildrenSpy).toHaveBeenCalledTimes(1);
-         expect(subLoadChildrenSpy).toHaveBeenCalledTimes(1);
-         expect(events.map(e => e.toString())).toEqual([
-           'RouteConfigLoadStart(path: lazy)', 'RouteConfigLoadEnd(path: lazy)',
-           'RouteConfigLoadStart(path: sub)', 'RouteConfigLoadEnd(path: sub)'
-         ]);
-       }));
-
-    it('and close the preload obsservable ', fakeAsync(() => {
-         const preloader = TestBed.inject(RouterPreloader);
-         const router = TestBed.inject(Router);
-         router.events.subscribe(e => {
-           if (e instanceof RouteConfigLoadEnd || e instanceof RouteConfigLoadStart) {
-             events.push(e);
-           }
-         });
-
-         lazyLoadChildrenSpy.and.returnValue(of(LoadedModule1));
-         subLoadChildrenSpy.and.returnValue(of(LoadedModule2));
-         const preloadSubscription = preloader.preload().subscribe((x) => {});
-
-         router.navigateByUrl('/lazy/LoadedModule1');
-         tick();
-         delayLoadUnPaused.next(['lazymodule', 'submodule']);
-         tick();
-
-         expect(lazyLoadChildrenSpy).toHaveBeenCalledTimes(1);
-         expect(subLoadChildrenSpy).toHaveBeenCalledTimes(1);
-         expect(preloadSubscription.closed).toBeTruthy();
-       }));
-
-    it('with overlapping loads from navigation and the preloader', fakeAsync(() => {
-         const preloader = TestBed.inject(RouterPreloader);
-         const router = TestBed.inject(Router);
-         router.events.subscribe(e => {
-           if (e instanceof RouteConfigLoadEnd || e instanceof RouteConfigLoadStart) {
-             events.push(e);
-           }
-         });
-
-         lazyLoadChildrenSpy.and.returnValue(of(LoadedModule1));
-         subLoadChildrenSpy.and.returnValue(of(LoadedModule2).pipe(delay(5)));
-         preloader.preload().subscribe((x) => {});
-         tick();
-
-         // Load the out modules at start of test and ensure it and only
-         // it is loaded
-         delayLoadUnPaused.next(['lazymodule']);
-         tick();
-         expect(lazyLoadChildrenSpy).toHaveBeenCalledTimes(1);
-         expect(events.map(e => e.toString())).toEqual([
-           'RouteConfigLoadStart(path: lazy)',
-           'RouteConfigLoadEnd(path: lazy)',
-         ]);
-
-         // Cause the load from router to start (has 5 tick delay)
-         router.navigateByUrl('/lazy/sub/LoadedModule2');
-         tick();  // T1
-         // Cause the load from preloader to start
-         delayLoadUnPaused.next(['lazymodule', 'submodule']);
-         tick();  // T2
-
-         expect(lazyLoadChildrenSpy).toHaveBeenCalledTimes(1);
-         expect(subLoadChildrenSpy).toHaveBeenCalledTimes(1);
-         tick(5);  // T2 to T7 enough time for mutiple loads to finish
-
-         expect(subLoadChildrenSpy).toHaveBeenCalledTimes(1);
-         expect(events.map(e => e.toString())).toEqual([
-           'RouteConfigLoadStart(path: lazy)', 'RouteConfigLoadEnd(path: lazy)',
-           'RouteConfigLoadStart(path: sub)', 'RouteConfigLoadEnd(path: sub)'
-         ]);
-       }));
-
-    it('cope with factory fail from broken modules', fakeAsync(() => {
-         const preloader = TestBed.inject(RouterPreloader);
-         const router = TestBed.inject(Router);
-         router.events.subscribe(e => {
-           if (e instanceof RouteConfigLoadEnd || e instanceof RouteConfigLoadStart) {
-             events.push(e);
-           }
-         });
-
-         class BrokenModuleFactory extends NgModuleFactory<any> {
-           moduleType: Type<any> = LoadedModule1;
-           constructor() {
-             super();
-           }
-           create(_parentInjector: Injector|null): NgModuleRef<any> {
-             throw 'Error: Broken module';
-           }
-         }
-
-         lazyLoadChildrenSpy.and.returnValue(of(new BrokenModuleFactory()));
-         preloader.preload().subscribe((x) => {});
-         tick();
-         expect(lazyLoadChildrenSpy).toHaveBeenCalledTimes(0);
-
-         router.navigateByUrl('/lazy/LoadedModule1').catch((reason) => {
-           expect(reason).toEqual('Error: Broken module');
-         });
-         tick();
-
-         expect(lazyLoadChildrenSpy).toHaveBeenCalledTimes(1);
-         lazyLoadChildrenSpy.and.returnValue(of(LoadedModule1));
-         router.navigateByUrl('/lazy/LoadedModule1').catch(() => {
-           fail('navigation should not throw');
-         });
-         tick();
-
-         expect(lazyLoadChildrenSpy).toHaveBeenCalledTimes(2);
-         expect(subLoadChildrenSpy).toHaveBeenCalledTimes(0);
-         expect(events.map(e => e.toString())).toEqual([
-           'RouteConfigLoadStart(path: lazy)', 'RouteConfigLoadEnd(path: lazy)',
-           'RouteConfigLoadStart(path: lazy)', 'RouteConfigLoadEnd(path: lazy)'
-         ]);
-       }));
-  });
-
   describe('should ignore errors', () => {
     @NgModule({
       declarations: [LazyLoadedCmp],