diff --git a/.changeset/neat-lions-peel.md b/.changeset/neat-lions-peel.md new file mode 100644 index 0000000000..1cd50641ec --- /dev/null +++ b/.changeset/neat-lions-peel.md @@ -0,0 +1,6 @@ +--- +'@backstage/core-api': patch +'@backstage/core': patch +--- + +Fixed a bug with `useRouteRef` where navigating from routes beneath a mount point would often fail. diff --git a/packages/core-api/src/routing/RouteResolver.test.ts b/packages/core-api/src/routing/RouteResolver.test.ts index e5394f6163..b46c5fc42a 100644 --- a/packages/core-api/src/routing/RouteResolver.test.ts +++ b/packages/core-api/src/routing/RouteResolver.test.ts @@ -19,9 +19,10 @@ import { createSubRouteRef } from './SubRouteRef'; import { createExternalRouteRef } from './ExternalRouteRef'; import { RouteResolver } from './RouteResolver'; import { ExternalRouteRef, RouteRef, SubRouteRef } from './types'; +import { MATCH_ALL_ROUTE } from './collectors'; const element = () => null; -const rest = { element, caseSensitive: false }; +const rest = { element, caseSensitive: false, children: [MATCH_ALL_ROUTE] }; const ref1 = createRouteRef({ id: 'rr1' }); const ref2 = createRouteRef({ id: 'rr2', params: ['x'] }); @@ -107,6 +108,7 @@ describe('RouteResolver', () => { path: '/my-parent/:x', ...rest, children: [ + MATCH_ALL_ROUTE, { routeRefs: new Set([ref1]), path: '/my-route', ...rest }, ], }, @@ -138,6 +140,59 @@ describe('RouteResolver', () => { ); }); + it('should resolve the most specific match', () => { + const r = new RouteResolver( + new Map([ + [ref1, '/deep'], + [ref2, '/root/:x'], + [ref3, '/sub/:y'], + ]), + new Map([ + [ref3, ref2], + [ref1, ref3], + ]), + [ + { + routeRefs: new Set([ref2]), + path: '/root/:x', + ...rest, + children: [ + MATCH_ALL_ROUTE, + { + routeRefs: new Set([ref3]), + path: '/sub/:y', + ...rest, + children: [ + MATCH_ALL_ROUTE, + { + routeRefs: new Set([ref1]), + path: '/deep', + ...rest, + }, + ], + }, + ], + }, + ], + new Map(), + ); + + expect(r.resolve(ref2, '/')?.({ x: 'x' })).toBe('/root/x'); + expect(r.resolve(ref3, '/root/x')?.({ y: 'y' })).toBe('/root/x/sub/y'); + + expect(() => r.resolve(ref1, '/')?.()).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(() => r.resolve(ref1, '/root/x')?.()).toThrow( + /^Cannot route.*with parent.*as it has parameters$/, + ); + expect(r.resolve(ref1, '/root/x/sub/y')?.()).toBe('/root/x/sub/y/deep'); + // Without the MATCH_ALL_ROUTE, we wouldn't properly match the route here + expect(r.resolve(ref1, '/root/x/sub/y/any/nested/path/here')?.()).toBe( + '/root/x/sub/y/deep', + ); + }); + it('should resolve an absolute route with multiple parents', () => { const r = new RouteResolver( new Map([ @@ -155,11 +210,13 @@ describe('RouteResolver', () => { path: '/my-grandparent/:y', ...rest, children: [ + MATCH_ALL_ROUTE, { routeRefs: new Set([ref2]), path: '/my-parent/:x', ...rest, children: [ + MATCH_ALL_ROUTE, { routeRefs: new Set([ref1]), path: '/my-route', ...rest }, ], }, diff --git a/packages/core-api/src/routing/collectors.test.tsx b/packages/core-api/src/routing/collectors.test.tsx index cd3e378fff..498d1dd9fc 100644 --- a/packages/core-api/src/routing/collectors.test.tsx +++ b/packages/core-api/src/routing/collectors.test.tsx @@ -90,13 +90,26 @@ function sortedEntries(map: Map): [RouteRef, T][] { ); } -function routeObj(path: string, refs: RouteRef[], children: any[] = []) { +function routeObj( + path: string, + refs: RouteRef[], + children: any[] = [], + type: 'mounted' | 'gathered' = 'mounted', +) { return { path: path, caseSensitive: false, - element: null, + element: type, routeRefs: new Set(refs), - children: children, + children: [ + { + path: '/*', + caseSensitive: false, + element: 'match-all', + routeRefs: new Set(), + }, + ...children, + ], }; } @@ -266,8 +279,12 @@ describe('discovery', () => { [ref5, ref3], ]); expect(routeObjects).toEqual([ - routeObj('/foo', [ref1, ref2]), - routeObj('/bar', [ref3], [routeObj('/baz', [ref4, ref5])]), + routeObj('/foo', [ref1, ref2], [], 'gathered'), + routeObj( + '/bar', + [ref3], + [routeObj('/baz', [ref4, ref5], [], 'gathered')], + ), ]); }); @@ -321,6 +338,7 @@ describe('discovery', () => { '/bar', [ref2, ref5], [routeObj('/baz', [ref3], [routeObj('/blop', [ref4])])], + 'gathered', ), ], ), diff --git a/packages/core-api/src/routing/collectors.tsx b/packages/core-api/src/routing/collectors.tsx index 37362cf8d4..9d651dbc0d 100644 --- a/packages/core-api/src/routing/collectors.tsx +++ b/packages/core-api/src/routing/collectors.tsx @@ -109,6 +109,17 @@ export const routeParentCollector = createCollector( }, ); +// We always add a child that matches all subroutes but without any route refs. This makes +// sure that we're always able to match each route no matter how deep the navigation goes. +// The route resolver then takes care of selecting the most specific match in order to find +// mount points that are as deep in the routing tree as possible. +export const MATCH_ALL_ROUTE: BackstageRouteObject = { + caseSensitive: false, + path: '/*', + element: 'match-all', // These elements aren't used, so we add in a bit of debug information + routeRefs: new Set(), +}; + export const routeObjectCollector = createCollector( () => Array(), (acc, node, parent, parentObj: BackstageRouteObject | undefined) => { @@ -126,9 +137,9 @@ export const routeObjectCollector = createCollector( const newObject: BackstageRouteObject = { caseSensitive, path, - element: null, + element: 'mounted', routeRefs: new Set([routeRef]), - children: [], + children: [MATCH_ALL_ROUTE], }; parentChildren.push(newObject); return newObject; @@ -148,9 +159,9 @@ export const routeObjectCollector = createCollector( const newObject: BackstageRouteObject = { caseSensitive, path, - element: null, + element: 'gathered', routeRefs: new Set(), - children: [], + children: [MATCH_ALL_ROUTE], }; parentChildren.push(newObject); return newObject;