From d32d71f488d14d84ceb1d9cfcfced71587c60f0c Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Thu, 12 Jan 2023 11:53:23 -0800 Subject: [PATCH 1/2] test(router): update createUrlTree tests to not use mocks The mocks in the tests make them hard to understand and they don't always test real router behavior --- packages/router/test/create_url_tree.spec.ts | 382 +++++++++---------- 1 file changed, 177 insertions(+), 205 deletions(-) diff --git a/packages/router/test/create_url_tree.spec.ts b/packages/router/test/create_url_tree.spec.ts index 2909c847acb0..a8c140afac29 100644 --- a/packages/router/test/create_url_tree.spec.ts +++ b/packages/router/test/create_url_tree.spec.ts @@ -9,156 +9,140 @@ import {Component, Injectable} from '@angular/core'; import {ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; -import {BehaviorSubject} from 'rxjs'; -import {createUrlTree, createUrlTreeFromSnapshot} from '../src/create_url_tree'; +import {createUrlTreeFromSnapshot} from '../src/create_url_tree'; import {Routes} from '../src/models'; import {Router} from '../src/router'; import {RouterModule} from '../src/router_module'; -import {ActivatedRoute, ActivatedRouteSnapshot, advanceActivatedRoute} from '../src/router_state'; +import {ActivatedRoute, ActivatedRouteSnapshot} from '../src/router_state'; import {Params, PRIMARY_OUTLET} from '../src/shared'; -import {DefaultUrlSerializer, UrlSegmentGroup, UrlTree} from '../src/url_tree'; +import {DefaultUrlSerializer, UrlTree} from '../src/url_tree'; import {RouterTestingModule} from '../testing'; -describe('createUrlTree', () => { +describe('createUrlTree', async () => { const serializer = new DefaultUrlSerializer(); + let router: Router; + beforeEach(() => { + router = TestBed.inject(Router); + router.resetConfig([ + { + path: 'parent', + children: [ + {path: 'child', component: class {}}, + {path: '**', outlet: 'secondary', component: class {}}, + ] + }, + { + path: 'a', + children: [ + {path: '**', component: class {}}, + {path: '**', outlet: 'right', component: class {}}, + {path: '**', outlet: 'left', component: class {}}, + ] + }, + {path: '**', component: class {}}, + {path: '**', outlet: 'right', component: class {}}, + {path: '**', outlet: 'left', component: class {}}, + {path: '**', outlet: 'rootSecondary', component: class {}}, + ]); + }); - describe('query parameters', () => { - it('should support parameter with multiple values', () => { + describe('query parameters', async () => { + it('should support parameter with multiple values', async () => { const p1 = serializer.parse('/'); - const t1 = createRoot(p1, ['/'], {m: ['v1', 'v2']}); + const t1 = await createRoot(p1, ['/'], {m: ['v1', 'v2']}); expect(serializer.serialize(t1)).toEqual('/?m=v1&m=v2'); - const p2 = serializer.parse('/a/c'); - const t2 = create(p2.root.children[PRIMARY_OUTLET], 1, p2, ['c2'], {m: ['v1', 'v2']}); + await router.navigateByUrl('/a/c'); + const t2 = create(router.routerState.root.children[0].children[0], ['c2'], {m: ['v1', 'v2']}); expect(serializer.serialize(t2)).toEqual('/a/c/c2?m=v1&m=v2'); }); - it('should support parameter with empty arrays as values', () => { - const p1 = serializer.parse('/a/c'); - const t1 = create(p1.root.children[PRIMARY_OUTLET], 1, p1, ['c2'], {m: []}); + it('should support parameter with empty arrays as values', async () => { + await router.navigateByUrl('/a/c'); + const t1 = create(router.routerState.root.children[0].children[0], ['c2'], {m: []}); expect(serializer.serialize(t1)).toEqual('/a/c/c2'); - const p2 = serializer.parse('/a/c'); - const t2 = create(p2.root.children[PRIMARY_OUTLET], 1, p2, ['c2'], {m: [], n: 1}); + const t2 = create(router.routerState.root.children[0].children[0], ['c2'], {m: [], n: 1}); expect(serializer.serialize(t2)).toEqual('/a/c/c2?n=1'); }); - it('should set query params', () => { + it('should set query params', async () => { const p = serializer.parse('/'); - const t = createRoot(p, [], {a: 'hey'}); + const t = await createRoot(p, [], {a: 'hey'}); expect(t.queryParams).toEqual({a: 'hey'}); expect(t.queryParamMap.get('a')).toEqual('hey'); }); - it('should stringify query params', () => { + it('should stringify query params', async () => { const p = serializer.parse('/'); - const t = createRoot(p, [], {a: 1}); + const t = await createRoot(p, [], {a: 1}); expect(t.queryParams).toEqual({a: '1'}); expect(t.queryParamMap.get('a')).toEqual('1'); }); }); - it('should navigate to the root', () => { + it('should navigate to the root', async () => { const p = serializer.parse('/'); - const t = createRoot(p, ['/']); + const t = await createRoot(p, ['/']); expect(serializer.serialize(t)).toEqual('/'); }); - it('should error when navigating to the root segment with params', () => { + it('should error when navigating to the root segment with params', async () => { const p = serializer.parse('/'); - expect(() => createRoot(p, ['/', {p: 11}])) - .toThrowError(/Root segment cannot have matrix parameters/); + await expectAsync(createRoot(p, [ + '/', {p: 11} + ])).toBeRejectedWithError(/Root segment cannot have matrix parameters/); }); - it('should support nested segments', () => { + it('should support nested segments', async () => { const p = serializer.parse('/a/b'); - const t = createRoot(p, ['/one', 11, 'two', 22]); + const t = await createRoot(p, ['/one', 11, 'two', 22]); expect(serializer.serialize(t)).toEqual('/one/11/two/22'); }); - it('should stringify positional parameters', () => { + it('should stringify positional parameters', async () => { const p = serializer.parse('/a/b'); - const t = createRoot(p, ['/one', 11]); + const t = await createRoot(p, ['/one', 11]); const params = t.root.children[PRIMARY_OUTLET].segments; expect(params[0].path).toEqual('one'); expect(params[1].path).toEqual('11'); }); - it('should work if command = null', () => { - const p = serializer.parse('/a/b'); - const t = createRoot(p, [null]); - const params = t.root.children[PRIMARY_OUTLET].segments; - expect(params[0].path).toEqual('null'); - }); - - it('should work if command is undefined', () => { - const p = serializer.parse('/a/b'); - const t = createRoot(p, [undefined]); - const params = t.root.children[PRIMARY_OUTLET].segments; - expect(params[0].path).toEqual('undefined'); - }); - - it('should support first segments containing slashes', () => { + it('should support first segments containing slashes', async () => { const p = serializer.parse('/'); - const t = createRoot(p, [{segmentPath: '/one'}, 'two/three']); + const t = await createRoot(p, [{segmentPath: '/one'}, 'two/three']); expect(serializer.serialize(t)).toEqual('/%2Fone/two%2Fthree'); }); - it('should preserve secondary segments', () => { + it('should preserve secondary segments', async () => { const p = serializer.parse('/a/11/b(right:c)'); - const t = createRoot(p, ['/a', 11, 'd']); + 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)', () => { + it('should support updating secondary segments (absolute)', async () => { const p = serializer.parse('/a(right:b)'); - const t = createRoot(p, ['/', {outlets: {right: ['c']}}]); + const t = await createRoot(p, ['/', {outlets: {right: ['c']}}]); expect(serializer.serialize(t)).toEqual('/a(right:c)'); }); - it('should support updating secondary segments', () => { + it('should support updating secondary segments', async () => { const p = serializer.parse('/a(right:b)'); - const t = createRoot(p, [{outlets: {right: ['c', 11, 'd']}}]); + 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)', () => { + it('should support updating secondary segments (nested case)', async () => { const p = serializer.parse('/a/(b//right:c)'); - const t = createRoot(p, ['a', {outlets: {right: ['d', 11, 'e']}}]); + const t = await createRoot(p, ['a', {outlets: {right: ['d', 11, 'e']}}]); expect(serializer.serialize(t)).toEqual('/a/(b//right:d/11/e)'); }); - describe('', () => { - /** - * In this group of scenarios, imagine a config like: - * { - * path: 'parent', - * children: [ - * { - * path: 'child', - * component: AnyCmp - * }, - * { - * path: 'popup', - * outlet: 'secondary', - * component: AnyCmp - * } - * ] - * }, - * { - * path: 'other', - * component: AnyCmp - * }, - * { - * path: 'rootPopup', - * outlet: 'rootSecondary', - * } - */ - - it('should support removing secondary outlet with prefix', () => { + describe('', async () => { + it('should support removing secondary outlet with prefix', async () => { const p = serializer.parse('/parent/(child//secondary:popup)'); - const t = createRoot(p, ['parent', {outlets: {secondary: null}}]); + const t = await createRoot(p, ['parent', {outlets: {secondary: null}}]); // - Segment index 0: // * match and keep existing 'parent' // - Segment index 1: @@ -167,279 +151,267 @@ describe('createUrlTree', () => { expect(serializer.serialize(t)).toEqual('/parent/child'); }); - it('should support updating secondary and primary outlets with prefix', () => { + it('should support updating secondary and primary outlets with prefix', async () => { const p = serializer.parse('/parent/child'); - const t = createRoot(p, ['parent', {outlets: {primary: 'child', secondary: 'popup'}}]); + const t = await createRoot(p, ['parent', {outlets: {primary: 'child', secondary: 'popup'}}]); expect(serializer.serialize(t)).toEqual('/parent/(child//secondary:popup)'); }); - it('should support updating two outlets at the same time relative to non-root segment', () => { - const p = serializer.parse('/parent/child'); - const t = create( - p.root.children[PRIMARY_OUTLET], 0 /* relativeTo: 'parent' */, p, - [{outlets: {primary: 'child', secondary: 'popup'}}]); - expect(serializer.serialize(t)).toEqual('/parent/(child//secondary:popup)'); - }); + it('should support updating two outlets at the same time relative to non-root segment', + async () => { + await router.navigateByUrl('/parent/child'); + const t = create( + router.routerState.root.children[0], + [{outlets: {primary: 'child', secondary: 'popup'}}]); + expect(serializer.serialize(t)).toEqual('/parent/(child//secondary:popup)'); + }); - it('should support adding multiple outlets with prefix', () => { + it('should support adding multiple outlets with prefix', async () => { const p = serializer.parse(''); - const t = createRoot(p, ['parent', {outlets: {primary: 'child', secondary: 'popup'}}]); + const t = await createRoot(p, ['parent', {outlets: {primary: 'child', secondary: 'popup'}}]); expect(serializer.serialize(t)).toEqual('/parent/(child//secondary:popup)'); }); - it('should support updating clearing primary and secondary with prefix', () => { + it('should support updating clearing primary and secondary with prefix', async () => { const p = serializer.parse('/parent/(child//secondary:popup)'); - const t = createRoot(p, ['other']); + const t = await createRoot(p, ['other']); // Because we navigate away from the 'parent' route, the children of that route are cleared // because they are note valid for the 'other' path. expect(serializer.serialize(t)).toEqual('/other'); }); - it('should not clear secondary outlet when at root and prefix is used', () => { + it('should not clear secondary outlet when at root and prefix is used', async () => { const p = serializer.parse('/other(rootSecondary:rootPopup)'); - const t = createRoot(p, ['parent', {outlets: {primary: 'child', rootSecondary: null}}]); + const t = await createRoot(p, ['parent', {outlets: {primary: 'child', rootSecondary: null}}]); // We prefixed the navigation with 'parent' so we cannot clear the "rootSecondary" outlet // because once the outlets object is consumed, traversal is beyond the root segment. expect(serializer.serialize(t)).toEqual('/parent/child(rootSecondary:rootPopup)'); }); - it('should not clear non-root secondary outlet when command is targeting root', () => { + it('should not clear non-root secondary outlet when command is targeting root', async () => { const p = serializer.parse('/parent/(child//secondary:popup)'); - const t = createRoot(p, [{outlets: {secondary: null}}]); + const t = await createRoot(p, [{outlets: {secondary: null}}]); // The start segment index for the command is at 0, but the outlet lives at index 1 // so we cannot clear the outlet from processing segment index 0. expect(serializer.serialize(t)).toEqual('/parent/(child//secondary:popup)'); }); - it('can clear an auxiliary outlet at the correct segment level', () => { + it('can clear an auxiliary outlet at the correct segment level', async () => { const p = serializer.parse('/parent/(child//secondary:popup)(rootSecondary:rootPopup)'); // ^^^^^^^^^^^^^^^^^^^^^^ // The parens here show that 'child' and 'secondary:popup' appear at the same 'level' in the // config, i.e. are part of the same children list. You can also imagine an implicit paren // group around the whole URL to visualize how 'parent' and 'rootSecondary:rootPopup' are also // defined at the same level. - const t = createRoot(p, ['parent', {outlets: {primary: 'child', secondary: null}}]); + const t = await createRoot(p, ['parent', {outlets: {primary: 'child', secondary: null}}]); expect(serializer.serialize(t)).toEqual('/parent/child(rootSecondary:rootPopup)'); }); }); - it('can navigate to nested route where commands is string', () => { + it('can navigate to nested route where commands is string', async () => { const p = serializer.parse('/'); - const t = createRoot( + const t = await createRoot( p, ['/', {outlets: {primary: ['child', {outlets: {primary: 'nested-primary'}}]}}]); expect(serializer.serialize(t)).toEqual('/child/nested-primary'); }); - it('should throw when outlets is not the last command', () => { + it('should throw when outlets is not the last command', async () => { const p = serializer.parse('/a'); - expect(() => createRoot(p, ['a', {outlets: {right: ['c']}}, 'c'])).toThrowError(); + await expectAsync(createRoot(p, ['a', {outlets: {right: ['c']}}, 'c'])).toBeRejected(); }); - it('should support updating using a string', () => { + it('should support updating using a string', async () => { const p = serializer.parse('/a(right:b)'); - const t = createRoot(p, [{outlets: {right: 'c/11/d'}}]); + const t = await createRoot(p, [{outlets: {right: 'c/11/d'}}]); expect(serializer.serialize(t)).toEqual('/a(right:c/11/d)'); }); - it('should support updating primary and secondary segments at once', () => { + it('should support updating primary and secondary segments at once', async () => { const p = serializer.parse('/a(right:b)'); - const t = createRoot(p, [{outlets: {primary: 'y/z', right: 'c/11/d'}}]); + const t = await createRoot(p, [{outlets: {primary: 'y/z', right: 'c/11/d'}}]); expect(serializer.serialize(t)).toEqual('/y/z(right:c/11/d)'); }); - it('should support removing primary segment', () => { + it('should support removing primary segment', async () => { const p = serializer.parse('/a/(b//right:c)'); - const t = createRoot(p, ['a', {outlets: {primary: null, right: 'd'}}]); + const t = await createRoot(p, ['a', {outlets: {primary: null, right: 'd'}}]); expect(serializer.serialize(t)).toEqual('/a/(right:d)'); }); - it('should support removing secondary segments', () => { + it('should support removing secondary segments', async () => { const p = serializer.parse('/a(right:b)'); - const t = createRoot(p, [{outlets: {right: null}}]); + const t = await createRoot(p, [{outlets: {right: null}}]); expect(serializer.serialize(t)).toEqual('/a'); }); - it('should support removing parenthesis for primary segment on second path element', () => { + it('should support removing parenthesis for primary segment on second path element', async () => { const p = serializer.parse('/a/(b//right:c)'); - const t = createRoot(p, ['a', {outlets: {right: null}}]); + const t = await createRoot(p, ['a', {outlets: {right: null}}]); expect(serializer.serialize(t)).toEqual('/a/b'); }); - it('should update matrix parameters', () => { + it('should update matrix parameters', async () => { const p = serializer.parse('/a;pp=11'); - const t = createRoot(p, ['/a', {pp: 22, dd: 33}]); + const t = await createRoot(p, ['/a', {pp: 22, dd: 33}]); expect(serializer.serialize(t)).toEqual('/a;pp=22;dd=33'); }); - it('should create matrix parameters', () => { + it('should create matrix parameters', async () => { const p = serializer.parse('/a'); - const t = createRoot(p, ['/a', {pp: 22, dd: 33}]); + const t = await createRoot(p, ['/a', {pp: 22, dd: 33}]); expect(serializer.serialize(t)).toEqual('/a;pp=22;dd=33'); }); - it('should create matrix parameters together with other segments', () => { + it('should create matrix parameters together with other segments', async () => { const p = serializer.parse('/a'); - const t = createRoot(p, ['/a', 'b', {aa: 22, bb: 33}]); + const t = await createRoot(p, ['/a', 'b', {aa: 22, bb: 33}]); expect(serializer.serialize(t)).toEqual('/a/b;aa=22;bb=33'); }); - it('should stringify matrix parameters', () => { - const pr = serializer.parse('/r'); - const relative = create(pr.root.children[PRIMARY_OUTLET], 0, pr, [{pp: 22}]); + it('should stringify matrix parameters', async () => { + await router.navigateByUrl('/a'); + const relative = create(router.routerState.root.children[0], [{pp: 22}]); const segmentR = relative.root.children[PRIMARY_OUTLET].segments[0]; expect(segmentR.parameterMap.get('pp')).toEqual('22'); const pa = serializer.parse('/a'); - const absolute = createRoot(pa, ['/b', {pp: 33}]); + const absolute = await createRoot(pa, ['/b', {pp: 33}]); const segmentA = absolute.root.children[PRIMARY_OUTLET].segments[0]; expect(segmentA.parameterMap.get('pp')).toEqual('33'); }); - describe('relative navigation', () => { - it('should work', () => { - const p = serializer.parse('/a/(c//left:cp)(left:ap)'); - const t = create(p.root.children[PRIMARY_OUTLET], 0, p, ['c2']); + describe('relative navigation', async () => { + it('should work', async () => { + await router.navigateByUrl('/a/(c//left:cp)(left:ap)'); + const t = create(router.routerState.root.children[0], ['c2']); expect(serializer.serialize(t)).toEqual('/a/(c2//left:cp)(left:ap)'); }); - it('should work when the first command starts with a ./', () => { - const p = serializer.parse('/a/(c//left:cp)(left:ap)'); - const t = create(p.root.children[PRIMARY_OUTLET], 0, p, ['./c2']); + it('should work when the first command starts with a ./', async () => { + await router.navigateByUrl('/a/(c//left:cp)(left:ap)'); + const t = create(router.routerState.root.children[0], ['./c2']); expect(serializer.serialize(t)).toEqual('/a/(c2//left:cp)(left:ap)'); }); - it('should work when the first command is ./)', () => { - const p = serializer.parse('/a/(c//left:cp)(left:ap)'); - const t = create(p.root.children[PRIMARY_OUTLET], 0, p, ['./', 'c2']); + it('should work when the first command is ./)', async () => { + await router.navigateByUrl('/a/(c//left:cp)(left:ap)'); + const t = create(router.routerState.root.children[0], ['./', 'c2']); expect(serializer.serialize(t)).toEqual('/a/(c2//left:cp)(left:ap)'); }); - it('should support parameters-only navigation', () => { - const p = serializer.parse('/a'); - const t = create(p.root.children[PRIMARY_OUTLET], 0, p, [{k: 99}]); + it('should support parameters-only navigation', async () => { + await router.navigateByUrl('/a'); + const t = create(router.routerState.root.children[0], [{k: 99}]); expect(serializer.serialize(t)).toEqual('/a;k=99'); }); - it('should support parameters-only navigation (nested case)', () => { - const p = serializer.parse('/a/(c//left:cp)(left:ap)'); - const t = create(p.root.children[PRIMARY_OUTLET], 0, p, [{'x': 99}]); + it('should support parameters-only navigation (nested case)', async () => { + await router.navigateByUrl('/a/(c//left:cp)(left:ap)'); + const t = create(router.routerState.root.children[0], [{'x': 99}]); expect(serializer.serialize(t)).toEqual('/a;x=99(left:ap)'); }); - it('should support parameters-only navigation (with a double dot)', () => { - const p = serializer.parse('/a/(c//left:cp)(left:ap)'); - const t = - create(p.root.children[PRIMARY_OUTLET].children[PRIMARY_OUTLET], 0, p, ['../', {x: 5}]); + it('should support parameters-only navigation (with a double dot)', async () => { + await router.navigateByUrl('/a/(c//left:cp)(left:ap)'); + const t = createUrlTreeFromSnapshot( + router.routerState.root.children[0].children[0].snapshot, ['../', {x: 5}]); + // TODO(atscott): This will work when the router uses createUrlTreeFromSnapshot + // const t = create(router.routerState.root.children[0].children[0], ['../', {x: 5}]); expect(serializer.serialize(t)).toEqual('/a;x=5(left:ap)'); }); - it('should work when index > 0', () => { - const p = serializer.parse('/a/c'); - const t = create(p.root.children[PRIMARY_OUTLET], 1, p, ['c2']); + it('should work when index > 0', async () => { + await router.navigateByUrl('/a/c'); + const t = create(router.routerState.root.children[0].children[0], ['c2']); expect(serializer.serialize(t)).toEqual('/a/c/c2'); }); - it('should support going to a parent (within a segment)', () => { - const p = serializer.parse('/a/c'); - const t = create(p.root.children[PRIMARY_OUTLET], 1, p, ['../c2']); + it('should support going to a parent (within a segment)', async () => { + await router.navigateByUrl('/a/c'); + const t = create(router.routerState.root.children[0].children[0], ['../c2']); expect(serializer.serialize(t)).toEqual('/a/c2'); }); - it('should support going to a parent (across segments)', () => { - const p = serializer.parse('/q/(a/(c//left:cp)//left:qp)(left:ap)'); + it('should support going to a parent (across segments)', async () => { + await router.navigateByUrl('/q/(a/(c//left:cp)//left:qp)(left:ap)'); - const t = - create(p.root.children[PRIMARY_OUTLET].children[PRIMARY_OUTLET], 0, p, ['../../q2']); + const t = create(router.routerState.root.children[0].children[0], ['../../q2']); expect(serializer.serialize(t)).toEqual('/q2(left:ap)'); }); - it('should navigate to the root', () => { - const p = serializer.parse('/a/c'); - const t = create(p.root.children[PRIMARY_OUTLET], 0, p, ['../']); + it('should navigate to the root', async () => { + await router.navigateByUrl('/a/c'); + const t = create(router.routerState.root.children[0], ['../']); expect(serializer.serialize(t)).toEqual('/'); }); - it('should work with ../ when absolute url', () => { - const p = serializer.parse('/a/c'); - const t = create(p.root.children[PRIMARY_OUTLET], 1, p, ['../', 'c2']); + it('should work with ../ when absolute url', async () => { + await router.navigateByUrl('/a/c'); + const t = create(router.routerState.root.children[0].children[0], ['../', 'c2']); expect(serializer.serialize(t)).toEqual('/a/c2'); }); - it('should work with position = -1', () => { - const p = serializer.parse('/'); - const t = create(p.root, -1, p, ['11']); + it('should work relative to root', async () => { + await router.navigateByUrl('/'); + const t = create(router.routerState.root, ['11']); expect(serializer.serialize(t)).toEqual('/11'); }); - it('should throw when too many ..', () => { - const p = serializer.parse('/a/(c//left:cp)(left:ap)'); - expect(() => create(p.root.children[PRIMARY_OUTLET], 0, p, ['../../'])).toThrowError(); + it('should throw when too many ..', async () => { + await router.navigateByUrl('/a/(c//left:cp)(left:ap)'); + expect(() => create(router.routerState.root.children[0], ['../../'])).toThrowError(); }); - it('should support updating secondary segments', () => { - const p = serializer.parse('/a/b'); - const t = create(p.root.children[PRIMARY_OUTLET], 1, p, [{outlets: {right: ['c']}}]); + it('should support updating secondary segments', async () => { + await router.navigateByUrl('/a/b'); + const t = + create(router.routerState.root.children[0].children[0], [{outlets: {right: ['c']}}]); expect(serializer.serialize(t)).toEqual('/a/b/(right:c)'); }); }); - it('should set fragment', () => { + it('should set fragment', async () => { const p = serializer.parse('/'); - const t = createRoot(p, [], {}, 'fragment'); + const t = await createRoot(p, [], {}, 'fragment'); expect(t.fragment).toEqual('fragment'); }); - it('should support pathless route', () => { + it('should support pathless route', async () => { const p = serializer.parse('/a'); - const t = create(p.root.children[PRIMARY_OUTLET], -1, p, ['b']); + const t = create(router.routerState.root.children[0], ['b']); expect(serializer.serialize(t)).toEqual('/b'); }); - it('should support pathless route with ../ at root', () => { + it('should support pathless route with ../ at root', async () => { const p = serializer.parse('/a'); - const t = create(p.root.children[PRIMARY_OUTLET], -1, p, ['../b']); + const t = create(router.routerState.root.children[0], ['../b']); expect(serializer.serialize(t)).toEqual('/b'); }); - it('should support pathless child of pathless root', () => { - // i.e. routes = {path: '', loadChildren: () => import('child')...} - // forChild: {path: '', component: Comp} - const p = serializer.parse(''); - const empty = new UrlSegmentGroup([], {}); - p.root.children[PRIMARY_OUTLET] = empty; - empty.parent = p.root; - const t = create(empty, -1, p, ['lazy']); + it('should support pathless child of pathless root', async () => { + router.resetConfig([ + {path: '', children: [{path: '', component: class {}}, {path: 'lazy', component: class {}}]} + ]); + await router.navigateByUrl('/'); + const t = create(router.routerState.root.children[0].children[0], ['lazy']); expect(serializer.serialize(t)).toEqual('/lazy'); }); }); -function createRoot(tree: UrlTree, commands: any[], queryParams?: Params, fragment?: string) { - const s = new (ActivatedRouteSnapshot as any)( - [], {}, {}, '', {}, PRIMARY_OUTLET, 'someComponent', null, tree.root, -1, null); - const a = new (ActivatedRoute as any)( - new BehaviorSubject(null), new BehaviorSubject(null), new BehaviorSubject(null), - new BehaviorSubject(null), new BehaviorSubject(null), PRIMARY_OUTLET, 'someComponent', s); - advanceActivatedRoute(a); - return createUrlTree(a, tree, commands, queryParams ?? null, fragment ?? null); +async function createRoot( + tree: UrlTree, commands: any[], queryParams?: Params, fragment?: string): Promise { + const router = TestBed.inject(Router); + await router.navigateByUrl(tree); + return router.createUrlTree( + commands, {relativeTo: router.routerState.root, queryParams, fragment}); } function create( - segment: UrlSegmentGroup, startIndex: number, tree: UrlTree, commands: any[], - queryParams?: Params, fragment?: string) { - if (!segment) { - expect(segment).toBeDefined(); - } - const s = new (ActivatedRouteSnapshot as any)( - segment.segments, {}, {}, '', {}, PRIMARY_OUTLET, 'someComponent', null, segment, startIndex, - null); - const a = new (ActivatedRoute as any)( - new BehaviorSubject(null), new BehaviorSubject(null), new BehaviorSubject(null), - new BehaviorSubject(null), new BehaviorSubject(null), PRIMARY_OUTLET, 'someComponent', s); - advanceActivatedRoute(a); - return createUrlTree(a, tree, commands, queryParams ?? null, fragment ?? null); + relativeTo: ActivatedRoute, commands: any[], queryParams?: Params, fragment?: string) { + return TestBed.inject(Router).createUrlTree(commands, {relativeTo, queryParams, fragment}); } -describe('createUrlTreeFromSnapshot', () => { +describe('createUrlTreeFromSnapshot', async () => { it('can create a UrlTree relative to empty path named parent', fakeAsync(() => { @Component({ template: ``, From 13f63fa6e762d207451003efe2be3c191aa48764 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Fri, 13 Jan 2023 07:36:46 -0800 Subject: [PATCH 2/2] fix(router): 'createUrlTreeFromSnapshot' with empty paths and named outlets The details of this commit are pretty thoroughly described in the tests and code comments. In short, it is sometimes ambiguous where to apply commands in a `SegmentGroup` tree that is created from an `ActivatedRoute` when dealing with empty paths. This adjusts the strategy to tolerate more ambiguity while still allowing developers to be explicit. This is a fix-forward for b/265215141 --- packages/router/src/create_url_tree.ts | 26 ++++ packages/router/test/create_url_tree.spec.ts | 143 ++++++++++++++++--- 2 files changed, 147 insertions(+), 22 deletions(-) diff --git a/packages/router/src/create_url_tree.ts b/packages/router/src/create_url_tree.ts index 8a67e9349b74..62407cb01efb 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 a8c140afac29..33720d641854 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)'); - }); - - 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)'); - }); + 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', 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 (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 (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 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)'); + }); - 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 () => {