Skip to content

Commit 7ad3264

Browse files
atscottmhevery
atscott
authored and
mhevery
committed
fix(router): support lazy loading for empty path named outlets (angular#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 angular#12842 PR Close angular#38379
1 parent 9ad69c1 commit 7ad3264

3 files changed

Lines changed: 171 additions & 27 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
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
}

‎packages/router/src/apply_redirects.ts‎

Lines changed: 45 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

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

1313
import {LoadedRouterConfig, Route, Routes} from './config';
1414
import {CanLoadFn} from './interfaces';
@@ -148,28 +148,47 @@ class ApplyRedirects {
148148
ngModule: NgModuleRef<any>, segmentGroup: UrlSegmentGroup, routes: Route[],
149149
segments: UrlSegment[], outlet: string,
150150
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;
160-
}
161-
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-
}));
151+
type MatchedSegment = {segment: UrlSegmentGroup, outlet: string};
152+
// This logic takes each route and switches to a new observable that depends on the result of
153+
// the previous route expansion. In this way, we compose a list of results where each one can
154+
// depend on and look at the previous to determine how to proceed with expansion of the
155+
// current route.
156+
return routes
157+
.reduce(
158+
(accumulatedResults: Observable<Array<MatchedSegment>>, r: Route) => {
159+
return accumulatedResults.pipe(switchMap(resultsThusFar => {
160+
// If we already matched a previous `Route` with the same outlet as the current,
161+
// we should not process the current one.
162+
if (resultsThusFar.some(result => result && result.outlet === getOutlet(r))) {
163+
return of(resultsThusFar);
164+
}
165+
const expanded$ = this.expandSegmentAgainstRoute(
166+
ngModule, segmentGroup, routes, r, segments, outlet, allowRedirects);
167+
return expanded$.pipe(
168+
map((segment) => resultsThusFar.concat({segment, outlet: getOutlet(r)})),
169+
catchError((e: any) => {
170+
if (e instanceof NoMatch) {
171+
return of(resultsThusFar);
172+
}
173+
throw e;
174+
}));
175+
}));
176+
},
177+
of([] as MatchedSegment[]))
178+
.pipe(
179+
// Find the matched segment whose outlet matches the one we're looking for.
180+
map(results => results.find(s => s.outlet === outlet)?.segment),
181+
first((s): s is UrlSegmentGroup => s !== undefined),
182+
catchError((e: any, _: any) => {
183+
if (e instanceof EmptyError || e.name === 'EmptyError') {
184+
if (this.noLeftoversInUrl(segmentGroup, segments, outlet)) {
185+
return of(new UrlSegmentGroup([], {}));
186+
}
187+
throw new NoMatch(segmentGroup);
188+
}
189+
throw e;
190+
}),
191+
);
173192
}
174193

175194
private noLeftoversInUrl(segmentGroup: UrlSegmentGroup, segments: UrlSegment[], outlet: string):
@@ -180,7 +199,9 @@ class ApplyRedirects {
180199
private expandSegmentAgainstRoute(
181200
ngModule: NgModuleRef<any>, segmentGroup: UrlSegmentGroup, routes: Route[], route: Route,
182201
paths: UrlSegment[], outlet: string, allowRedirects: boolean): Observable<UrlSegmentGroup> {
183-
if (getOutlet(route) !== outlet) {
202+
// Empty string segments are special because multiple outlets can match a single path, i.e.
203+
// `[{path: '', component: B}, {path: '', loadChildren: () => {}, outlet: "about"}]`
204+
if (getOutlet(route) !== outlet && route.path !== '') {
184205
return noMatch(segmentGroup);
185206
}
186207

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

Lines changed: 125 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,89 @@ 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(1);
508+
tick(100);
509+
expect(loaded).toEqual(['root']);
510+
tick(200);
511+
expect(loadCalls).toBe(2);
512+
expect(loaded).toEqual(['root', 'aux']);
513+
}));
514+
515+
it('loads only the first match when two Routes with the same outlet have the same path', () => {
516+
const loadedConfig = new LoadedRouterConfig([{path: '', component: ComponentA}], testModule);
517+
let loadCalls = 0;
518+
let loaded: string[] = [];
519+
const loader = {
520+
load: (injector: any, p: Route) => {
521+
loadCalls++;
522+
return of(loadedConfig)
523+
.pipe(
524+
tap(() => loaded.push(p.loadChildren! as string)),
525+
);
526+
}
527+
};
528+
529+
const config: Routes =
530+
[{path: 'a', loadChildren: 'first'}, {path: 'a', loadChildren: 'second'}];
531+
532+
applyRedirects(testModule.injector, <any>loader, serializer, tree('a'), config).subscribe();
533+
expect(loadCalls).toBe(1);
534+
expect(loaded).toEqual(['first']);
535+
});
536+
537+
it('should load the configuration of empty root path if the entry is an aux outlet',
538+
fakeAsync(() => {
539+
const loadedConfig =
540+
new LoadedRouterConfig([{path: '', component: ComponentA}], testModule);
541+
let loaded: string[] = [];
542+
const rootDelay = 100;
543+
const auxDelay = 1;
544+
const loader = {
545+
load: (injector: any, p: Route) => {
546+
const delayMs = p.loadChildren! as string === 'aux' ? auxDelay : rootDelay;
547+
return of(loadedConfig)
548+
.pipe(
549+
delay(delayMs),
550+
tap(() => loaded.push(p.loadChildren! as string)),
551+
);
552+
}
553+
};
554+
555+
const config: Routes = [
556+
// Define aux route first so it matches before the primary outlet
557+
{path: 'modal', loadChildren: 'aux', outlet: 'popup'},
558+
{path: '', loadChildren: 'root'},
559+
];
560+
561+
applyRedirects(testModule.injector, <any>loader, serializer, tree('(popup:modal)'), config)
562+
.subscribe();
563+
tick(auxDelay);
564+
expect(loaded).toEqual(['aux']);
565+
tick(rootDelay);
566+
expect(loaded).toEqual(['aux', 'root']);
567+
}));
485568
});
486569

487570
describe('empty paths', () => {
@@ -754,6 +837,46 @@ describe('applyRedirects', () => {
754837
});
755838
});
756839

840+
describe('multiple matches with empty path named outlets', () => {
841+
it('should work with redirects when other outlet comes before the one being activated', () => {
842+
applyRedirects(
843+
testModule.injector, null!, serializer, tree(''),
844+
[
845+
{
846+
path: '',
847+
children: [
848+
{path: '', component: ComponentA, outlet: 'aux'},
849+
{path: '', redirectTo: 'b', pathMatch: 'full'},
850+
{path: 'b', component: ComponentB},
851+
],
852+
},
853+
])
854+
.subscribe(
855+
(tree: UrlTree) => {
856+
expect(tree.toString()).toEqual('/b');
857+
},
858+
() => {
859+
fail('should not be reached');
860+
});
861+
});
862+
863+
it('should work when entry point is named outlet', () => {
864+
applyRedirects(
865+
testModule.injector, null!, serializer, tree('(popup:modal)'),
866+
[
867+
{path: '', component: ComponentA},
868+
{path: 'modal', component: ComponentB, outlet: 'popup'},
869+
])
870+
.subscribe(
871+
(tree: UrlTree) => {
872+
expect(tree.toString()).toEqual('/(popup:modal)');
873+
},
874+
(e) => {
875+
fail('should not be reached' + e.message);
876+
});
877+
});
878+
});
879+
757880
describe('redirecting to named outlets', () => {
758881
it('should work when using absolute redirects', () => {
759882
checkRedirect(

0 commit comments

Comments
 (0)