From e43f7e26fe8d3010682c56c68c0e09791bb3ed55 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Mon, 30 Nov 2020 11:56:27 -0800 Subject: [PATCH] fix(router): apply redirects should match named outlets with empty path parents (#40029) There are two parts to this commit: 1. Revert the changes from #38379. This change had an incomplete view of how things worked and also diverged the implementations of `applyRedirects` and `recognize` even more. 2. Apply the fixes from the `recognize` algorithm to ensure that named outlets with empty path parents can be matched. This change also passes all the tests that were added in #38379 with the added benefit of being a more complete fix that stays in-line with the `recognize` algorithm. This was made possible by using the same approach for `split` by always creating segments for empty path matches (previously, this was only done in `applyRedirects` if there was a `redirectTo` value). At the end of the expansions, we need to squash all empty segments so that serializing the final `UrlTree` returns the same result as before. Fixes #39952 Fixes #10726 Closes #30410 PR Close #40029 --- .../size-tracking/integration-payloads.json | 6 +- .../router/bundle.golden_symbols.json | 31 +++- packages/router/src/apply_redirects.ts | 140 ++++++++--------- packages/router/src/recognize.ts | 22 +-- packages/router/src/utils/collection.ts | 26 ---- packages/router/src/utils/config.ts | 23 ++- packages/router/src/utils/config_matching.ts | 5 +- packages/router/test/apply_redirects.spec.ts | 144 +++++++++++++++++- packages/router/test/integration.spec.ts | 21 ++- 9 files changed, 280 insertions(+), 138 deletions(-) diff --git a/goldens/size-tracking/integration-payloads.json b/goldens/size-tracking/integration-payloads.json index 4cd45a5f52..b198268033 100644 --- a/goldens/size-tracking/integration-payloads.json +++ b/goldens/size-tracking/integration-payloads.json @@ -39,7 +39,7 @@ "master": { "uncompressed": { "runtime-es2015": 2285, - "main-es2015": 241837, + "main-es2015": 240909, "polyfills-es2015": 36709, "5-es2015": 745 } @@ -49,7 +49,7 @@ "master": { "uncompressed": { "runtime-es2015": 2289, - "main-es2015": 218507, + "main-es2015": 217591, "polyfills-es2015": 36723, "5-es2015": 781 } @@ -66,4 +66,4 @@ } } } -} +} \ No newline at end of file diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index fb04c8493f..bef5f35ee1 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -1457,6 +1457,9 @@ { "name": "handleError" }, + { + "name": "hasEmptyPathConfig" + }, { "name": "hasParentInjector" }, @@ -1542,13 +1545,13 @@ "name": "isDirectiveHost" }, { - "name": "isEmptyPathRedirect" + "name": "isFunction" }, { "name": "isFunction" }, { - "name": "isFunction" + "name": "isImmediateMatch" }, { "name": "isInCheckNoChangesMode" @@ -1646,9 +1649,6 @@ { "name": "map" }, - { - "name": "mapChildrenIntoArray" - }, { "name": "markAsComponentHost" }, @@ -1682,9 +1682,6 @@ { "name": "mergeMap" }, - { - "name": "mergeTrivialChildren" - }, { "name": "modules" }, @@ -1706,6 +1703,9 @@ { "name": "navigationCancelingError" }, + { + "name": "newObservableError" + }, { "name": "nextBindingIndex" }, @@ -1715,6 +1715,12 @@ { "name": "ngOnChangesSetInput" }, + { + "name": "noLeftoversInUrl" + }, + { + "name": "noMatch" + }, { "name": "noMatch" }, @@ -1838,6 +1844,9 @@ { "name": "saveNameToExportMap" }, + { + "name": "scan" + }, { "name": "scheduleArray" }, @@ -1910,9 +1919,15 @@ { "name": "shouldSearchParent" }, + { + "name": "sortByMatchingOutlets" + }, { "name": "split" }, + { + "name": "squashSegmentGroup" + }, { "name": "standardizeConfig" }, diff --git a/packages/router/src/apply_redirects.ts b/packages/router/src/apply_redirects.ts index a6b76085b2..22228b1290 100644 --- a/packages/router/src/apply_redirects.ts +++ b/packages/router/src/apply_redirects.ts @@ -8,7 +8,7 @@ import {Injector, NgModuleRef} from '@angular/core'; import {EmptyError, from, Observable, Observer, of} from 'rxjs'; -import {catchError, combineAll, concatMap, first, map, mergeMap, tap} from 'rxjs/operators'; +import {catchError, concatMap, first, last, map, mergeMap, scan, tap} from 'rxjs/operators'; import {LoadedRouterConfig, Route, Routes} from './config'; import {CanLoadFn} from './interfaces'; @@ -16,9 +16,9 @@ import {prioritizedGuardValue} from './operators/prioritized_guard_value'; import {RouterConfigLoader} from './router_config_loader'; import {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 {match, noLeftoversInUrl, split} from './utils/config_matching'; +import {forEach, wrapIntoObservable} from './utils/collection'; +import {getOutlet, sortByMatchingOutlets} from './utils/config'; +import {isImmediateMatch, match, noLeftoversInUrl, split} from './utils/config_matching'; import {isCanLoad, isFunction, isUrlTree} from './utils/type_guards'; class NoMatch { @@ -78,8 +78,17 @@ class ApplyRedirects { } apply(): Observable { + const splitGroup = split(this.urlTree.root, [], [], this.config).segmentGroup; + // TODO(atscott): creating a new segment removes the _sourceSegment _segmentIndexShift, which is + // only necessary to prevent failures in tests which assert exact object matches. The `split` is + // now shared between `applyRedirects` and `recognize` but only the `recognize` step needs these + // properties. Before the implementations were merged, the `applyRedirects` would not assign + // them. We should be able to remove this logic as a "breaking change" but should do some more + // investigation into the failures first. + const rootSegmentGroup = new UrlSegmentGroup(splitGroup.segments, splitGroup.children); + const expanded$ = - this.expandSegmentGroup(this.ngModule, this.config, this.urlTree.root, PRIMARY_OUTLET); + this.expandSegmentGroup(this.ngModule, this.config, rootSegmentGroup, PRIMARY_OUTLET); const urlTrees$ = expanded$.pipe(map((rootSegmentGroup: UrlSegmentGroup) => { return this.createUrlTree( squashSegmentGroup(rootSegmentGroup), this.urlTree.queryParams, this.urlTree.fragment!); @@ -143,74 +152,73 @@ class ApplyRedirects { private expandChildren( ngModule: NgModuleRef, routes: Route[], segmentGroup: UrlSegmentGroup): Observable<{[name: string]: UrlSegmentGroup}> { - return waitForMap( - segmentGroup.children, - (childOutlet, child) => this.expandSegmentGroup(ngModule, routes, child, childOutlet)); + // Expand outlets one at a time, starting with the primary outlet. We need to do it this way + // because an absolute redirect from the primary outlet takes precedence. + const childOutlets: string[] = []; + for (const child of Object.keys(segmentGroup.children)) { + if (child === 'primary') { + childOutlets.unshift(child); + } else { + childOutlets.push(child); + } + } + + return from(childOutlets) + .pipe( + concatMap(childOutlet => { + const child = segmentGroup.children[childOutlet]; + // Sort the routes so routes with outlets that match the the segment appear + // first, followed by routes for other outlets, which might match if they have an + // empty path. + const sortedRoutes = sortByMatchingOutlets(routes, childOutlet); + return this.expandSegmentGroup(ngModule, sortedRoutes, child, childOutlet) + .pipe(map(s => ({segment: s, outlet: childOutlet}))); + }), + scan( + (children, expandedChild) => { + children[expandedChild.outlet] = expandedChild.segment; + return children; + }, + {} as {[outlet: string]: UrlSegmentGroup}), + last(), + ); } private expandSegment( ngModule: NgModuleRef, segmentGroup: UrlSegmentGroup, routes: Route[], segments: UrlSegment[], outlet: string, allowRedirects: boolean): Observable { - // 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 (noLeftoversInUrl(segmentGroup, segments, outlet)) { - return of(new UrlSegmentGroup([], {})); - } - throw new NoMatch(segmentGroup); + return from(routes).pipe( + concatMap((r: any) => { + const expanded$ = this.expandSegmentAgainstRoute( + ngModule, segmentGroup, routes, r, segments, outlet, allowRedirects); + return expanded$.pipe(catchError((e: any) => { + if (e instanceof NoMatch) { + return of(null); } 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)!), - ); + })); + }), + first((s): s is UrlSegmentGroup => !!s), catchError((e: any, _: any) => { + if (e instanceof EmptyError || e.name === 'EmptyError') { + if (noLeftoversInUrl(segmentGroup, segments, outlet)) { + return of(new UrlSegmentGroup([], {})); + } + throw new NoMatch(segmentGroup); + } + throw e; + })); } private expandSegmentAgainstRoute( ngModule: NgModuleRef, segmentGroup: UrlSegmentGroup, routes: Route[], route: Route, paths: UrlSegment[], outlet: string, allowRedirects: boolean): Observable { - // 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 !== '') { + if (!isImmediateMatch(route, segmentGroup, paths, outlet)) { return noMatch(segmentGroup); } if (route.redirectTo === undefined) { - return this.matchSegmentAgainstRoute(ngModule, segmentGroup, route, paths); + return this.matchSegmentAgainstRoute(ngModule, segmentGroup, route, paths, outlet); } if (allowRedirects && this.allowRedirects) { @@ -269,7 +277,7 @@ class ApplyRedirects { private matchSegmentAgainstRoute( ngModule: NgModuleRef, rawSegmentGroup: UrlSegmentGroup, route: Route, - segments: UrlSegment[]): Observable { + segments: UrlSegment[], outlet: string): Observable { if (route.path === '**') { if (route.loadChildren) { return this.configLoader.load(ngModule.injector, route) @@ -292,16 +300,11 @@ class ApplyRedirects { const childModule = routerConfig.module; const childConfig = routerConfig.routes; - const {segmentGroup, slicedSegments} = + const {segmentGroup: splitSegmentGroup, slicedSegments} = split(rawSegmentGroup, consumedSegments, rawSlicedSegments, childConfig); - // TODO(atscott): clearing the source segment and segment index shift is only necessary to - // prevent failures in tests which assert exact object matches. The `split` is now shared - // between applyRedirects and recognize and only the `recognize` step needs these properties. - // Before the implementations were merged, the applyRedirects would not assign them. - // We should be able to remove this logic as a "breaking change" but should do some more - // investigation into the failures first. - segmentGroup._sourceSegment = undefined; - segmentGroup._segmentIndexShift = undefined; + // See comment on the other call to `split` about why this is necessary. + const segmentGroup = + new UrlSegmentGroup(splitSegmentGroup.segments, splitSegmentGroup.children); if (slicedSegments.length === 0 && segmentGroup.hasChildren()) { const expanded$ = this.expandChildren(childModule, childConfig, segmentGroup); @@ -313,8 +316,10 @@ class ApplyRedirects { return of(new UrlSegmentGroup(consumedSegments, {})); } + const matchedOnOutlet = getOutlet(route) === outlet; const expanded$ = this.expandSegment( - childModule, segmentGroup, childConfig, slicedSegments, PRIMARY_OUTLET, true); + childModule, segmentGroup, childConfig, slicedSegments, + matchedOnOutlet ? PRIMARY_OUTLET : outlet, true); return expanded$.pipe( map((cs: UrlSegmentGroup) => new UrlSegmentGroup(consumedSegments.concat(cs.segments), cs.children))); @@ -473,7 +478,6 @@ class ApplyRedirects { } } - /** * When possible, merges the primary outlet child into the parent `UrlSegmentGroup`. * diff --git a/packages/router/src/recognize.ts b/packages/router/src/recognize.ts index d55513beb2..a3568d1b1c 100644 --- a/packages/router/src/recognize.ts +++ b/packages/router/src/recognize.ts @@ -14,7 +14,7 @@ import {ActivatedRouteSnapshot, inheritedParamsDataResolve, ParamsInheritanceStr import {PRIMARY_OUTLET} from './shared'; import {UrlSegment, UrlSegmentGroup, UrlTree} from './url_tree'; import {last} from './utils/collection'; -import {getOutlet} from './utils/config'; +import {getOutlet, sortByMatchingOutlets} from './utils/config'; import {isImmediateMatch, match, noLeftoversInUrl, split} from './utils/config_matching'; import {TreeNode} from './utils/tree'; @@ -64,6 +64,8 @@ export class Recognizer { return null; } + // Use Object.freeze to prevent readers of the Router state from modifying it outside of a + // navigation, resulting in the router being out of sync with the browser. const root = new ActivatedRouteSnapshot( [], Object.freeze({}), Object.freeze({...this.urlTree.queryParams}), this.urlTree.fragment!, {}, PRIMARY_OUTLET, this.rootComponentType, null, this.urlTree.root, -1, {}); @@ -108,8 +110,7 @@ export class Recognizer { const child = segmentGroup.children[childOutlet]; // Sort the config so that routes with outlets that match the one being activated appear // first, followed by routes for other outlets, which might match if they have an empty path. - const sortedConfig = config.filter(r => getOutlet(r) === childOutlet); - sortedConfig.push(...config.filter(r => getOutlet(r) !== childOutlet)); + const sortedConfig = sortByMatchingOutlets(config, childOutlet); const outletChildren = this.processSegmentGroup(sortedConfig, child, childOutlet); if (outletChildren === null) { // Configs must match all segment children so because we did not find a match for this @@ -182,6 +183,9 @@ export class Recognizer { const {segmentGroup, slicedSegments} = split( rawSegment, consumedSegments, rawSlicedSegments, + // Filter out routes with redirectTo because we are trying to create activated route + // snapshots and don't handle redirects here. That should have been done in + // `applyRedirects`. childConfig.filter(c => c.redirectTo === undefined), this.relativeLinkResolution); if (slicedSegments.length === 0 && segmentGroup.hasChildren()) { @@ -234,6 +238,11 @@ function getChildConfig(route: Route): Route[] { return []; } +function hasEmptyPathConfig(node: TreeNode) { + const config = node.value.routeConfig; + return config && config.path === '' && config.redirectTo === undefined; +} + /** * Finds `TreeNode`s with matching empty path route configs and merges them into `TreeNode` with the * children from each duplicate. This is necessary because different outlets can match a single @@ -243,13 +252,8 @@ function mergeEmptyPathMatches(nodes: Array>): Array> { const result: Array> = []; - function hasEmptyConfig(node: TreeNode) { - const config = node.value.routeConfig; - return config && config.path === '' && config.redirectTo === undefined; - } - for (const node of nodes) { - if (!hasEmptyConfig(node)) { + if (!hasEmptyPathConfig(node)) { result.push(node); continue; } diff --git a/packages/router/src/utils/collection.ts b/packages/router/src/utils/collection.ts index f3a9359a6a..6c4991910b 100644 --- a/packages/router/src/utils/collection.ts +++ b/packages/router/src/utils/collection.ts @@ -83,32 +83,6 @@ export function forEach(map: {[key: string]: V}, callback: (v: V, k: strin } } -export function waitForMap( - obj: {[k: string]: A}, fn: (k: string, a: A) => Observable): Observable<{[k: string]: B}> { - if (Object.keys(obj).length === 0) { - return of({}); - } - - const waitHead: Observable[] = []; - const waitTail: Observable[] = []; - const res: {[k: string]: B} = {}; - - forEach(obj, (a: A, k: string) => { - const mapped = fn(k, a).pipe(map((r: B) => res[k] = r)); - if (k === PRIMARY_OUTLET) { - waitHead.push(mapped); - } else { - waitTail.push(mapped); - } - }); - - // Closure compiler has problem with using spread operator here. So we use "Array.concat". - // Note that we also need to cast the new promise because TypeScript cannot infer the type - // when calling the "of" function through "Function.apply" - return (of.apply(null, waitHead.concat(waitTail)) as Observable>) - .pipe(concatAll(), lastValue(), map(() => res)); -} - export function wrapIntoObservable(value: T|Promise|Observable): Observable { if (isObservable(value)) { return value; diff --git a/packages/router/src/utils/config.ts b/packages/router/src/utils/config.ts index ed45e9b151..fd284e5b4d 100644 --- a/packages/router/src/utils/config.ts +++ b/packages/router/src/utils/config.ts @@ -123,20 +123,17 @@ 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; } + +/** + * Sorts the `routes` such that the ones with an outlet matching `outletName` come first. + * The order of the configs is otherwise preserved. + */ +export function sortByMatchingOutlets(routes: Routes, outletName: string): Routes { + const sortedConfig = routes.filter(r => getOutlet(r) === outletName); + sortedConfig.push(...routes.filter(r => getOutlet(r) !== outletName)); + return sortedConfig; +} \ No newline at end of file diff --git a/packages/router/src/utils/config_matching.ts b/packages/router/src/utils/config_matching.ts index b52f2ea344..52e625e8a3 100644 --- a/packages/router/src/utils/config_matching.ts +++ b/packages/router/src/utils/config_matching.ts @@ -21,7 +21,7 @@ export interface MatchResult { positionalParamSegments: {[k: string]: UrlSegment}; } -const noMatch = { +const noMatch: MatchResult = { matched: false, consumedSegments: [], lastChild: 0, @@ -183,9 +183,8 @@ export function isImmediateMatch( } if (route.path === '**') { return true; - } else { - return match(rawSegment, route, segments).matched; } + return match(rawSegment, route, segments).matched; } export function noLeftoversInUrl( diff --git a/packages/router/test/apply_redirects.spec.ts b/packages/router/test/apply_redirects.spec.ts index 3b27bf1fa5..18dcf9718f 100644 --- a/packages/router/test/apply_redirects.spec.ts +++ b/packages/router/test/apply_redirects.spec.ts @@ -8,6 +8,8 @@ import {NgModuleRef} from '@angular/core'; import {fakeAsync, TestBed, tick} from '@angular/core/testing'; +import {ActivatedRouteSnapshot} from '@angular/router'; +import {TreeNode} from '@angular/router/src/utils/tree'; import {Observable, of} from 'rxjs'; import {delay, tap} from 'rxjs/operators'; @@ -504,10 +506,11 @@ describe('applyRedirects', () => { [{path: '', loadChildren: 'root'}, {path: '', loadChildren: 'aux', outlet: 'popup'}]; applyRedirects(testModule.injector, loader, serializer, tree(''), config).subscribe(); - expect(loadCalls).toBe(2); + expect(loadCalls).toBe(1); tick(100); expect(loaded).toEqual(['root']); - tick(100); + expect(loadCalls).toBe(2); + tick(200); expect(loaded).toEqual(['root', 'aux']); })); @@ -560,9 +563,8 @@ describe('applyRedirects', () => { applyRedirects(testModule.injector, loader, serializer, tree('(popup:modal)'), config) .subscribe(); tick(auxDelay); - expect(loaded).toEqual(['aux']); tick(rootDelay); - expect(loaded).toEqual(['aux', 'root']); + expect(loaded.sort()).toEqual(['aux', 'root'].sort()); })); }); @@ -681,6 +683,108 @@ describe('applyRedirects', () => { }); }); + describe('aux split after empty path parent', () => { + it('should work with non-empty auxiliary path', () => { + checkRedirect( + [{ + path: '', + children: [ + {path: 'a', component: ComponentA}, + {path: 'c', component: ComponentC, outlet: 'aux'}, + {path: 'b', redirectTo: 'c', outlet: 'aux'} + ] + }], + '(aux:b)', (t: UrlTree) => { + expectTreeToBe(t, '(aux:c)'); + }); + }); + + it('should work with empty auxiliary path', () => { + checkRedirect( + [{ + path: '', + children: [ + {path: 'a', component: ComponentA}, + {path: 'c', component: ComponentC, outlet: 'aux'}, + {path: '', redirectTo: 'c', outlet: 'aux'} + ] + }], + '', (t: UrlTree) => { + expectTreeToBe(t, '(aux:c)'); + }); + }); + + it('should work with empty auxiliary path and matching primary', () => { + checkRedirect( + [{ + path: '', + children: [ + {path: 'a', component: ComponentA}, + {path: 'c', component: ComponentC, outlet: 'aux'}, + {path: '', redirectTo: 'c', outlet: 'aux'} + ] + }], + 'a', (t: UrlTree) => { + expect(t.toString()).toEqual('/a(aux:c)'); + }); + }); + + it('should work with aux outlets adjacent to and children of empty path at once', () => { + checkRedirect( + [ + { + path: '', + component: ComponentA, + children: [{path: 'b', outlet: 'b', component: ComponentB}] + }, + {path: 'c', outlet: 'c', component: ComponentC} + ], + '(b:b//c:c)', (t: UrlTree) => { + expect(t.toString()).toEqual('/(b:b//c:c)'); + }); + }); + + + it('should work with children outlets within two levels of empty parents', () => { + checkRedirect( + [{ + path: '', + component: ComponentA, + children: [{ + path: '', + component: ComponentB, + children: [ + {path: 'd', outlet: 'aux', redirectTo: 'c'}, + {path: 'c', outlet: 'aux', component: ComponentC} + ] + }] + }], + '(aux:d)', (t: UrlTree) => { + expect(t.toString()).toEqual('/(aux:c)'); + }); + }); + + it('does not persist a primary segment beyond the boundary of a named outlet match', () => { + const config: Routes = [ + { + path: '', + component: ComponentA, + outlet: 'aux', + children: [{path: 'b', component: ComponentB, redirectTo: '/c'}] + }, + {path: 'c', component: ComponentC} + ]; + applyRedirects(testModule.injector, null!, serializer, tree('/b'), config) + .subscribe( + (_) => { + throw 'Should not be reached'; + }, + e => { + expect(e.message).toEqual(`Cannot match any routes. URL Segment: 'b'`); + }); + }); + }); + describe('split at the end (no right child)', () => { it('should create a new child (non-terminal)', () => { checkRedirect( @@ -844,7 +948,8 @@ describe('applyRedirects', () => { { path: '', children: [ - {path: '', component: ComponentA, outlet: 'aux'}, + {path: '', outlet: 'aux', redirectTo: 'b'}, + {path: 'b', component: ComponentA, outlet: 'aux'}, {path: '', redirectTo: 'b', pathMatch: 'full'}, {path: 'b', component: ComponentB}, ], @@ -852,13 +957,40 @@ describe('applyRedirects', () => { ]) .subscribe( (tree: UrlTree) => { - expect(tree.toString()).toEqual('/b'); + expect(tree.toString()).toEqual('/b(aux:b)'); + expect(tree.root.children['primary'].toString()).toEqual('b'); + expect(tree.root.children['aux']).toBeDefined(); + expect(tree.root.children['aux'].toString()).toEqual('b'); }, () => { fail('should not be reached'); }); }); + it('should prevent empty named outlets from appearing in leaves, resulting in odd tree url', + () => { + 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)'), diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts index cae9479ef0..038626f933 100644 --- a/packages/router/test/integration.spec.ts +++ b/packages/router/test/integration.spec.ts @@ -1095,6 +1095,22 @@ describe('Integration', () => { expect(fixture.nativeElement).toHaveText('team 22 [ user victor, right: simple ]'); }))); + it('should support secondary routes as child of empty path parent', + fakeAsync(inject([Router], (router: Router) => { + const fixture = createRoot(router, RootCmp); + + router.resetConfig([{ + path: '', + component: TeamCmp, + children: [{path: 'simple', component: SimpleCmp, outlet: 'right'}] + }]); + + router.navigateByUrl('/(right:simple)'); + advance(fixture); + + expect(fixture.nativeElement).toHaveText('team [ , right: simple ]'); + }))); + it('should deactivate outlets', fakeAsync(inject([Router], (router: Router) => { const fixture = createRoot(router, RootCmp); @@ -1688,13 +1704,14 @@ describe('Integration', () => { data: {one: 1}, resolve: {two: 'resolveTwo'}, children: [ - {path: '', data: {three: 3}, resolve: {four: 'resolveFour'}, component: RouteCmp}, { + {path: '', data: {three: 3}, resolve: {four: 'resolveFour'}, component: RouteCmp}, + { path: '', data: {five: 5}, resolve: {six: 'resolveSix'}, component: RouteCmp, outlet: 'right' - } + }, ] }]);