Skip to content

Commit

Permalink
fix(router): fix load interaction of navigation and preload strategies
Browse files Browse the repository at this point in the history
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 angular#26557
 * Prevention of duplicate RouteConfigLoad(Start|End) events
   related to angular#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 angular#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 angular#26557 angular#22842 angular#26557
  • Loading branch information
pgammans committed Jan 28, 2021
1 parent baadd10 commit 6ac2011
Show file tree
Hide file tree
Showing 4 changed files with 350 additions and 21 deletions.
5 changes: 5 additions & 0 deletions packages/router/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,11 @@ export interface Route {
* @internal
*/
_loadedConfig?: LoadedRouterConfig;
/**
* Filled for routes with `loadChildren` during load
* @internal
*/
_loader$?: Observable<LoadedRouterConfig>;
}

export class LoadedRouterConfig {
Expand Down
52 changes: 33 additions & 19 deletions packages/router/src/router_config_loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -28,27 +28,41 @@ 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);
}));
// Use custom ConnectableObservable as share increasing the bundle size too much
route._loader$ =
new ConnectableObservable(
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;
}),
),
() => new Subject<LoadedRouterConfig>())
.pipe(refCount());
return route._loader$;
}

private loadModuleFactory(loadChildren: LoadChildren): Observable<NgModuleFactory<any>> {
Expand Down
3 changes: 2 additions & 1 deletion packages/router/src/router_preloader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading

0 comments on commit 6ac2011

Please sign in to comment.