diff --git a/goldens/size-tracking/integration-payloads.json b/goldens/size-tracking/integration-payloads.json index 2d5ce83c09..e96c939e37 100644 --- a/goldens/size-tracking/integration-payloads.json +++ b/goldens/size-tracking/integration-payloads.json @@ -39,7 +39,7 @@ "master": { "uncompressed": { "runtime-es2015": 2289, - "main-es2015": 245351, + "main-es2015": 245885, "polyfills-es2015": 36938, "5-es2015": 751 } @@ -49,7 +49,7 @@ "master": { "uncompressed": { "runtime-es2015": 2289, - "main-es2015": 221939, + "main-es2015": 222476, "polyfills-es2015": 36723, "5-es2015": 781 } diff --git a/packages/router/src/apply_redirects.ts b/packages/router/src/apply_redirects.ts index 5eb4050473..dcf44e727a 100644 --- a/packages/router/src/apply_redirects.ts +++ b/packages/router/src/apply_redirects.ts @@ -7,8 +7,8 @@ */ import {Injector, NgModuleRef} from '@angular/core'; -import {defer, EmptyError, Observable, Observer, of} from 'rxjs'; -import {catchError, concatAll, first, map, mergeMap, tap} from 'rxjs/operators'; +import {defer, EmptyError, from, Observable, Observer, of} from 'rxjs'; +import {catchError, combineAll, concatMap, first, map, mergeMap, tap} from 'rxjs/operators'; import {LoadedRouterConfig, Route, Routes} from './config'; import {CanLoadFn} from './interfaces'; @@ -17,6 +17,7 @@ import {RouterConfigLoader} from './router_config_loader'; import {defaultUrlMatcher, navigationCancelingError, Params, PRIMARY_OUTLET} from './shared'; import {UrlSegment, UrlSegmentGroup, UrlSerializer, UrlTree} from './url_tree'; import {forEach, waitForMap, wrapIntoObservable} from './utils/collection'; +import {getOutlet, groupRoutesByOutlet} from './utils/config'; import {isCanLoad, isFunction, isUrlTree} from './utils/type_guards'; class NoMatch { @@ -148,28 +149,52 @@ class ApplyRedirects { ngModule: NgModuleRef, segmentGroup: UrlSegmentGroup, routes: Route[], segments: UrlSegment[], outlet: string, allowRedirects: boolean): Observable { - return of(...routes).pipe( - map((r: any) => { - const expanded$ = this.expandSegmentAgainstRoute( - ngModule, segmentGroup, routes, r, segments, outlet, allowRedirects); - return expanded$.pipe(catchError((e: any) => { - if (e instanceof NoMatch) { - // TODO(i): this return type doesn't match the declared Observable - - // talk to Jason - return of(null) as any; + // We need to expand each outlet group independently to ensure that we not only load modules + // for routes matching the given `outlet`, but also those which will be activated because + // their path is empty string. This can result in multiple outlets being activated at once. + const routesByOutlet: Map = groupRoutesByOutlet(routes); + if (!routesByOutlet.has(outlet)) { + routesByOutlet.set(outlet, []); + } + + const expandRoutes = (routes: Route[]) => { + return from(routes).pipe( + concatMap((r: Route) => { + const expanded$ = this.expandSegmentAgainstRoute( + ngModule, segmentGroup, routes, r, segments, outlet, allowRedirects); + return expanded$.pipe(catchError(e => { + if (e instanceof NoMatch) { + return of(null); + } + throw e; + })); + }), + first((s: UrlSegmentGroup|null): s is UrlSegmentGroup => s !== null), + catchError(e => { + if (e instanceof EmptyError || e.name === 'EmptyError') { + if (this.noLeftoversInUrl(segmentGroup, segments, outlet)) { + return of(new UrlSegmentGroup([], {})); + } + throw new NoMatch(segmentGroup); } throw e; - })); - }), - concatAll(), first((s: any) => !!s), catchError((e: any, _: any) => { - if (e instanceof EmptyError || e.name === 'EmptyError') { - if (this.noLeftoversInUrl(segmentGroup, segments, outlet)) { - return of(new UrlSegmentGroup([], {})); - } - throw new NoMatch(segmentGroup); - } - throw e; - })); + }), + ); + }; + + const expansions = Array.from(routesByOutlet.entries()).map(([routeOutlet, routes]) => { + const expanded = expandRoutes(routes); + // Map all results from outlets we aren't activating to `null` so they can be ignored later + return routeOutlet === outlet ? expanded : + expanded.pipe(map(() => null), catchError(() => of(null))); + }); + return from(expansions) + .pipe( + combineAll(), + first(), + // Return only the expansion for the route outlet we are trying to activate. + map(results => results.find(result => result !== null)!), + ); } private noLeftoversInUrl(segmentGroup: UrlSegmentGroup, segments: UrlSegment[], outlet: string): @@ -180,7 +205,9 @@ class ApplyRedirects { private expandSegmentAgainstRoute( ngModule: NgModuleRef, segmentGroup: UrlSegmentGroup, routes: Route[], route: Route, paths: UrlSegment[], outlet: string, allowRedirects: boolean): Observable { - if (getOutlet(route) !== outlet) { + // Empty string segments are special because multiple outlets can match a single path, i.e. + // `[{path: '', component: B}, {path: '', loadChildren: () => {}, outlet: "about"}]` + if (getOutlet(route) !== outlet && route.path !== '') { return noMatch(segmentGroup); } @@ -551,7 +578,3 @@ function isEmptyPathRedirect( return r.path === '' && r.redirectTo !== undefined; } - -function getOutlet(route: Route): string { - return route.outlet || PRIMARY_OUTLET; -} diff --git a/packages/router/src/utils/config.ts b/packages/router/src/utils/config.ts index 961ee3c54f..f38e56cfd4 100644 --- a/packages/router/src/utils/config.ts +++ b/packages/router/src/utils/config.ts @@ -113,3 +113,21 @@ export function standardizeConfig(r: Route): Route { } return c; } + +/** Returns of `Map` of outlet names to the `Route`s for that outlet. */ +export function groupRoutesByOutlet(routes: Route[]): Map { + return routes.reduce((map, route) => { + const routeOutlet = getOutlet(route); + if (map.has(routeOutlet)) { + map.get(routeOutlet)!.push(route); + } else { + map.set(routeOutlet, [route]); + } + return map; + }, new Map()); +} + +/** Returns the `route.outlet` or PRIMARY_OUTLET if none exists. */ +export function getOutlet(route: Route): string { + return route.outlet || PRIMARY_OUTLET; +} diff --git a/packages/router/test/apply_redirects.spec.ts b/packages/router/test/apply_redirects.spec.ts index d564727a88..3b27bf1fa5 100644 --- a/packages/router/test/apply_redirects.spec.ts +++ b/packages/router/test/apply_redirects.spec.ts @@ -7,9 +7,9 @@ */ import {NgModuleRef} from '@angular/core'; -import {TestBed} from '@angular/core/testing'; +import {fakeAsync, TestBed, tick} from '@angular/core/testing'; import {Observable, of} from 'rxjs'; -import {delay} from 'rxjs/operators'; +import {delay, tap} from 'rxjs/operators'; import {applyRedirects} from '../src/apply_redirects'; import {LoadedRouterConfig, Route, Routes} from '../src/config'; @@ -482,6 +482,88 @@ describe('applyRedirects', () => { expect((config[0] as any)._loadedConfig).toBe(loadedConfig); }); }); + + it('should load all matching configurations of empty path, including an auxiliary outlets', + fakeAsync(() => { + const loadedConfig = + new LoadedRouterConfig([{path: '', 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: 'root'}, {path: '', loadChildren: 'aux', outlet: 'popup'}]; + + applyRedirects(testModule.injector, loader, serializer, tree(''), config).subscribe(); + expect(loadCalls).toBe(2); + tick(100); + expect(loaded).toEqual(['root']); + tick(100); + expect(loaded).toEqual(['root', 'aux']); + })); + + 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; + let loaded: string[] = []; + const loader = { + load: (injector: any, p: Route) => { + loadCalls++; + return of(loadedConfig) + .pipe( + tap(() => loaded.push(p.loadChildren! as string)), + ); + } + }; + + const config: Routes = + [{path: 'a', loadChildren: 'first'}, {path: 'a', loadChildren: 'second'}]; + + applyRedirects(testModule.injector, loader, serializer, tree('a'), config).subscribe(); + expect(loadCalls).toBe(1); + expect(loaded).toEqual(['first']); + }); + + it('should load the configuration of empty root path if the entry is an aux outlet', + fakeAsync(() => { + const loadedConfig = + new LoadedRouterConfig([{path: '', component: ComponentA}], testModule); + let loaded: string[] = []; + const rootDelay = 100; + const auxDelay = 1; + const loader = { + load: (injector: any, p: Route) => { + const delayMs = p.loadChildren! as string === 'aux' ? auxDelay : rootDelay; + return of(loadedConfig) + .pipe( + delay(delayMs), + tap(() => loaded.push(p.loadChildren! as string)), + ); + } + }; + + const config: Routes = [ + // Define aux route first so it matches before the primary outlet + {path: 'modal', loadChildren: 'aux', outlet: 'popup'}, + {path: '', loadChildren: 'root'}, + ]; + + applyRedirects(testModule.injector, loader, serializer, tree('(popup:modal)'), config) + .subscribe(); + tick(auxDelay); + expect(loaded).toEqual(['aux']); + tick(rootDelay); + expect(loaded).toEqual(['aux', 'root']); + })); }); describe('empty paths', () => { @@ -754,6 +836,46 @@ describe('applyRedirects', () => { }); }); + describe('multiple matches with empty path named outlets', () => { + it('should work with redirects when other outlet comes before the one being activated', () => { + applyRedirects( + testModule.injector, null!, serializer, tree(''), + [ + { + path: '', + children: [ + {path: '', component: ComponentA, outlet: 'aux'}, + {path: '', redirectTo: 'b', pathMatch: 'full'}, + {path: 'b', component: ComponentB}, + ], + }, + ]) + .subscribe( + (tree: UrlTree) => { + expect(tree.toString()).toEqual('/b'); + }, + () => { + fail('should not be reached'); + }); + }); + + it('should work when entry point is named outlet', () => { + applyRedirects( + testModule.injector, null!, serializer, tree('(popup:modal)'), + [ + {path: '', component: ComponentA}, + {path: 'modal', component: ComponentB, outlet: 'popup'}, + ]) + .subscribe( + (tree: UrlTree) => { + expect(tree.toString()).toEqual('/(popup:modal)'); + }, + (e) => { + fail('should not be reached' + e.message); + }); + }); + }); + describe('redirecting to named outlets', () => { it('should work when using absolute redirects', () => { checkRedirect( @@ -794,6 +916,18 @@ describe('applyRedirects', () => { }); }); }); + + // internal failure b/165719418 + it('does not fail with large configs', () => { + const config: Routes = []; + for (let i = 0; i < 400; i++) { + config.push({path: 'no_match', component: ComponentB}); + } + config.push({path: 'match', component: ComponentA}); + applyRedirects(testModule.injector, null!, serializer, tree('match'), config).forEach(r => { + expectTreeToBe(r, 'match'); + }); + }); }); function checkRedirect(config: Routes, url: string, callback: any): void {