Skip to content

Commit 8f24bc9

Browse files
mheverymhevery
committed
Revert "fix(router): support lazy loading for empty path named outlets (#38379)"
This reverts commit 7ad3264.
1 parent ac461e1 commit 8f24bc9

3 files changed

Lines changed: 27 additions & 171 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": 245885,
42+
"main-es2015": 245351,
4343
"polyfills-es2015": 36938,
4444
"5-es2015": 751
4545
}

‎packages/router/src/apply_redirects.ts‎

Lines changed: 24 additions & 45 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, first, map, mergeMap, switchMap, tap} from 'rxjs/operators';
11+
import {catchError, concatAll, first, map, mergeMap, tap} from 'rxjs/operators';
1212

1313
import {LoadedRouterConfig, Route, Routes} from './config';
1414
import {CanLoadFn} from './interfaces';
@@ -148,47 +148,28 @@ class ApplyRedirects {
148148
ngModule: NgModuleRef<any>, segmentGroup: UrlSegmentGroup, routes: Route[],
149149
segments: UrlSegment[], outlet: string,
150150
allowRedirects: boolean): Observable<UrlSegmentGroup> {
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-
);
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+
}));
192173
}
193174

194175
private noLeftoversInUrl(segmentGroup: UrlSegmentGroup, segments: UrlSegment[], outlet: string):
@@ -199,9 +180,7 @@ class ApplyRedirects {
199180
private expandSegmentAgainstRoute(
200181
ngModule: NgModuleRef<any>, segmentGroup: UrlSegmentGroup, routes: Route[], route: Route,
201182
paths: UrlSegment[], outlet: string, allowRedirects: boolean): Observable<UrlSegmentGroup> {
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 !== '') {
183+
if (getOutlet(route) !== outlet) {
205184
return noMatch(segmentGroup);
206185
}
207186

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

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

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

1414
import {applyRedirects} from '../src/apply_redirects';
1515
import {LoadedRouterConfig, Route, Routes} from '../src/config';
@@ -482,89 +482,6 @@ 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-
}));
568485
});
569486

570487
describe('empty paths', () => {
@@ -837,46 +754,6 @@ describe('applyRedirects', () => {
837754
});
838755
});
839756

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-
880757
describe('redirecting to named outlets', () => {
881758
it('should work when using absolute redirects', () => {
882759
checkRedirect(

0 commit comments

Comments
 (0)