diff --git a/packages/router/src/create_url_tree.ts b/packages/router/src/create_url_tree.ts index 8a67e9349b7..62407cb01ef 100644 --- a/packages/router/src/create_url_tree.ts +++ b/packages/router/src/create_url_tree.ts @@ -388,6 +388,32 @@ function updateSegmentGroupChildren( } else { const outlets = getOutlets(commands); const children: {[key: string]: UrlSegmentGroup} = {}; + // If the set of commands does not apply anything to the primary outlet and the child segment is + // an empty path primary segment on its own, we want to skip applying the commands at this + // level. Imagine the following config: + // + // {path: '', children: [{path: '**', outlet: 'popup'}]}. + // + // Navigation to /(popup:a) will activate the child outlet correctly Given a follow-up + // navigation with commands + // ['/', {outlets: {'popup': 'b'}}], we _would not_ want to apply the outlet commands to the + // root segment because that would result in + // //(popup:a)(popup:b) since the outlet command got applied one level above where it appears in + // the `ActivatedRoute` rather than updating the existing one. + // + // Because empty paths do not appear in the URL segments and the fact that the segments used in + // the output `UrlTree` are squashed to eliminate these empty paths where possible + // https://github.com/angular/angular/blob/13f10de40e25c6900ca55bd83b36bd533dacfa9e/packages/router/src/url_tree.ts#L755 + // it can be hard to determine what is the right thing to do when applying commands to a + // `UrlSegmentGroup` that is created from an "unsquashed"/expanded `ActivatedRoute` tree. + // This code effectively "squashes" empty path primary routes when they have no siblings on + // the same level of the tree. + if (!outlets[PRIMARY_OUTLET] && segmentGroup.children[PRIMARY_OUTLET] && + segmentGroup.numberOfChildren === 1 && + segmentGroup.children[PRIMARY_OUTLET].segments.length === 0) { + return updateSegmentGroupChildren( + segmentGroup.children[PRIMARY_OUTLET], startIndex, commands); + } forEach(outlets, (commands, outlet) => { if (typeof commands === 'string') { diff --git a/packages/router/test/create_url_tree.spec.ts b/packages/router/test/create_url_tree.spec.ts index a8c140afac2..33720d64185 100644 --- a/packages/router/test/create_url_tree.spec.ts +++ b/packages/router/test/create_url_tree.spec.ts @@ -115,31 +115,30 @@ describe('createUrlTree', async () => { expect(serializer.serialize(t)).toEqual('/%2Fone/two%2Fthree'); }); - it('should preserve secondary segments', async () => { - const p = serializer.parse('/a/11/b(right:c)'); - const t = await createRoot(p, ['/a', 11, 'd']); - expect(serializer.serialize(t)).toEqual('/a/11/d(right:c)'); - }); + describe('named outlets', async () => { + it('should preserve secondary segments', async () => { + const p = serializer.parse('/a/11/b(right:c)'); + const t = await createRoot(p, ['/a', 11, 'd']); + expect(serializer.serialize(t)).toEqual('/a/11/d(right:c)'); + }); - it('should support updating secondary segments (absolute)', async () => { - const p = serializer.parse('/a(right:b)'); - const t = await createRoot(p, ['/', {outlets: {right: ['c']}}]); - expect(serializer.serialize(t)).toEqual('/a(right:c)'); - }); + it('should support updating secondary segments (absolute)', async () => { + const p = serializer.parse('/a(right:b)'); + const t = await createRoot(p, ['/', {outlets: {right: ['c']}}]); + expect(serializer.serialize(t)).toEqual('/a(right:c)'); + }); - it('should support updating secondary segments', async () => { - const p = serializer.parse('/a(right:b)'); - const t = await createRoot(p, [{outlets: {right: ['c', 11, 'd']}}]); - expect(serializer.serialize(t)).toEqual('/a(right:c/11/d)'); - }); + it('should support updating secondary segments', async () => { + const p = serializer.parse('/a(right:b)'); + const t = await createRoot(p, [{outlets: {right: ['c', 11, 'd']}}]); + expect(serializer.serialize(t)).toEqual('/a(right:c/11/d)'); + }); - it('should support updating secondary segments (nested case)', async () => { - const p = serializer.parse('/a/(b//right:c)'); - const t = await createRoot(p, ['a', {outlets: {right: ['d', 11, 'e']}}]); - expect(serializer.serialize(t)).toEqual('/a/(b//right:d/11/e)'); - }); - - describe('', async () => { + it('should support updating secondary segments (nested case)', async () => { + const p = serializer.parse('/a/(b//right:c)'); + const t = await createRoot(p, ['a', {outlets: {right: ['d', 11, 'e']}}]); + expect(serializer.serialize(t)).toEqual('/a/(b//right:d/11/e)'); + }); it('should support removing secondary outlet with prefix', async () => { const p = serializer.parse('/parent/(child//secondary:popup)'); const t = await createRoot(p, ['parent', {outlets: {secondary: null}}]); @@ -206,6 +205,106 @@ describe('createUrlTree', async () => { const t = await createRoot(p, ['parent', {outlets: {primary: 'child', secondary: null}}]); expect(serializer.serialize(t)).toEqual('/parent/child(rootSecondary:rootPopup)'); }); + + describe('absolute navigations', () => { + it('with and pathless root', async () => { + router.resetConfig([ + { + path: '', + children: [ + {path: '**', outlet: 'left', component: class {}}, + ], + }, + ]); + await router.navigateByUrl('(left:search)'); + expect(router.url).toEqual('/(left:search)'); + expect(router.createUrlTree(['/', {outlets: {'left': ['projects', '123']}}]).toString()) + .toEqual('/(left:projects/123)'); + }); + it('empty path parent and sibling with a path', async () => { + router.resetConfig([ + { + path: '', + children: [ + {path: 'x', component: class {}}, + {path: '**', outlet: 'left', component: class {}}, + ], + }, + ]); + await router.navigateByUrl('/x(left:search)'); + expect(router.url).toEqual('/x(left:search)'); + expect(router.createUrlTree(['/', {outlets: {'left': ['projects', '123']}}]).toString()) + .toEqual('/x(left:projects/123)'); + // TODO(atscott): router.createUrlTree uses the "legacy" strategy based on the current + // UrlTree to generate new URLs. Once that changes, this can be `router.createUrlTree` + // again. + expect(createUrlTreeFromSnapshot( + router.routerState.root.snapshot, + [ + '/', { + outlets: { + 'primary': [{ + outlets: { + 'left': ['projects', '123'], + } + }] + } + } + ]) + .toString()) + .toEqual('/x(left:projects/123)'); + }); + + it('empty path parent and sibling', async () => { + router.resetConfig([ + { + path: '', + children: [ + {path: '', component: class {}}, + {path: '**', outlet: 'left', component: class {}}, + {path: '**', outlet: 'right', component: class {}}, + ], + }, + ]); + await router.navigateByUrl('/(left:search//right:define)'); + expect(router.url).toEqual('/(left:search//right:define)'); + expect(router.createUrlTree(['/', {outlets: {'left': ['projects', '123']}}]).toString()) + .toEqual('/(left:projects/123//right:define)'); + }); + it('two pathless parents', async () => { + router.resetConfig([ + { + path: '', + children: [{ + path: '', + children: [ + {path: '**', outlet: 'left', component: class {}}, + ] + }], + }, + ]); + await router.navigateByUrl('(left:search)'); + expect(router.url).toEqual('/(left:search)'); + expect(router.createUrlTree(['/', {outlets: {'left': ['projects', '123']}}]).toString()) + .toEqual('/(left:projects/123)'); + }); + + it('maintains structure when primary outlet is not pathless', async () => { + router.resetConfig([ + { + path: 'a', + children: [ + {path: '**', outlet: 'left', component: class {}}, + ], + }, + {path: '**', outlet: 'left', component: class {}}, + ]); + await router.navigateByUrl('/a/(left:search)'); + expect(router.url).toEqual('/a/(left:search)'); + expect(router.createUrlTree(['/', {outlets: {'left': ['projects', '123']}}]).toString()) + .toEqual('/a/(left:search)(left:projects/123)'); + }); + }); }); it('can navigate to nested route where commands is string', async () => {