Skip to content

Commit 926ffcd

Browse files
atscottatscott
committed
fix(router): support lazy loading for empty path named outlets (#38379)
In general, the router only matches and loads a single Route config tree. However, named outlets with empty paths are a special case where the router can and should actually match two different `Route`s and ensure that the modules are loaded for each match. This change updates the "ApplyRedirects" stage to ensure that named outlets with empty paths finish loading their configs before proceeding to the next stage in the routing pipe. This is necessary because if the named outlet has `loadChildren` but the associated lazy config is not loaded before following stages attempt to match and activate relevant `Route`s, an error will occur. fixes #12842 PR Close #38379
1 parent 97475d7 commit 926ffcd

4 files changed

Lines changed: 206 additions & 31 deletions

File tree

‎goldens/size-tracking/integration-payloads.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
"master": {
4040
"uncompressed": {
4141
"runtime-es2015": 2289,
42-
"main-es2015": 245351,
42+
"main-es2015": 245885,
4343
"polyfills-es2015": 36938,
4444
"5-es2015": 751
4545
}
@@ -49,7 +49,7 @@
4949
"master": {
5050
"uncompressed": {
5151
"runtime-es2015": 2289,
52-
"main-es2015": 221939,
52+
"main-es2015": 222476,
5353
"polyfills-es2015": 36723,
5454
"5-es2015": 781
5555
}

‎packages/router/src/apply_redirects.ts‎

Lines changed: 50 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
*/
88

99
import {Injector, NgModuleRef} from '@angular/core';
10-
import {defer, EmptyError, Observable, Observer, of} from 'rxjs';
11-
import {catchError, concatAll, first, map, mergeMap, tap} from 'rxjs/operators';
10+
import {defer, EmptyError, from, Observable, Observer, of} from 'rxjs';
11+
import {catchError, combineAll, concatMap, first, map, mergeMap, tap} from 'rxjs/operators';
1212

1313
import {LoadedRouterConfig, Route, Routes} from './config';
1414
import {CanLoadFn} from './interfaces';
@@ -17,6 +17,7 @@ import {RouterConfigLoader} from './router_config_loader';
1717
import {defaultUrlMatcher, navigationCancelingError, Params, PRIMARY_OUTLET} from './shared';
1818
import {UrlSegment, UrlSegmentGroup, UrlSerializer, UrlTree} from './url_tree';
1919
import {forEach, waitForMap, wrapIntoObservable} from './utils/collection';
20+
import {getOutlet, groupRoutesByOutlet} from './utils/config';
2021
import {isCanLoad, isFunction, isUrlTree} from './utils/type_guards';
2122

2223
class NoMatch {
@@ -148,28 +149,52 @@ class ApplyRedirects {
148149
ngModule: NgModuleRef<any>, segmentGroup: UrlSegmentGroup, routes: Route[],
149150
segments: UrlSegment[], outlet: string,
150151
allowRedirects: boolean): Observable<UrlSegmentGroup> {
151-
return of(...routes).pipe(
152-
map((r: any) => {
153-
const expanded$ = this.expandSegmentAgainstRoute(
154-
ngModule, segmentGroup, routes, r, segments, outlet, allowRedirects);
155-
return expanded$.pipe(catchError((e: any) => {
156-
if (e instanceof NoMatch) {
157-
// TODO(i): this return type doesn't match the declared Observable<UrlSegmentGroup> -
158-
// talk to Jason
159-
return of(null) as any;
152+
// We need to expand each outlet group independently to ensure that we not only load modules
153+
// for routes matching the given `outlet`, but also those which will be activated because
154+
// their path is empty string. This can result in multiple outlets being activated at once.
155+
const routesByOutlet: Map<string, Route[]> = groupRoutesByOutlet(routes);
156+
if (!routesByOutlet.has(outlet)) {
157+
routesByOutlet.set(outlet, []);
158+
}
159+
160+
const expandRoutes = (routes: Route[]) => {
161+
return from(routes).pipe(
162+
concatMap((r: Route) => {
163+
const expanded$ = this.expandSegmentAgainstRoute(
164+
ngModule, segmentGroup, routes, r, segments, outlet, allowRedirects);
165+
return expanded$.pipe(catchError(e => {
166+
if (e instanceof NoMatch) {
167+
return of(null);
168+
}
169+
throw e;
170+
}));
171+
}),
172+
first((s: UrlSegmentGroup|null): s is UrlSegmentGroup => s !== null),
173+
catchError(e => {
174+
if (e instanceof EmptyError || e.name === 'EmptyError') {
175+
if (this.noLeftoversInUrl(segmentGroup, segments, outlet)) {
176+
return of(new UrlSegmentGroup([], {}));
177+
}
178+
throw new NoMatch(segmentGroup);
160179
}
161180
throw e;
162-
}));
163-
}),
164-
concatAll(), first((s: any) => !!s), catchError((e: any, _: any) => {
165-
if (e instanceof EmptyError || e.name === 'EmptyError') {
166-
if (this.noLeftoversInUrl(segmentGroup, segments, outlet)) {
167-
return of(new UrlSegmentGroup([], {}));
168-
}
169-
throw new NoMatch(segmentGroup);
170-
}
171-
throw e;
172-
}));
181+
}),
182+
);
183+
};
184+
185+
const expansions = Array.from(routesByOutlet.entries()).map(([routeOutlet, routes]) => {
186+
const expanded = expandRoutes(routes);
187+
// Map all results from outlets we aren't activating to `null` so they can be ignored later
188+
return routeOutlet === outlet ? expanded :
189+
expanded.pipe(map(() => null), catchError(() => of(null)));
190+
});
191+
return from(expansions)
192+
.pipe(
193+
combineAll(),
194+
first(),
195+
// Return only the expansion for the route outlet we are trying to activate.
196+
map(results => results.find(result => result !== null)!),
197+
);
173198
}
174199

175200
private noLeftoversInUrl(segmentGroup: UrlSegmentGroup, segments: UrlSegment[], outlet: string):
@@ -180,7 +205,9 @@ class ApplyRedirects {
180205
private expandSegmentAgainstRoute(
181206
ngModule: NgModuleRef<any>, segmentGroup: UrlSegmentGroup, routes: Route[], route: Route,
182207
paths: UrlSegment[], outlet: string, allowRedirects: boolean): Observable<UrlSegmentGroup> {
183-
if (getOutlet(route) !== outlet) {
208+
// Empty string segments are special because multiple outlets can match a single path, i.e.
209+
// `[{path: '', component: B}, {path: '', loadChildren: () => {}, outlet: "about"}]`
210+
if (getOutlet(route) !== outlet && route.path !== '') {
184211
return noMatch(segmentGroup);
185212
}
186213

@@ -551,7 +578,3 @@ function isEmptyPathRedirect(
551578

552579
return r.path === '' && r.redirectTo !== undefined;
553580
}
554-
555-
function getOutlet(route: Route): string {
556-
return route.outlet || PRIMARY_OUTLET;
557-
}

‎packages/router/src/utils/config.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,3 +113,21 @@ export function standardizeConfig(r: Route): Route {
113113
}
114114
return c;
115115
}
116+
117+
/** Returns of `Map` of outlet names to the `Route`s for that outlet. */
118+
export function groupRoutesByOutlet(routes: Route[]): Map<string, Route[]> {
119+
return routes.reduce((map, route) => {
120+
const routeOutlet = getOutlet(route);
121+
if (map.has(routeOutlet)) {
122+
map.get(routeOutlet)!.push(route);
123+
} else {
124+
map.set(routeOutlet, [route]);
125+
}
126+
return map;
127+
}, new Map<string, Route[]>());
128+
}
129+
130+
/** Returns the `route.outlet` or PRIMARY_OUTLET if none exists. */
131+
export function getOutlet(route: Route): string {
132+
return route.outlet || PRIMARY_OUTLET;
133+
}

‎packages/router/test/apply_redirects.spec.ts‎

Lines changed: 136 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
*/
88

99
import {NgModuleRef} from '@angular/core';
10-
import {TestBed} from '@angular/core/testing';
10+
import {fakeAsync, TestBed, tick} from '@angular/core/testing';
1111
import {Observable, of} from 'rxjs';
12-
import {delay} from 'rxjs/operators';
12+
import {delay, tap} from 'rxjs/operators';
1313

1414
import {applyRedirects} from '../src/apply_redirects';
1515
import {LoadedRouterConfig, Route, Routes} from '../src/config';
@@ -482,6 +482,88 @@ describe('applyRedirects', () => {
482482
expect((config[0] as any)._loadedConfig).toBe(loadedConfig);
483483
});
484484
});
485+
486+
it('should load all matching configurations of empty path, including an auxiliary outlets',
487+
fakeAsync(() => {
488+
const loadedConfig =
489+
new LoadedRouterConfig([{path: '', component: ComponentA}], testModule);
490+
let loadCalls = 0;
491+
let loaded: string[] = [];
492+
const loader = {
493+
load: (injector: any, p: Route) => {
494+
loadCalls++;
495+
return of(loadedConfig)
496+
.pipe(
497+
delay(100 * loadCalls),
498+
tap(() => loaded.push(p.loadChildren! as string)),
499+
);
500+
}
501+
};
502+
503+
const config: Routes =
504+
[{path: '', loadChildren: 'root'}, {path: '', loadChildren: 'aux', outlet: 'popup'}];
505+
506+
applyRedirects(testModule.injector, <any>loader, serializer, tree(''), config).subscribe();
507+
expect(loadCalls).toBe(2);
508+
tick(100);
509+
expect(loaded).toEqual(['root']);
510+
tick(100);
511+
expect(loaded).toEqual(['root', 'aux']);
512+
}));
513+
514+
it('loads only the first match when two Routes with the same outlet have the same path', () => {
515+
const loadedConfig = new LoadedRouterConfig([{path: '', component: ComponentA}], testModule);
516+
let loadCalls = 0;
517+
let loaded: string[] = [];
518+
const loader = {
519+
load: (injector: any, p: Route) => {
520+
loadCalls++;
521+
return of(loadedConfig)
522+
.pipe(
523+
tap(() => loaded.push(p.loadChildren! as string)),
524+
);
525+
}
526+
};
527+
528+
const config: Routes =
529+
[{path: 'a', loadChildren: 'first'}, {path: 'a', loadChildren: 'second'}];
530+
531+
applyRedirects(testModule.injector, <any>loader, serializer, tree('a'), config).subscribe();
532+
expect(loadCalls).toBe(1);
533+
expect(loaded).toEqual(['first']);
534+
});
535+
536+
it('should load the configuration of empty root path if the entry is an aux outlet',
537+
fakeAsync(() => {
538+
const loadedConfig =
539+
new LoadedRouterConfig([{path: '', component: ComponentA}], testModule);
540+
let loaded: string[] = [];
541+
const rootDelay = 100;
542+
const auxDelay = 1;
543+
const loader = {
544+
load: (injector: any, p: Route) => {
545+
const delayMs = p.loadChildren! as string === 'aux' ? auxDelay : rootDelay;
546+
return of(loadedConfig)
547+
.pipe(
548+
delay(delayMs),
549+
tap(() => loaded.push(p.loadChildren! as string)),
550+
);
551+
}
552+
};
553+
554+
const config: Routes = [
555+
// Define aux route first so it matches before the primary outlet
556+
{path: 'modal', loadChildren: 'aux', outlet: 'popup'},
557+
{path: '', loadChildren: 'root'},
558+
];
559+
560+
applyRedirects(testModule.injector, <any>loader, serializer, tree('(popup:modal)'), config)
561+
.subscribe();
562+
tick(auxDelay);
563+
expect(loaded).toEqual(['aux']);
564+
tick(rootDelay);
565+
expect(loaded).toEqual(['aux', 'root']);
566+
}));
485567
});
486568

487569
describe('empty paths', () => {
@@ -754,6 +836,46 @@ describe('applyRedirects', () => {
754836
});
755837
});
756838

839+
describe('multiple matches with empty path named outlets', () => {
840+
it('should work with redirects when other outlet comes before the one being activated', () => {
841+
applyRedirects(
842+
testModule.injector, null!, serializer, tree(''),
843+
[
844+
{
845+
path: '',
846+
children: [
847+
{path: '', component: ComponentA, outlet: 'aux'},
848+
{path: '', redirectTo: 'b', pathMatch: 'full'},
849+
{path: 'b', component: ComponentB},
850+
],
851+
},
852+
])
853+
.subscribe(
854+
(tree: UrlTree) => {
855+
expect(tree.toString()).toEqual('/b');
856+
},
857+
() => {
858+
fail('should not be reached');
859+
});
860+
});
861+
862+
it('should work when entry point is named outlet', () => {
863+
applyRedirects(
864+
testModule.injector, null!, serializer, tree('(popup:modal)'),
865+
[
866+
{path: '', component: ComponentA},
867+
{path: 'modal', component: ComponentB, outlet: 'popup'},
868+
])
869+
.subscribe(
870+
(tree: UrlTree) => {
871+
expect(tree.toString()).toEqual('/(popup:modal)');
872+
},
873+
(e) => {
874+
fail('should not be reached' + e.message);
875+
});
876+
});
877+
});
878+
757879
describe('redirecting to named outlets', () => {
758880
it('should work when using absolute redirects', () => {
759881
checkRedirect(
@@ -794,6 +916,18 @@ describe('applyRedirects', () => {
794916
});
795917
});
796918
});
919+
920+
// internal failure b/165719418
921+
it('does not fail with large configs', () => {
922+
const config: Routes = [];
923+
for (let i = 0; i < 400; i++) {
924+
config.push({path: 'no_match', component: ComponentB});
925+
}
926+
config.push({path: 'match', component: ComponentA});
927+
applyRedirects(testModule.injector, null!, serializer, tree('match'), config).forEach(r => {
928+
expectTreeToBe(r, 'match');
929+
});
930+
});
797931
});
798932

799933
function checkRedirect(config: Routes, url: string, callback: any): void {

0 commit comments

Comments
 (0)