core-api: fix a bug where resolving routes beneath a mount point would fail

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>

.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.

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<RouteRef, string>([
+        [ref1, '/deep'],
+        [ref2, '/root/:x'],
+        [ref3, '/sub/:y'],
+      ]),
+      new Map<RouteRef, RouteRef>([
+        [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<ExternalRouteRef, RouteRef | SubRouteRef>(),
+    );
+
+    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<RouteRef, string>([
@@ -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 },
               ],
             },

packages/core-api/src/routing/collectors.test.tsx

@@ -90,13 +90,26 @@ function sortedEntries<T>(map: Map<RouteRef, T>): [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',
           ),
         ],
       ),

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<BackstageRouteObject>(),
   (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;