Skip to content

Commit

Permalink
Revert "fix(router): fix load interaction of navigation and preload s…
Browse files Browse the repository at this point in the history
…trategies (#40389)" (#40806)

This reverts commit e9a19a6.

PR Close #40806
  • Loading branch information
josephperrott committed Feb 11, 2021
1 parent f72626d commit 267c566
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 398 deletions.
6 changes: 3 additions & 3 deletions goldens/size-tracking/aio-payloads.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 3033,
"main-es2015": 448036,
"main-es2015": 447514,
"polyfills-es2015": 52493
}
}
Expand All @@ -21,9 +21,9 @@
"master": {
"uncompressed": {
"runtime-es2015": 3153,
"main-es2015": 432647,
"main-es2015": 432078,
"polyfills-es2015": 52493
}
}
}
}
}
4 changes: 2 additions & 2 deletions goldens/size-tracking/integration-payloads.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 2285,
"main-es2015": 241843,
"main-es2015": 241202,
"polyfills-es2015": 36709,
"5-es2015": 745
}
Expand All @@ -66,4 +66,4 @@
}
}
}
}
}
11 changes: 5 additions & 6 deletions packages/router/src/apply_redirects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) :
this.configLoader.load(ngModule.injector, route);
return loaded$.pipe(map((cfg: LoadedRouterConfig) => {
route._loadedConfig = cfg;
return new UrlSegmentGroup(segments, {});
}));
return this.configLoader.load(ngModule.injector, route)
.pipe(map((cfg: LoadedRouterConfig) => {
route._loadedConfig = cfg;
return new UrlSegmentGroup(segments, {});
}));
}

return of(new UrlSegmentGroup(segments, {}));
Expand Down
5 changes: 0 additions & 5 deletions packages/router/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -483,11 +483,6 @@ export interface Route {
* @internal
*/
_loadedConfig?: LoadedRouterConfig;
/**
* Filled for routes with `loadChildren` during load
* @internal
*/
_loader$?: Observable<LoadedRouterConfig>;
}

export class LoadedRouterConfig {
Expand Down
50 changes: 19 additions & 31 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 {ConnectableObservable, from, Observable, of, Subject} from 'rxjs';
import {catchError, map, mergeMap, refCount, tap} from 'rxjs/operators';
import {from, Observable, of} from 'rxjs';
import {map, mergeMap} from 'rxjs/operators';

import {LoadChildren, LoadedRouterConfig, Route} from './config';
import {flatten, wrapIntoObservable} from './utils/collection';
Expand All @@ -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>) => {
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$;

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);
}));
}

private loadModuleFactory(loadChildren: LoadChildren): Observable<NgModuleFactory<any>> {
Expand Down
3 changes: 1 addition & 2 deletions packages/router/src/router_preloader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) :
this.loader.load(ngModule.injector, route);
const loaded$ = this.loader.load(ngModule.injector, route);
return loaded$.pipe(mergeMap((config: LoadedRouterConfig) => {
route._loadedConfig = config;
return this.processRoutes(config.module, config.routes);
Expand Down
39 changes: 0 additions & 39 deletions packages/router/test/apply_redirects.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 267c566

Please sign in to comment.