From 3f2e59867154c7cd69b1d27de984f06171b15f37 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 29 Sep 2023 16:17:59 +0200 Subject: [PATCH 1/9] frontend-app-api: partial support for old routing system Co-authored-by: Camila Belo Co-authored-by: Philipp Hugenroth Signed-off-by: Patrik Oldsberg --- .../extractRouteInfoFromInstanceTree.test.ts | 467 ++++++++++++++++++ .../extractRouteInfoFromInstanceTree.ts | 67 +++ .../frontend-app-api/src/wiring/createApp.tsx | 53 +- 3 files changed, 547 insertions(+), 40 deletions(-) create mode 100644 packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts create mode 100644 packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts new file mode 100644 index 0000000000..24cf450fcd --- /dev/null +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts @@ -0,0 +1,467 @@ +/* + * Copyright 2020 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React from 'react'; +import { RouteRef, createRouteRef } from '@backstage/core-plugin-api'; +import { extractRouteInfoFromInstanceTree } from './extractRouteInfoFromInstanceTree'; +import { + coreExtensionData, + createExtensionInput, + createPageExtension, + createPlugin, +} from '@backstage/frontend-plugin-api'; +import { createInstances } from '../wiring/createApp'; +import { MockConfigApi } from '@backstage/test-utils'; + +const ref1 = createRouteRef({ id: 'page1' }); +const ref2 = createRouteRef({ id: 'page2' }); +const ref3 = createRouteRef({ id: 'page3' }); +const ref4 = createRouteRef({ id: 'page4' }); +const ref5 = createRouteRef({ id: 'page5' }); +const refOrder = [ref1, ref2, ref3, ref4, ref5]; + +function sortedEntries(map: Map): [RouteRef, T][] { + return Array.from(map).sort( + ([a], [b]) => refOrder.indexOf(a) - refOrder.indexOf(b), + ); +} + +// async function routeObj( +// path: string, +// refs: RouteRef[], +// children: any[] = [], +// type: 'mounted' | 'gathered' = 'mounted', +// backstagePlugin?: BackstagePlugin, +// ) { +// return { +// path: path, +// caseSensitive: false, +// element: type, +// routeRefs: new Set(refs), +// children: [ +// { +// path: '*', +// caseSensitive: false, +// element: 'match-all', +// routeRefs: new Set(), +// plugins: new Set(), +// }, +// ...children, +// ], +// plugins: backstagePlugin ? new Set([backstagePlugin]) : new Set(), +// }; +// } + +const emptyLoader = () => Promise.resolve(React.createElement('div')); + +describe('discovery', () => { + it('should collect routes', () => { + const extensions = [ + createPageExtension({ + id: 'nothing', + defaultPath: 'nothing', + loader: emptyLoader, + }), + createPageExtension({ + id: 'page1', + defaultPath: 'foo', + routeRef: ref1, + inputs: { + routes: createExtensionInput({ + element: coreExtensionData.reactElement, + }), + }, + loader: emptyLoader, + }), + createPageExtension({ + id: 'page2', + at: 'page1/routes', + defaultPath: 'bar/:id', + routeRef: ref2, + inputs: { + routes: createExtensionInput({ + element: coreExtensionData.reactElement, + }), + }, + loader: emptyLoader, + }), + createPageExtension({ + id: 'page3', + at: 'page2/routes', + defaultPath: 'baz', + routeRef: ref3, + loader: emptyLoader, + }), + createPageExtension({ + id: 'page4', + defaultPath: 'divsoup', + routeRef: ref4, + loader: emptyLoader, + }), + createPageExtension({ + id: 'page5', + at: 'page1/routes', + defaultPath: 'blop', + routeRef: ref5, + loader: emptyLoader, + }), + ]; + + const { rootInstances } = createInstances({ + config: new MockConfigApi({}), + plugins: [ + createPlugin({ + id: 'test', + extensions, + }), + ], + }); + + const info = extractRouteInfoFromInstanceTree(rootInstances); + + expect(sortedEntries(info.routePaths)).toEqual([ + [ref1, 'foo'], + [ref2, 'bar/:id'], + [ref3, 'baz'], + [ref4, 'divsoup'], + [ref5, 'blop'], + ]); + expect(sortedEntries(info.routeParents)).toEqual([ + [ref1, undefined], + [ref2, ref1], + [ref3, ref2], + [ref4, undefined], + [ref5, ref1], + ]); + // expect(routing.objects).toEqual([ + // routeObj( + // 'foo', + // [ref1], + // [ + // routeObj( + // 'bar/:id', + // [ref2], + // [routeObj('baz', [ref3], undefined, undefined, plugin)], + // undefined, + // plugin, + // ), + // routeObj('blop', [ref5], undefined, undefined, plugin), + // ], + // undefined, + // plugin, + // ), + // routeObj('divsoup', [ref4], undefined, undefined, plugin), + // ]); + }); + + // it('should handle all react router Route patterns', () => { + // const root = ( + // + // + // }> + // + // } /> + // + // + // }> + // } /> + // } /> + // + // + // + // ); + + // const { routing } = traverseElementTree({ + // root, + // discoverers: [childDiscoverer, routeElementDiscoverer], + // collectors: { + // routing: routingV2Collector, + // }, + // }); + // expect(sortedEntries(routing.paths)).toEqual([ + // [ref1, 'foo'], + // [ref2, 'bar/:id'], + // [ref3, 'baz'], + // [ref4, 'divsoup'], + // [ref5, 'blop'], + // ]); + // expect(sortedEntries(routing.parents)).toEqual([ + // [ref1, undefined], + // [ref2, ref1], + // [ref3, undefined], + // [ref4, ref3], + // [ref5, ref3], + // ]); + // }); + + // it('should handle absolute route paths', () => { + // const root = ( + // + // + // }> + // + // } /> + // + // + // }> + // } /> + // } /> + // + // + // + // ); + + // const { routing } = traverseElementTree({ + // root, + // discoverers: [childDiscoverer, routeElementDiscoverer], + // collectors: { + // routing: routingV2Collector, + // }, + // }); + // expect(sortedEntries(routing.paths)).toEqual([ + // [ref1, 'foo'], + // [ref2, 'bar/:id'], + // [ref3, 'baz'], + // [ref4, 'divsoup'], + // [ref5, 'blop'], + // ]); + // expect(sortedEntries(routing.parents)).toEqual([ + // [ref1, undefined], + // [ref2, ref1], + // [ref3, undefined], + // [ref4, ref3], + // [ref5, ref3], + // ]); + // }); + + // it('should use the route aggregator key to bind child routes to the same path', () => { + // const root = ( + // + // + // + // + //
+ // + //
+ // HELLO + //
+ // }> + // + // + // + // + // + // + //
+ //
+ // ); + + // const { routing } = traverseElementTree({ + // root, + // discoverers: [childDiscoverer, routeElementDiscoverer], + // collectors: { + // routing: routingV2Collector, + // }, + // }); + // expect(sortedEntries(routing.paths)).toEqual([ + // [ref1, 'foo'], + // [ref2, 'foo'], + // [ref3, 'bar'], + // [ref4, ''], + // [ref5, ''], + // ]); + // expect(sortedEntries(routing.parents)).toEqual([ + // [ref1, undefined], + // [ref2, undefined], + // [ref3, undefined], + // [ref4, ref3], + // [ref5, ref3], + // ]); + // expect(routing.objects).toEqual([ + // routeObj('foo', [ref1, ref2], [], 'gathered', plugin), + // routeObj( + // 'bar', + // [ref3], + // [routeObj('', [ref4, ref5], [], 'gathered', plugin)], + // undefined, + // plugin, + // ), + // ]); + // }); + + // it('should use the route aggregator but stop when encountering explicit path', () => { + // const root = ( + // + // + // }> + // + // + // + // }> + // } /> + // + // + // + // + // + // + // + // + // ); + + // const { routing } = traverseElementTree({ + // root, + // discoverers: [childDiscoverer, routeElementDiscoverer], + // collectors: { + // routing: routingV2Collector, + // }, + // }); + // expect(sortedEntries(routing.paths)).toEqual([ + // [ref1, 'foo'], + // [ref2, 'bar'], + // [ref3, 'baz'], + // [ref4, 'blop'], + // [ref5, 'bar'], + // ]); + // expect(sortedEntries(routing.parents)).toEqual([ + // [ref1, undefined], + // [ref2, ref1], + // [ref3, ref2], + // [ref4, ref3], + // [ref5, ref1], + // ]); + // expect(routing.objects).toEqual([ + // routeObj( + // 'foo', + // [ref1], + // [ + // routeObj( + // 'bar', + // [ref2, ref5], + // [ + // routeObj( + // 'baz', + // [ref3], + // [routeObj('blop', [ref4], undefined, undefined, plugin)], + // undefined, + // plugin, + // ), + // ], + // 'gathered', + // plugin, + // ), + // ], + // undefined, + // plugin, + // ), + // ]); + // }); + + // it('should throw when you provide path property on an extension', () => { + // expect(() => { + // traverseElementTree({ + // root: , + // discoverers: [childDiscoverer, routeElementDiscoverer], + // collectors: { + // routing: routingV2Collector, + // }, + // }); + // }).toThrow( + // 'Path property may not be set directly on a routable extension "Extension(Extension1)"', + // ); + // }); + + // it('should throw when element prop is not a string', () => { + // const Div = 'div' as unknown as ComponentType<{ path: boolean }>; + // expect(() => { + // traverseElementTree({ + // root:
, + // discoverers: [childDiscoverer, routeElementDiscoverer], + // collectors: { + // routing: routingV2Collector, + // }, + // }); + // }).toThrow('Element path must be a string at "div"'); + // }); + + // it('should throw when the mount point gatherers have an element prop', () => { + // const AnyAggregationComponent = AggregationComponent as any; + // expect(() => { + // traverseElementTree({ + // root: } />, + // discoverers: [childDiscoverer, routeElementDiscoverer], + // collectors: { + // routing: routingV2Collector, + // }, + // }); + // }).toThrow( + // 'Mount point gatherers may not have an element prop "AggregationComponent"', + // ); + // }); + + // it('should ignore path props within route elements', () => { + // const { routing } = traverseElementTree({ + // root: } />, + // discoverers: [childDiscoverer, routeElementDiscoverer], + // collectors: { + // routing: routingV2Collector, + // }, + // }); + // expect(sortedEntries(routing.paths)).toEqual([[ref1, 'foo']]); + // expect(sortedEntries(routing.parents)).toEqual([[ref1, undefined]]); + // expect(routing.objects).toEqual([ + // routeObj('foo', [ref1], [], undefined, plugin), + // ]); + // }); + + // it('should throw when a routable extension does not have a path set', () => { + // expect(() => { + // traverseElementTree({ + // root: , + // discoverers: [childDiscoverer, routeElementDiscoverer], + // collectors: { + // routing: routingV2Collector, + // }, + // }); + // }).toThrow( + // 'Routable extension "Extension(Extension3)" with mount point "routeRef{type=absolute,id=ref3}" must be assigned a path', + // ); + // }); + + // it('should throw when Route elements contain multiple routable extensions', () => { + // expect(() => { + // traverseElementTree({ + // root: ( + // + // + // + // + // } + // /> + // ), + // discoverers: [childDiscoverer, routeElementDiscoverer], + // collectors: { + // routing: routingV2Collector, + // }, + // }); + // }).toThrow( + // 'Route element with path "foo" may not contain multiple routable extensions', + // ); + // }); +}); diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts new file mode 100644 index 0000000000..290f5707e9 --- /dev/null +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts @@ -0,0 +1,67 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { RouteRef } from '@backstage/core-plugin-api'; +import { coreExtensionData } from '@backstage/frontend-plugin-api'; +import { ExtensionInstance } from '../wiring/createExtensionInstance'; +// eslint-disable-next-line @backstage/no-relative-monorepo-imports +import { BackstageRouteObject } from '../../../core-app-api/src/routing/types'; + +// Joins a list of paths together, avoiding trailing and duplicate slashes +export function joinPaths(...paths: string[]): string { + const normalized = paths.join('/').replace(/\/\/+/g, '/'); + if (normalized !== '/' && normalized.endsWith('/')) { + return normalized.slice(0, -1); + } + return normalized; +} + +export function extractRouteInfoFromInstanceTree(roots: ExtensionInstance[]): { + routePaths: Map; + routeParents: Map; + routeObjects: BackstageRouteObject[]; +} { + const routePaths = new Map(); + const routeParents = new Map(); + + function visit(current: ExtensionInstance, parent?: RouteRef) { + const routePath = current.getData(coreExtensionData.routePath) ?? ''; + const routeRef = current.getData(coreExtensionData.routeRef); + + if (routeRef) { + const routeRefId = (routeRef as any).id; // TODO: properly + if (routeRefId !== current.id) { + throw new Error( + `Route ref '${routeRefId}' must have the same ID as extension '${current.id}'`, + ); + } + routePaths.set(routeRef, routePath); + routeParents.set(routeRef, parent); + } + + for (const children of current.attachments.values()) { + for (const child of children) { + visit(child, routeRef ?? parent); + } + } + } + + for (const root of roots) { + visit(root); + } + + return { routePaths, routeParents, routeObjects: [] }; +} diff --git a/packages/frontend-app-api/src/wiring/createApp.tsx b/packages/frontend-app-api/src/wiring/createApp.tsx index deb19d919d..4afd1d1a2f 100644 --- a/packages/frontend-app-api/src/wiring/createApp.tsx +++ b/packages/frontend-app-api/src/wiring/createApp.tsx @@ -74,6 +74,8 @@ import { defaultConfigLoaderSync } from '../../../core-app-api/src/app/defaultCo // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { overrideBaseUrlConfigs } from '../../../core-app-api/src/app/overrideBaseUrlConfigs'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports +import { RoutingProvider as LegacyRoutingProvider } from '../../../core-app-api/src/routing/RoutingProvider'; +// eslint-disable-next-line @backstage/no-relative-monorepo-imports import { apis as defaultApis, components as defaultComponents, @@ -82,6 +84,7 @@ import { import { BrowserRouter, Route } from 'react-router-dom'; import { SidebarItem } from '@backstage/core-components'; import { DarkTheme, LightTheme } from '../extensions/themes'; +import { extractRouteInfoFromInstanceTree } from '../routing/extractRouteInfoFromInstanceTree'; /** @public */ export interface ExtensionTreeNode { @@ -281,7 +284,7 @@ export function createApp(options: { config, }); - const routePaths = extractRouteInfoFromInstanceTree(rootInstances); + const routeInfo = extractRouteInfoFromInstanceTree(rootInstances); const coreInstance = rootInstances.find(({ id }) => id === 'core'); if (!coreInstance) { @@ -304,10 +307,15 @@ export function createApp(options: { - - {/* TODO: set base path using the logic from AppRouter */} - {rootElements} - + + + {/* TODO: set base path using the logic from AppRouter */} + {rootElements} + + @@ -458,38 +466,3 @@ function createApiHolder( return new ApiResolver(factoryRegistry); } - -/** @internal */ -export function extractRouteInfoFromInstanceTree( - roots: ExtensionInstance[], -): Map { - const results = new Map(); - - function visit(current: ExtensionInstance, basePath: string) { - const routePath = current.getData(coreExtensionData.routePath) ?? ''; - const routeRef = current.getData(coreExtensionData.routeRef); - - // TODO: join paths in a more robust way - const fullPath = basePath + routePath; - if (routeRef) { - const routeRefId = (routeRef as any).id; // TODO: properly - if (routeRefId !== current.id) { - throw new Error( - `Route ref '${routeRefId}' must have the same ID as extension '${current.id}'`, - ); - } - results.set(routeRef, fullPath); - } - - for (const children of current.attachments.values()) { - for (const child of children) { - visit(child, fullPath); - } - } - } - - for (const root of roots) { - visit(root, ''); - } - return results; -} From 6fd3af691259dc4fbcdd4b8e53325640a88a028d Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 29 Sep 2023 16:53:47 +0200 Subject: [PATCH 2/9] frontend-app-api: add support for collecting route objects Co-authored-by: Camila Belo Co-authored-by: Philipp Hugenroth Signed-off-by: Patrik Oldsberg --- .../extractRouteInfoFromInstanceTree.test.ts | 108 +++++++++--------- .../extractRouteInfoFromInstanceTree.ts | 47 +++++++- .../frontend-app-api/src/wiring/createApp.tsx | 2 +- .../src/wiring/createExtensionInstance.ts | 4 +- 4 files changed, 102 insertions(+), 59 deletions(-) diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts index 24cf450fcd..b556662236 100644 --- a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts @@ -15,7 +15,11 @@ */ import React from 'react'; -import { RouteRef, createRouteRef } from '@backstage/core-plugin-api'; +import { + BackstagePlugin, + RouteRef, + createRouteRef, +} from '@backstage/core-plugin-api'; import { extractRouteInfoFromInstanceTree } from './extractRouteInfoFromInstanceTree'; import { coreExtensionData, @@ -23,7 +27,7 @@ import { createPageExtension, createPlugin, } from '@backstage/frontend-plugin-api'; -import { createInstances } from '../wiring/createApp'; +import { createInstances, toLegacyPlugin } from '../wiring/createApp'; import { MockConfigApi } from '@backstage/test-utils'; const ref1 = createRouteRef({ id: 'page1' }); @@ -39,31 +43,31 @@ function sortedEntries(map: Map): [RouteRef, T][] { ); } -// async function routeObj( -// path: string, -// refs: RouteRef[], -// children: any[] = [], -// type: 'mounted' | 'gathered' = 'mounted', -// backstagePlugin?: BackstagePlugin, -// ) { -// return { -// path: path, -// caseSensitive: false, -// element: type, -// routeRefs: new Set(refs), -// children: [ -// { -// path: '*', -// caseSensitive: false, -// element: 'match-all', -// routeRefs: new Set(), -// plugins: new Set(), -// }, -// ...children, -// ], -// plugins: backstagePlugin ? new Set([backstagePlugin]) : new Set(), -// }; -// } +function routeObj( + path: string, + refs: RouteRef[], + children: any[] = [], + type: 'mounted' | 'gathered' = 'mounted', + backstagePlugin?: BackstagePlugin, +) { + return { + path: path, + caseSensitive: false, + element: type, + routeRefs: new Set(refs), + children: [ + { + path: '*', + caseSensitive: false, + element: 'match-all', + routeRefs: new Set(), + plugins: new Set(), + }, + ...children, + ], + plugins: backstagePlugin ? new Set([backstagePlugin]) : new Set(), + }; +} const emptyLoader = () => Promise.resolve(React.createElement('div')); @@ -120,14 +124,13 @@ describe('discovery', () => { }), ]; + const plugin = createPlugin({ + id: 'test', + extensions, + }); const { rootInstances } = createInstances({ config: new MockConfigApi({}), - plugins: [ - createPlugin({ - id: 'test', - extensions, - }), - ], + plugins: [plugin], }); const info = extractRouteInfoFromInstanceTree(rootInstances); @@ -146,25 +149,26 @@ describe('discovery', () => { [ref4, undefined], [ref5, ref1], ]); - // expect(routing.objects).toEqual([ - // routeObj( - // 'foo', - // [ref1], - // [ - // routeObj( - // 'bar/:id', - // [ref2], - // [routeObj('baz', [ref3], undefined, undefined, plugin)], - // undefined, - // plugin, - // ), - // routeObj('blop', [ref5], undefined, undefined, plugin), - // ], - // undefined, - // plugin, - // ), - // routeObj('divsoup', [ref4], undefined, undefined, plugin), - // ]); + expect(info.routeObjects).toEqual([ + routeObj('nothing', []), + routeObj( + 'foo', + [ref1], + [ + routeObj( + 'bar/:id', + [ref2], + [routeObj('baz', [ref3], undefined, undefined, expect.any(Object))], + undefined, + expect.any(Object), + ), + routeObj('blop', [ref5], undefined, undefined, expect.any(Object)), + ], + undefined, + expect.any(Object), + ), + routeObj('divsoup', [ref4], undefined, undefined, expect.any(Object)), + ]); }); // it('should handle all react router Route patterns', () => { diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts index 290f5707e9..8d3947b970 100644 --- a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts @@ -19,6 +19,19 @@ import { coreExtensionData } from '@backstage/frontend-plugin-api'; import { ExtensionInstance } from '../wiring/createExtensionInstance'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { BackstageRouteObject } from '../../../core-app-api/src/routing/types'; +import { toLegacyPlugin } from '../wiring/createApp'; + +// 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(), + plugins: new Set(), +}; // Joins a list of paths together, avoiding trailing and duplicate slashes export function joinPaths(...paths: string[]): string { @@ -36,10 +49,30 @@ export function extractRouteInfoFromInstanceTree(roots: ExtensionInstance[]): { } { const routePaths = new Map(); const routeParents = new Map(); + const routeObjects = new Array(); - function visit(current: ExtensionInstance, parent?: RouteRef) { - const routePath = current.getData(coreExtensionData.routePath) ?? ''; + function visit( + current: ExtensionInstance, + parentRef?: RouteRef, + parentObj?: BackstageRouteObject, + ) { + const routePath = current.getData(coreExtensionData.routePath) ?? ''; // TODO: need to gather routes instead const routeRef = current.getData(coreExtensionData.routeRef); + const parentChildren = parentObj?.children ?? routeObjects; + let currentObj = parentObj; + + if (routePath) { + currentObj = { + path: routePath, + element: 'mounted', + routeRefs: new Set(), + caseSensitive: false, + children: [MATCH_ALL_ROUTE], + plugins: new Set(), + }; + + parentChildren.push(currentObj); + } if (routeRef) { const routeRefId = (routeRef as any).id; // TODO: properly @@ -49,12 +82,16 @@ export function extractRouteInfoFromInstanceTree(roots: ExtensionInstance[]): { ); } routePaths.set(routeRef, routePath); - routeParents.set(routeRef, parent); + routeParents.set(routeRef, parentRef); + currentObj?.routeRefs.add(routeRef); + if (current.source) { + currentObj?.plugins.add(toLegacyPlugin(current.source)); + } } for (const children of current.attachments.values()) { for (const child of children) { - visit(child, routeRef ?? parent); + visit(child, routeRef ?? parentRef, currentObj); } } } @@ -63,5 +100,5 @@ export function extractRouteInfoFromInstanceTree(roots: ExtensionInstance[]): { visit(root); } - return { routePaths, routeParents, routeObjects: [] }; + return { routePaths, routeParents, routeObjects }; } diff --git a/packages/frontend-app-api/src/wiring/createApp.tsx b/packages/frontend-app-api/src/wiring/createApp.tsx index 4afd1d1a2f..4f32bc2219 100644 --- a/packages/frontend-app-api/src/wiring/createApp.tsx +++ b/packages/frontend-app-api/src/wiring/createApp.tsx @@ -336,7 +336,7 @@ export function createApp(options: { }; } -function toLegacyPlugin(plugin: BackstagePlugin): LegacyBackstagePlugin { +export function toLegacyPlugin(plugin: BackstagePlugin): LegacyBackstagePlugin { const errorMsg = 'Not implemented in legacy plugin compatibility layer'; const notImplemented = () => { throw new Error(errorMsg); diff --git a/packages/frontend-app-api/src/wiring/createExtensionInstance.ts b/packages/frontend-app-api/src/wiring/createExtensionInstance.ts index afa3cbfefc..18551eb8da 100644 --- a/packages/frontend-app-api/src/wiring/createExtensionInstance.ts +++ b/packages/frontend-app-api/src/wiring/createExtensionInstance.ts @@ -36,6 +36,8 @@ export interface ExtensionInstance { * Maps input names to the actual instances given to them. */ readonly attachments: Map; + + readonly source?: BackstagePlugin; } function resolveInputData( @@ -141,7 +143,7 @@ export function createExtensionInstance(options: { getData(ref: ExtensionDataRef): T | undefined { return extensionData.get(ref.id) as T | undefined; }, - + source, attachments, }; } From 07c11f7598e31520f1d1cae9a5b57b1e05394434 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 1 Oct 2023 15:49:39 +0200 Subject: [PATCH 3/9] frontend-app-api: slim down route infro extraction test setup Signed-off-by: Patrik Oldsberg --- .../extractRouteInfoFromInstanceTree.test.ts | 66 ++++++++++--------- 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts index b556662236..1b7f12391c 100644 --- a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts @@ -27,7 +27,7 @@ import { createPageExtension, createPlugin, } from '@backstage/frontend-plugin-api'; -import { createInstances, toLegacyPlugin } from '../wiring/createApp'; +import { createInstances } from '../wiring/createApp'; import { MockConfigApi } from '@backstage/test-utils'; const ref1 = createRouteRef({ id: 'page1' }); @@ -37,6 +37,28 @@ const ref4 = createRouteRef({ id: 'page4' }); const ref5 = createRouteRef({ id: 'page5' }); const refOrder = [ref1, ref2, ref3, ref4, ref5]; +const emptyLoader = () => Promise.resolve(React.createElement('div')); + +function createTestExtension(options: { + id: string; + at?: string; + path: string; + routeRef?: RouteRef; +}) { + return createPageExtension({ + id: options.id, + at: options.at, + defaultPath: options.path, + routeRef: options.routeRef, + inputs: { + routes: createExtensionInput({ + element: coreExtensionData.reactElement, + }), + }, + loader: emptyLoader, + }); +} + function sortedEntries(map: Map): [RouteRef, T][] { return Array.from(map).sort( ([a], [b]) => refOrder.indexOf(a) - refOrder.indexOf(b), @@ -69,58 +91,40 @@ function routeObj( }; } -const emptyLoader = () => Promise.resolve(React.createElement('div')); - describe('discovery', () => { it('should collect routes', () => { const extensions = [ - createPageExtension({ + createTestExtension({ id: 'nothing', - defaultPath: 'nothing', - loader: emptyLoader, + path: 'nothing', }), - createPageExtension({ + createTestExtension({ id: 'page1', - defaultPath: 'foo', + path: 'foo', routeRef: ref1, - inputs: { - routes: createExtensionInput({ - element: coreExtensionData.reactElement, - }), - }, - loader: emptyLoader, }), - createPageExtension({ + createTestExtension({ id: 'page2', at: 'page1/routes', - defaultPath: 'bar/:id', + path: 'bar/:id', routeRef: ref2, - inputs: { - routes: createExtensionInput({ - element: coreExtensionData.reactElement, - }), - }, - loader: emptyLoader, }), - createPageExtension({ + createTestExtension({ id: 'page3', at: 'page2/routes', - defaultPath: 'baz', + path: 'baz', routeRef: ref3, - loader: emptyLoader, }), - createPageExtension({ + createTestExtension({ id: 'page4', - defaultPath: 'divsoup', + path: 'divsoup', routeRef: ref4, - loader: emptyLoader, }), - createPageExtension({ + createTestExtension({ id: 'page5', at: 'page1/routes', - defaultPath: 'blop', + path: 'blop', routeRef: ref5, - loader: emptyLoader, }), ]; From 261e6b31613c043731ab0ae8ad15d23cd9f1a6e8 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 1 Oct 2023 15:53:53 +0200 Subject: [PATCH 4/9] frontend-app-api: abstraction for route info extractino tests + another test Signed-off-by: Patrik Oldsberg --- .../extractRouteInfoFromInstanceTree.test.ts | 113 ++++++++++-------- 1 file changed, 62 insertions(+), 51 deletions(-) diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts index 1b7f12391c..fbc3c95fa8 100644 --- a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts @@ -22,6 +22,7 @@ import { } from '@backstage/core-plugin-api'; import { extractRouteInfoFromInstanceTree } from './extractRouteInfoFromInstanceTree'; import { + Extension, coreExtensionData, createExtensionInput, createPageExtension, @@ -59,6 +60,19 @@ function createTestExtension(options: { }); } +function routeInfoFromExtensions(extensions: Extension[]) { + const plugin = createPlugin({ + id: 'test', + extensions, + }); + const { rootInstances } = createInstances({ + config: new MockConfigApi({}), + plugins: [plugin], + }); + + return extractRouteInfoFromInstanceTree(rootInstances); +} + function sortedEntries(map: Map): [RouteRef, T][] { return Array.from(map).sort( ([a], [b]) => refOrder.indexOf(a) - refOrder.indexOf(b), @@ -93,7 +107,7 @@ function routeObj( describe('discovery', () => { it('should collect routes', () => { - const extensions = [ + const info = routeInfoFromExtensions([ createTestExtension({ id: 'nothing', path: 'nothing', @@ -126,18 +140,7 @@ describe('discovery', () => { path: 'blop', routeRef: ref5, }), - ]; - - const plugin = createPlugin({ - id: 'test', - extensions, - }); - const { rootInstances } = createInstances({ - config: new MockConfigApi({}), - plugins: [plugin], - }); - - const info = extractRouteInfoFromInstanceTree(rootInstances); + ]); expect(sortedEntries(info.routePaths)).toEqual([ [ref1, 'foo'], @@ -175,45 +178,53 @@ describe('discovery', () => { ]); }); - // it('should handle all react router Route patterns', () => { - // const root = ( - // - // - // }> - // - // } /> - // - // - // }> - // } /> - // } /> - // - // - // - // ); + it('should handle all react router Route patterns', () => { + const info = routeInfoFromExtensions([ + createTestExtension({ + id: 'page1', + path: 'foo', + routeRef: ref1, + }), + createTestExtension({ + id: 'page2', + at: 'page1/routes', + path: 'bar/:id', + routeRef: ref2, + }), + createTestExtension({ + id: 'page3', + path: 'baz', + routeRef: ref3, + }), + createTestExtension({ + id: 'page4', + at: 'page3/routes', + path: 'divsoup', + routeRef: ref4, + }), + createTestExtension({ + id: 'page5', + at: 'page3/routes', + path: 'blop', + routeRef: ref5, + }), + ]); - // const { routing } = traverseElementTree({ - // root, - // discoverers: [childDiscoverer, routeElementDiscoverer], - // collectors: { - // routing: routingV2Collector, - // }, - // }); - // expect(sortedEntries(routing.paths)).toEqual([ - // [ref1, 'foo'], - // [ref2, 'bar/:id'], - // [ref3, 'baz'], - // [ref4, 'divsoup'], - // [ref5, 'blop'], - // ]); - // expect(sortedEntries(routing.parents)).toEqual([ - // [ref1, undefined], - // [ref2, ref1], - // [ref3, undefined], - // [ref4, ref3], - // [ref5, ref3], - // ]); - // }); + expect(sortedEntries(info.routePaths)).toEqual([ + [ref1, 'foo'], + [ref2, 'bar/:id'], + [ref3, 'baz'], + [ref4, 'divsoup'], + [ref5, 'blop'], + ]); + expect(sortedEntries(info.routeParents)).toEqual([ + [ref1, undefined], + [ref2, ref1], + [ref3, undefined], + [ref4, ref3], + [ref5, ref3], + ]); + }); // it('should handle absolute route paths', () => { // const root = ( From 4cfc21d6eb28a25242ac6b5be41e533ebd4d7bbe Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 3 Oct 2023 15:46:44 +0200 Subject: [PATCH 5/9] frontend-app-api: improve legacy plugin conversion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: Vincenzo Scamporlino Co-authored-by: Camila Belo Signed-off-by: Patrik Oldsberg --- packages/frontend-app-api/package.json | 1 + .../frontend-app-api/src/wiring/createApp.tsx | 26 +++++++++++++++---- yarn.lock | 1 + 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/packages/frontend-app-api/package.json b/packages/frontend-app-api/package.json index 5fb84070d4..477cf8f736 100644 --- a/packages/frontend-app-api/package.json +++ b/packages/frontend-app-api/package.json @@ -42,6 +42,7 @@ "@backstage/plugin-graphiql": "workspace:^", "@backstage/theme": "workspace:^", "@backstage/types": "workspace:^", + "@backstage/version-bridge": "workspace:^", "@material-ui/core": "^4.12.4", "@material-ui/icons": "^4.11.3", "@types/react": "^16.13.1 || ^17.0.0", diff --git a/packages/frontend-app-api/src/wiring/createApp.tsx b/packages/frontend-app-api/src/wiring/createApp.tsx index 4f32bc2219..540b47849f 100644 --- a/packages/frontend-app-api/src/wiring/createApp.tsx +++ b/packages/frontend-app-api/src/wiring/createApp.tsx @@ -85,6 +85,7 @@ import { BrowserRouter, Route } from 'react-router-dom'; import { SidebarItem } from '@backstage/core-components'; import { DarkTheme, LightTheme } from '../extensions/themes'; import { extractRouteInfoFromInstanceTree } from '../routing/extractRouteInfoFromInstanceTree'; +import { getOrCreateGlobalSingleton } from '@backstage/version-bridge'; /** @public */ export interface ExtensionTreeNode { @@ -336,25 +337,40 @@ export function createApp(options: { }; } +// Make sure that we only convert each new plugin instance to its legacy equivalent once +const legacyPluginStore = getOrCreateGlobalSingleton( + 'legacy-plugin-compatibility-store', + () => new WeakMap(), +); + export function toLegacyPlugin(plugin: BackstagePlugin): LegacyBackstagePlugin { + let legacy = legacyPluginStore.get(plugin); + if (legacy) { + return legacy; + } + const errorMsg = 'Not implemented in legacy plugin compatibility layer'; const notImplemented = () => { throw new Error(errorMsg); }; - return { + + legacy = { getId(): string { return plugin.id; }, - get routes(): never { - throw new Error(errorMsg); + get routes() { + return {}; }, - get externalRoutes(): never { - throw new Error(errorMsg); + get externalRoutes() { + return {}; }, getApis: notImplemented, getFeatureFlags: notImplemented, provide: notImplemented, }; + + legacyPluginStore.set(plugin, legacy); + return legacy; } function createLegacyAppContext(plugins: BackstagePlugin[]): AppContext { diff --git a/yarn.lock b/yarn.lock index 8f0279aff3..37455d3e56 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4311,6 +4311,7 @@ __metadata: "@backstage/test-utils": "workspace:^" "@backstage/theme": "workspace:^" "@backstage/types": "workspace:^" + "@backstage/version-bridge": "workspace:^" "@material-ui/core": ^4.12.4 "@material-ui/icons": ^4.11.3 "@testing-library/jest-dom": ^5.10.1 From 7aad08e56403c6b956d6c90d6f4f498b89a294a1 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 3 Oct 2023 15:47:38 +0200 Subject: [PATCH 6/9] frontend-app-api: further routing info extraction improvements + test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: Camila Belo Co-authored-by: Vincenzo Scamporlino Signed-off-by: Patrik Oldsberg --- .../extractRouteInfoFromInstanceTree.test.ts | 217 ++++++++++-------- .../extractRouteInfoFromInstanceTree.ts | 32 ++- 2 files changed, 152 insertions(+), 97 deletions(-) diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts index fbc3c95fa8..b4e2675d51 100644 --- a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts @@ -24,8 +24,8 @@ import { extractRouteInfoFromInstanceTree } from './extractRouteInfoFromInstance import { Extension, coreExtensionData, + createExtension, createExtensionInput, - createPageExtension, createPlugin, } from '@backstage/frontend-plugin-api'; import { createInstances } from '../wiring/createApp'; @@ -38,25 +38,32 @@ const ref4 = createRouteRef({ id: 'page4' }); const ref5 = createRouteRef({ id: 'page5' }); const refOrder = [ref1, ref2, ref3, ref4, ref5]; -const emptyLoader = () => Promise.resolve(React.createElement('div')); - function createTestExtension(options: { id: string; at?: string; - path: string; + path?: string; routeRef?: RouteRef; }) { - return createPageExtension({ + return createExtension({ id: options.id, - at: options.at, - defaultPath: options.path, - routeRef: options.routeRef, + at: options.at ?? 'core.routes/children', + output: { + element: coreExtensionData.reactElement, + path: coreExtensionData.routePath.optional(), + routeRef: coreExtensionData.routeRef.optional(), + }, inputs: { - routes: createExtensionInput({ + children: createExtensionInput({ element: coreExtensionData.reactElement, }), }, - loader: emptyLoader, + factory({ bind }) { + bind({ + path: options.path, + routeRef: options.routeRef, + element: React.createElement('div'), + }); + }, }); } @@ -119,13 +126,13 @@ describe('discovery', () => { }), createTestExtension({ id: 'page2', - at: 'page1/routes', + at: 'page1/children', path: 'bar/:id', routeRef: ref2, }), createTestExtension({ id: 'page3', - at: 'page2/routes', + at: 'page2/children', path: 'baz', routeRef: ref3, }), @@ -136,7 +143,7 @@ describe('discovery', () => { }), createTestExtension({ id: 'page5', - at: 'page1/routes', + at: 'page1/children', path: 'blop', routeRef: ref5, }), @@ -187,7 +194,7 @@ describe('discovery', () => { }), createTestExtension({ id: 'page2', - at: 'page1/routes', + at: 'page1/children', path: 'bar/:id', routeRef: ref2, }), @@ -198,13 +205,13 @@ describe('discovery', () => { }), createTestExtension({ id: 'page4', - at: 'page3/routes', + at: 'page3/children', path: 'divsoup', routeRef: ref4, }), createTestExtension({ id: 'page5', - at: 'page3/routes', + at: 'page3/children', path: 'blop', routeRef: ref5, }), @@ -227,37 +234,45 @@ describe('discovery', () => { }); // it('should handle absolute route paths', () => { - // const root = ( - // - // - // }> - // - // } /> - // - // - // }> - // } /> - // } /> - // - // - // - // ); + // const info = routeInfoFromExtensions([ + // createTestExtension({ + // id: 'page1', + // path: '/foo', + // routeRef: ref1, + // }), + // createTestExtension({ + // id: 'page2', + // at: 'page1/children', + // path: '/bar/:id', + // routeRef: ref2, + // }), + // createTestExtension({ + // id: 'page3', + // path: '/baz', + // routeRef: ref3, + // }), + // createTestExtension({ + // id: 'page4', + // at: 'page3/children', + // path: '/divsoup', + // routeRef: ref4, + // }), + // createTestExtension({ + // id: 'page5', + // at: 'page3/children', + // path: '/blop', + // routeRef: ref5, + // }), + // ]); - // const { routing } = traverseElementTree({ - // root, - // discoverers: [childDiscoverer, routeElementDiscoverer], - // collectors: { - // routing: routingV2Collector, - // }, - // }); - // expect(sortedEntries(routing.paths)).toEqual([ + // expect(sortedEntries(info.routePaths)).toEqual([ // [ref1, 'foo'], // [ref2, 'bar/:id'], // [ref3, 'baz'], // [ref4, 'divsoup'], // [ref5, 'blop'], // ]); - // expect(sortedEntries(routing.parents)).toEqual([ + // expect(sortedEntries(info.routeParents)).toEqual([ // [ref1, undefined], // [ref2, ref1], // [ref3, undefined], @@ -266,60 +281,76 @@ describe('discovery', () => { // ]); // }); - // it('should use the route aggregator key to bind child routes to the same path', () => { - // const root = ( - // - // - // - // - //
- // - //
- // HELLO - //
- // }> - // - // - // - // - // - // - //
- //
- // ); + it('should use the route aggregator key to bind child routes to the same path', () => { + const info = routeInfoFromExtensions([ + createTestExtension({ + id: 'foo', + path: 'foo', + }), + createTestExtension({ + id: 'page1', + at: 'foo/children', + routeRef: ref1, + }), + createTestExtension({ + id: 'fooChild', + at: 'foo/children', + }), + createTestExtension({ + id: 'page2', + at: 'fooChild/children', + routeRef: ref2, + }), + createTestExtension({ + id: 'fooEmpty', + }), + createTestExtension({ + id: 'page3', + path: 'bar', + routeRef: ref3, + }), + createTestExtension({ + id: 'page3Child', + at: 'page3/children', + path: '', + }), + createTestExtension({ + id: 'page4', + at: 'page3Child/children', + routeRef: ref4, + }), + createTestExtension({ + id: 'page5', + at: 'page4/children', + routeRef: ref5, + }), + ]); - // const { routing } = traverseElementTree({ - // root, - // discoverers: [childDiscoverer, routeElementDiscoverer], - // collectors: { - // routing: routingV2Collector, - // }, - // }); - // expect(sortedEntries(routing.paths)).toEqual([ - // [ref1, 'foo'], - // [ref2, 'foo'], - // [ref3, 'bar'], - // [ref4, ''], - // [ref5, ''], - // ]); - // expect(sortedEntries(routing.parents)).toEqual([ - // [ref1, undefined], - // [ref2, undefined], - // [ref3, undefined], - // [ref4, ref3], - // [ref5, ref3], - // ]); - // expect(routing.objects).toEqual([ - // routeObj('foo', [ref1, ref2], [], 'gathered', plugin), - // routeObj( - // 'bar', - // [ref3], - // [routeObj('', [ref4, ref5], [], 'gathered', plugin)], - // undefined, - // plugin, - // ), - // ]); - // }); + expect(sortedEntries(info.routePaths)).toEqual([ + [ref1, 'foo'], + [ref2, 'foo'], + [ref3, 'bar'], + [ref4, ''], + [ref5, ''], + ]); + expect(sortedEntries(info.routeParents)).toEqual([ + [ref1, undefined], + [ref2, undefined], + [ref3, undefined], + [ref4, ref3], + [ref5, ref4], + ]); + expect(info.routeObjects).toEqual([ + routeObj('foo', [ref1, ref2], [], 'mounted', expect.any(Object)), + routeObj( + 'bar', + [ref3], + [routeObj('', [ref4, ref5], [], 'mounted', expect.any(Object))], + 'mounted', + expect.any(Object), + ), + ]); + }); // it('should use the route aggregator but stop when encountering explicit path', () => { // const root = ( diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts index 8d3947b970..2d6af97198 100644 --- a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts @@ -53,15 +53,19 @@ export function extractRouteInfoFromInstanceTree(roots: ExtensionInstance[]): { function visit( current: ExtensionInstance, + collectedPath?: string, + foundRefForCollectedPath: boolean = false, parentRef?: RouteRef, parentObj?: BackstageRouteObject, ) { - const routePath = current.getData(coreExtensionData.routePath) ?? ''; // TODO: need to gather routes instead + const routePath = current.getData(coreExtensionData.routePath); const routeRef = current.getData(coreExtensionData.routeRef); const parentChildren = parentObj?.children ?? routeObjects; let currentObj = parentObj; + let newCollectedPath = collectedPath; + let newFoundRefForCollectedPath = foundRefForCollectedPath; - if (routePath) { + if (routePath !== undefined) { currentObj = { path: routePath, element: 'mounted', @@ -72,6 +76,15 @@ export function extractRouteInfoFromInstanceTree(roots: ExtensionInstance[]): { }; parentChildren.push(currentObj); + + if (newFoundRefForCollectedPath) { + newCollectedPath = routePath; + newFoundRefForCollectedPath = false; + } else { + newCollectedPath = collectedPath + ? joinPaths(collectedPath, routePath) + : routePath; + } } if (routeRef) { @@ -81,7 +94,12 @@ export function extractRouteInfoFromInstanceTree(roots: ExtensionInstance[]): { `Route ref '${routeRefId}' must have the same ID as extension '${current.id}'`, ); } - routePaths.set(routeRef, routePath); + + if (newCollectedPath !== undefined) { + routePaths.set(routeRef, newCollectedPath); + newFoundRefForCollectedPath = true; + } + routeParents.set(routeRef, parentRef); currentObj?.routeRefs.add(routeRef); if (current.source) { @@ -91,7 +109,13 @@ export function extractRouteInfoFromInstanceTree(roots: ExtensionInstance[]): { for (const children of current.attachments.values()) { for (const child of children) { - visit(child, routeRef ?? parentRef, currentObj); + visit( + child, + newCollectedPath, + newFoundRefForCollectedPath, + routeRef ?? parentRef, + currentObj, + ); } } } From e92ad1c067372e0737a8960a2a53ecb23288e09a Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 3 Oct 2023 17:39:45 +0200 Subject: [PATCH 7/9] frontend-app-api: finish route info collection implementation and tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: Vincenzo Scamporlino Co-authored-by: Camila Belo Signed-off-by: Patrik Oldsberg --- .../extractRouteInfoFromInstanceTree.test.ts | 423 +++++++++--------- .../extractRouteInfoFromInstanceTree.ts | 19 +- 2 files changed, 233 insertions(+), 209 deletions(-) diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts index b4e2675d51..c64dc7a3b9 100644 --- a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts @@ -233,53 +233,53 @@ describe('discovery', () => { ]); }); - // it('should handle absolute route paths', () => { - // const info = routeInfoFromExtensions([ - // createTestExtension({ - // id: 'page1', - // path: '/foo', - // routeRef: ref1, - // }), - // createTestExtension({ - // id: 'page2', - // at: 'page1/children', - // path: '/bar/:id', - // routeRef: ref2, - // }), - // createTestExtension({ - // id: 'page3', - // path: '/baz', - // routeRef: ref3, - // }), - // createTestExtension({ - // id: 'page4', - // at: 'page3/children', - // path: '/divsoup', - // routeRef: ref4, - // }), - // createTestExtension({ - // id: 'page5', - // at: 'page3/children', - // path: '/blop', - // routeRef: ref5, - // }), - // ]); + it('should strip leading slashes in route paths', () => { + const info = routeInfoFromExtensions([ + createTestExtension({ + id: 'page1', + path: '/foo', + routeRef: ref1, + }), + createTestExtension({ + id: 'page2', + at: 'page1/children', + path: '/bar/:id', + routeRef: ref2, + }), + createTestExtension({ + id: 'page3', + path: '/baz', + routeRef: ref3, + }), + createTestExtension({ + id: 'page4', + at: 'page3/children', + path: '/divsoup', + routeRef: ref4, + }), + createTestExtension({ + id: 'page5', + at: 'page3/children', + path: '/blop', + routeRef: ref5, + }), + ]); - // expect(sortedEntries(info.routePaths)).toEqual([ - // [ref1, 'foo'], - // [ref2, 'bar/:id'], - // [ref3, 'baz'], - // [ref4, 'divsoup'], - // [ref5, 'blop'], - // ]); - // expect(sortedEntries(info.routeParents)).toEqual([ - // [ref1, undefined], - // [ref2, ref1], - // [ref3, undefined], - // [ref4, ref3], - // [ref5, ref3], - // ]); - // }); + expect(sortedEntries(info.routePaths)).toEqual([ + [ref1, 'foo'], + [ref2, 'bar/:id'], + [ref3, 'baz'], + [ref4, 'divsoup'], + [ref5, 'blop'], + ]); + expect(sortedEntries(info.routeParents)).toEqual([ + [ref1, undefined], + [ref2, ref1], + [ref3, undefined], + [ref4, ref3], + [ref5, ref3], + ]); + }); it('should use the route aggregator key to bind child routes to the same path', () => { const info = routeInfoFromExtensions([ @@ -338,7 +338,7 @@ describe('discovery', () => { [ref2, undefined], [ref3, undefined], [ref4, ref3], - [ref5, ref4], + [ref5, ref3], ]); expect(info.routeObjects).toEqual([ routeObj('foo', [ref1, ref2], [], 'mounted', expect.any(Object)), @@ -352,166 +352,177 @@ describe('discovery', () => { ]); }); - // it('should use the route aggregator but stop when encountering explicit path', () => { - // const root = ( - // - // - // }> - // - // - // - // }> - // } /> - // - // - // - // - // - // - // - // - // ); + it('should use the route aggregator but stop when encountering explicit path', () => { + const info = routeInfoFromExtensions([ + createTestExtension({ + id: 'page1', + path: 'foo', + routeRef: ref1, + }), + createTestExtension({ + id: 'page1Child', + at: 'page1/children', + path: 'bar', + }), + createTestExtension({ + id: 'page2', + at: 'page1Child/children', + routeRef: ref2, + }), + createTestExtension({ + id: 'page3', + at: 'page2/children', + path: 'baz', + routeRef: ref3, + }), + createTestExtension({ + id: 'page4', + at: 'page3/children', + path: '/blop', + routeRef: ref4, + }), + createTestExtension({ + id: 'page5', + at: 'page2/children', + routeRef: ref5, + }), + ]); - // const { routing } = traverseElementTree({ - // root, - // discoverers: [childDiscoverer, routeElementDiscoverer], - // collectors: { - // routing: routingV2Collector, - // }, - // }); - // expect(sortedEntries(routing.paths)).toEqual([ - // [ref1, 'foo'], - // [ref2, 'bar'], - // [ref3, 'baz'], - // [ref4, 'blop'], - // [ref5, 'bar'], - // ]); - // expect(sortedEntries(routing.parents)).toEqual([ - // [ref1, undefined], - // [ref2, ref1], - // [ref3, ref2], - // [ref4, ref3], - // [ref5, ref1], - // ]); - // expect(routing.objects).toEqual([ - // routeObj( - // 'foo', - // [ref1], - // [ - // routeObj( - // 'bar', - // [ref2, ref5], - // [ - // routeObj( - // 'baz', - // [ref3], - // [routeObj('blop', [ref4], undefined, undefined, plugin)], - // undefined, - // plugin, - // ), - // ], - // 'gathered', - // plugin, - // ), - // ], - // undefined, - // plugin, - // ), - // ]); - // }); + expect(sortedEntries(info.routePaths)).toEqual([ + [ref1, 'foo'], + [ref2, 'bar'], + [ref3, 'baz'], + [ref4, 'blop'], + [ref5, 'bar'], + ]); + expect(sortedEntries(info.routeParents)).toEqual([ + [ref1, undefined], + [ref2, ref1], + [ref3, ref2], + [ref4, ref3], + [ref5, ref1], + ]); + expect(info.routeObjects).toEqual([ + routeObj( + 'foo', + [ref1], + [ + routeObj( + 'bar', + [ref2, ref5], + [ + routeObj( + 'baz', + [ref3], + [ + routeObj( + 'blop', + [ref4], + undefined, + undefined, + expect.any(Object), + ), + ], + undefined, + expect.any(Object), + ), + ], + 'mounted', + expect.any(Object), + ), + ], + undefined, + expect.any(Object), + ), + ]); + }); - // it('should throw when you provide path property on an extension', () => { - // expect(() => { - // traverseElementTree({ - // root: , - // discoverers: [childDiscoverer, routeElementDiscoverer], - // collectors: { - // routing: routingV2Collector, - // }, - // }); - // }).toThrow( - // 'Path property may not be set directly on a routable extension "Extension(Extension1)"', - // ); - // }); + it('should account for loose route paths', () => { + const info = routeInfoFromExtensions([ + createTestExtension({ + id: 'r', + path: 'r', + }), + createTestExtension({ + id: 'page1', + at: 'r/children', + path: 'x', + routeRef: ref1, + }), + createTestExtension({ + id: 'y', + path: 'y', + at: 'r/children', + }), + createTestExtension({ + id: 'page2', + at: 'y/children', + path: '1', + routeRef: ref2, + }), + createTestExtension({ + id: 'page3', + at: 'page2/children', + path: 'a', + routeRef: ref3, + }), + createTestExtension({ + id: 'page4', + at: 'page2/children', + path: 'b', + routeRef: ref4, + }), + ]); - // it('should throw when element prop is not a string', () => { - // const Div = 'div' as unknown as ComponentType<{ path: boolean }>; - // expect(() => { - // traverseElementTree({ - // root:
, - // discoverers: [childDiscoverer, routeElementDiscoverer], - // collectors: { - // routing: routingV2Collector, - // }, - // }); - // }).toThrow('Element path must be a string at "div"'); - // }); - - // it('should throw when the mount point gatherers have an element prop', () => { - // const AnyAggregationComponent = AggregationComponent as any; - // expect(() => { - // traverseElementTree({ - // root: } />, - // discoverers: [childDiscoverer, routeElementDiscoverer], - // collectors: { - // routing: routingV2Collector, - // }, - // }); - // }).toThrow( - // 'Mount point gatherers may not have an element prop "AggregationComponent"', - // ); - // }); - - // it('should ignore path props within route elements', () => { - // const { routing } = traverseElementTree({ - // root: } />, - // discoverers: [childDiscoverer, routeElementDiscoverer], - // collectors: { - // routing: routingV2Collector, - // }, - // }); - // expect(sortedEntries(routing.paths)).toEqual([[ref1, 'foo']]); - // expect(sortedEntries(routing.parents)).toEqual([[ref1, undefined]]); - // expect(routing.objects).toEqual([ - // routeObj('foo', [ref1], [], undefined, plugin), - // ]); - // }); - - // it('should throw when a routable extension does not have a path set', () => { - // expect(() => { - // traverseElementTree({ - // root: , - // discoverers: [childDiscoverer, routeElementDiscoverer], - // collectors: { - // routing: routingV2Collector, - // }, - // }); - // }).toThrow( - // 'Routable extension "Extension(Extension3)" with mount point "routeRef{type=absolute,id=ref3}" must be assigned a path', - // ); - // }); - - // it('should throw when Route elements contain multiple routable extensions', () => { - // expect(() => { - // traverseElementTree({ - // root: ( - // - // - // - // - // } - // /> - // ), - // discoverers: [childDiscoverer, routeElementDiscoverer], - // collectors: { - // routing: routingV2Collector, - // }, - // }); - // }).toThrow( - // 'Route element with path "foo" may not contain multiple routable extensions', - // ); - // }); + expect(sortedEntries(info.routePaths)).toEqual([ + [ref1, 'r/x'], + [ref2, 'r/y/1'], + [ref3, 'a'], + [ref4, 'b'], + ]); + expect(sortedEntries(info.routeParents)).toEqual([ + [ref1, undefined], + [ref2, undefined], + [ref3, ref2], + [ref4, ref2], + ]); + expect(info.routeObjects).toEqual([ + routeObj( + 'r', + [], + [ + routeObj('x', [ref1], [], 'mounted', expect.any(Object)), + routeObj( + 'y', + [], + [ + routeObj( + '1', + [ref2], + [ + routeObj( + 'a', + [ref3], + undefined, + 'mounted', + expect.any(Object), + ), + routeObj( + 'b', + [ref4], + undefined, + 'mounted', + expect.any(Object), + ), + ], + 'mounted', + expect.any(Object), + ), + ], + 'mounted', + ), + ], + ), + ]); + }); }); diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts index 2d6af97198..a7df7978cc 100644 --- a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts @@ -56,14 +56,19 @@ export function extractRouteInfoFromInstanceTree(roots: ExtensionInstance[]): { collectedPath?: string, foundRefForCollectedPath: boolean = false, parentRef?: RouteRef, + candidateParentRef?: RouteRef, parentObj?: BackstageRouteObject, ) { - const routePath = current.getData(coreExtensionData.routePath); + const routePath = current + .getData(coreExtensionData.routePath) + ?.replace(/^\//, ''); const routeRef = current.getData(coreExtensionData.routeRef); const parentChildren = parentObj?.children ?? routeObjects; let currentObj = parentObj; let newCollectedPath = collectedPath; let newFoundRefForCollectedPath = foundRefForCollectedPath; + let newParentRef = parentRef; + let newCandidateParentRef = candidateParentRef; if (routePath !== undefined) { currentObj = { @@ -85,6 +90,9 @@ export function extractRouteInfoFromInstanceTree(roots: ExtensionInstance[]): { ? joinPaths(collectedPath, routePath) : routePath; } + + newParentRef = candidateParentRef; + newCandidateParentRef = undefined; } if (routeRef) { @@ -95,12 +103,16 @@ export function extractRouteInfoFromInstanceTree(roots: ExtensionInstance[]): { ); } + if (!newCandidateParentRef) { + newCandidateParentRef = routeRef; + } + if (newCollectedPath !== undefined) { routePaths.set(routeRef, newCollectedPath); newFoundRefForCollectedPath = true; } - routeParents.set(routeRef, parentRef); + routeParents.set(routeRef, newParentRef); currentObj?.routeRefs.add(routeRef); if (current.source) { currentObj?.plugins.add(toLegacyPlugin(current.source)); @@ -113,7 +125,8 @@ export function extractRouteInfoFromInstanceTree(roots: ExtensionInstance[]): { child, newCollectedPath, newFoundRefForCollectedPath, - routeRef ?? parentRef, + newParentRef, + newCandidateParentRef, currentObj, ); } From 783a6b70e671207c2d6e1bf137209bdd443f6cf6 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 3 Oct 2023 17:58:07 +0200 Subject: [PATCH 8/9] frontend-app-api: explain the route info extraction a bit Signed-off-by: Patrik Oldsberg --- .../extractRouteInfoFromInstanceTree.ts | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts index a7df7978cc..30305b0d2c 100644 --- a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts @@ -47,8 +47,13 @@ export function extractRouteInfoFromInstanceTree(roots: ExtensionInstance[]): { routeParents: Map; routeObjects: BackstageRouteObject[]; } { + // This tracks the route path for each route ref, the value is the route path relative to the parent ref const routePaths = new Map(); + // This tracks the parents of each route ref. To find the full path of any route ref you traverse + // upwards in this tree and substitute each route ref for its route path along then way. const routeParents = new Map(); + // This route object tree is passed to react-router in order to be able to look up the current route + // ref or extension/source based on our current location. const routeObjects = new Array(); function visit( @@ -65,11 +70,14 @@ export function extractRouteInfoFromInstanceTree(roots: ExtensionInstance[]): { const routeRef = current.getData(coreExtensionData.routeRef); const parentChildren = parentObj?.children ?? routeObjects; let currentObj = parentObj; + let newCollectedPath = collectedPath; let newFoundRefForCollectedPath = foundRefForCollectedPath; + let newParentRef = parentRef; let newCandidateParentRef = candidateParentRef; + // Whenever a route path is encountered, a new node is created in the routing tree. if (routePath !== undefined) { currentObj = { path: routePath, @@ -79,9 +87,16 @@ export function extractRouteInfoFromInstanceTree(roots: ExtensionInstance[]): { children: [MATCH_ALL_ROUTE], plugins: new Set(), }; - parentChildren.push(currentObj); + // Each route path that we discover creates a new node in the routing tree, at that point + // we also switch out our candidate parent ref to be the active one. + newParentRef = candidateParentRef; + newCandidateParentRef = undefined; + + // We need to collect and concatenate route paths until the path has been assigned a route ref: + // Once we find a route ref the collection starts over from an empty path, that way each route + // path assignment only contains the diff from the parent ref. if (newFoundRefForCollectedPath) { newCollectedPath = routePath; newFoundRefForCollectedPath = false; @@ -90,11 +105,9 @@ export function extractRouteInfoFromInstanceTree(roots: ExtensionInstance[]): { ? joinPaths(collectedPath, routePath) : routePath; } - - newParentRef = candidateParentRef; - newCandidateParentRef = undefined; } + // Whenever a route ref is encountered, we need to give it a route path and position in the ref tree. if (routeRef) { const routeRefId = (routeRef as any).id; // TODO: properly if (routeRefId !== current.id) { @@ -103,10 +116,15 @@ export function extractRouteInfoFromInstanceTree(roots: ExtensionInstance[]): { ); } + // The first route ref we find after encountering a route path is selected to be used as the + // parent ref further down the tree. We don't start using this candidate ref until we encounter + // another route path though, at which point we repeat the process and select another candidate. if (!newCandidateParentRef) { newCandidateParentRef = routeRef; } + // Check if we've encountered any route paths since the closest route ref, in that case we assign + // that path to this and following route refs until we encounter another route path. if (newCollectedPath !== undefined) { routePaths.set(routeRef, newCollectedPath); newFoundRefForCollectedPath = true; From 1718ec75b78988c067f4c991d6b47752e0356b75 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 3 Oct 2023 17:58:46 +0200 Subject: [PATCH 9/9] changesets: add changeset for routing system support in frontend-app-api Signed-off-by: Patrik Oldsberg --- .changeset/unlucky-experts-breathe.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/unlucky-experts-breathe.md diff --git a/.changeset/unlucky-experts-breathe.md b/.changeset/unlucky-experts-breathe.md new file mode 100644 index 0000000000..018b1826d2 --- /dev/null +++ b/.changeset/unlucky-experts-breathe.md @@ -0,0 +1,5 @@ +--- +'@backstage/frontend-app-api': patch +--- + +Added support for the existing routing system.