From 5e5054d369ad68c9ac43e12439d71fb42d6ca26b Mon Sep 17 00:00:00 2001 From: Sean Perkins Date: Mon, 6 Dec 2021 17:25:40 -0500 Subject: [PATCH] fix(router): popping route now accounts for route params (#24315) --- .../components/router/test/matching.spec.tsx | 25 +++++--- core/src/components/router/utils/matching.ts | 62 ++++++++++++++++--- 2 files changed, 70 insertions(+), 17 deletions(-) diff --git a/core/src/components/router/test/matching.spec.tsx b/core/src/components/router/test/matching.spec.tsx index 41306df4b2..daa035cd9f 100644 --- a/core/src/components/router/test/matching.spec.tsx +++ b/core/src/components/router/test/matching.spec.tsx @@ -30,15 +30,24 @@ const CHAIN_3: RouteChain = [ describe('matchesIDs', () => { it('should match simple set of ids', () => { const chain: RouteChain = CHAIN_1; - expect(matchesIDs(['2'], chain)).toBe(1); - expect(matchesIDs(['2', '1'], chain)).toBe(2); - expect(matchesIDs(['2', '1', '3'], chain)).toBe(3); - expect(matchesIDs(['2', '1', '3', '4'], chain)).toBe(4); - expect(matchesIDs(['2', '1', '3', '4', '5'], chain)).toBe(4); + expect(matchesIDs([{ id: '2' }], chain)).toBe(1); + expect(matchesIDs([{ id: '2' }, { id: '1' }], chain)).toBe(2); + expect(matchesIDs([{ id: '2' }, { id: '1' }, { id: '3' }], chain)).toBe(3); + expect(matchesIDs([{ id: '2' }, { id: '1' }, { id: '3' }, { id: '4' }], chain)).toBe(4); + expect(matchesIDs([{ id: '2' }, { id: '1' }, { id: '3' }, { id: '4' }, { id: '5' }], chain)).toBe(4); expect(matchesIDs([], chain)).toBe(0); - expect(matchesIDs(['1'], chain)).toBe(0); + expect(matchesIDs([{ id: '1' }], chain)).toBe(0); }); + + it('should match path with params', () => { + const ids = [{ id: 'my-page', params: { s1: 'a', s2: 'b' } }]; + + expect(matchesIDs(ids, [{ id: 'my-page', path: [''], params: {} }])).toBe(1); + expect(matchesIDs(ids, [{ id: 'my-page', path: [':s1'], params: {} }])).toBe(1); + expect(matchesIDs(ids, [{ id: 'my-page', path: [':s1', ':s2'], params: {} }])).toBe(3); + expect(matchesIDs(ids, [{ id: 'my-page', path: [':s1', ':s2', ':s3'], params: {} }])).toBe(1); + }) }); describe('matchesPath', () => { @@ -227,7 +236,7 @@ describe('mergeParams', () => { }); describe('RouterSegments', () => { - it ('should initialize with empty array', () => { + it('should initialize with empty array', () => { const s = new RouterSegments([]); expect(s.next()).toEqual(''); expect(s.next()).toEqual(''); @@ -236,7 +245,7 @@ describe('RouterSegments', () => { expect(s.next()).toEqual(''); }); - it ('should initialize with array', () => { + it('should initialize with array', () => { const s = new RouterSegments(['', 'path', 'to', 'destination']); expect(s.next()).toEqual(''); expect(s.next()).toEqual('path'); diff --git a/core/src/components/router/utils/matching.ts b/core/src/components/router/utils/matching.ts index a975c7786f..9335bae388 100644 --- a/core/src/components/router/utils/matching.ts +++ b/core/src/components/router/utils/matching.ts @@ -32,16 +32,60 @@ export const findRouteRedirect = (path: string[], redirects: RouteRedirect[]) => return redirects.find(redirect => matchesRedirect(path, redirect)); }; -export const matchesIDs = (ids: string[], chain: RouteChain): number => { +export const matchesIDs = (ids: Pick[], chain: RouteChain): number => { const len = Math.min(ids.length, chain.length); - let i = 0; - for (; i < len; i++) { - if (ids[i].toLowerCase() !== chain[i].id) { + + let score = 0; + + for (let i = 0; i < len; i++) { + const routeId = ids[i]; + const routeChain = chain[i]; + // Skip results where the route id does not match the chain at the same index + if (routeId.id.toLowerCase() !== routeChain.id) { break; } + if (routeId.params) { + const routeIdParams = Object.keys(routeId.params); + /** + * Only compare routes with the chain that have the same number of parameters. + */ + if (routeIdParams.length === routeChain.path.length) { + /** + * Maps the route's params into a path based on the path variable names, + * to compare against the route chain format. + * + * Before: + * ```ts + * { + * params: { + * s1: 'a', + * s2: 'b' + * } + * } + * ``` + * + * After: + * ```ts + * [':s1',':s2'] + * ``` + */ + const pathWithParams = routeIdParams.map(key => `:${key}`); + for (let j = 0; j < pathWithParams.length; j++) { + // Skip results where the path variable is not a match + if (pathWithParams[j].toLowerCase() !== routeChain.path[j]) { + break; + } + // Weight path matches for the same index higher. + score++; + } + + } + } + // Weight id matches + score++; } - return i; -}; + return score; +} export const matchesPath = (inputPath: string[], chain: RouteChain): RouteChain | null => { const segments = new RouterSegments(inputPath); @@ -90,16 +134,16 @@ export const matchesPath = (inputPath: string[], chain: RouteChain): RouteChain // Merges the route parameter objects. // Returns undefined when both parameters are undefined. -export const mergeParams = (a: {[key: string]: any} | undefined, b: {[key: string]: any} | undefined): {[key: string]: any} | undefined => { +export const mergeParams = (a: { [key: string]: any } | undefined, b: { [key: string]: any } | undefined): { [key: string]: any } | undefined => { return a || b ? { ...a, ...b } : undefined; }; export const routerIDsToChain = (ids: RouteID[], chains: RouteChain[]): RouteChain | null => { let match: RouteChain | null = null; let maxMatches = 0; - const plainIDs = ids.map(i => i.id); + for (const chain of chains) { - const score = matchesIDs(plainIDs, chain); + const score = matchesIDs(ids, chain); if (score > maxMatches) { match = chain; maxMatches = score;