From 46b63dea672ca789e16a7577fc33c542194cc25f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 23 Jan 2024 09:26:38 +0100 Subject: [PATCH] add defaultTarget to ExternalRouteRef MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/twelve-pens-rescue.md | 7 ++ packages/app-next/app-config.yaml | 1 - .../app-next/src/examples/pagesPlugin.tsx | 4 +- .../src/convertLegacyRouteRef.ts | 3 + .../src/routing/resolveRouteBindings.test.ts | 44 ++++++++++++ .../src/routing/resolveRouteBindings.ts | 72 +++++++++++-------- packages/frontend-plugin-api/api-report.md | 1 + .../src/routing/ExternalRouteRef.ts | 15 ++++ 8 files changed, 117 insertions(+), 30 deletions(-) create mode 100644 .changeset/twelve-pens-rescue.md diff --git a/.changeset/twelve-pens-rescue.md b/.changeset/twelve-pens-rescue.md new file mode 100644 index 0000000000..9d40fb2851 --- /dev/null +++ b/.changeset/twelve-pens-rescue.md @@ -0,0 +1,7 @@ +--- +'@backstage/frontend-plugin-api': patch +'@backstage/frontend-app-api': patch +'@backstage/core-compat-api': patch +--- + +Allow external route refs in the new system to have a `defaultTarget` pointing to a route that it'll resolve to by default if no explicit bindings were made by the adopter. diff --git a/packages/app-next/app-config.yaml b/packages/app-next/app-config.yaml index 9cc3507fa4..a79bb866db 100644 --- a/packages/app-next/app-config.yaml +++ b/packages/app-next/app-config.yaml @@ -3,7 +3,6 @@ app: packages: 'all' # ✨ routes: bindings: - pages.pageX: pages.pageX catalog.viewTechDoc: techdocs.docRoot catalog.createComponent: catalog-import.importPage diff --git a/packages/app-next/src/examples/pagesPlugin.tsx b/packages/app-next/src/examples/pagesPlugin.tsx index c5069f99c3..826040a6cb 100644 --- a/packages/app-next/src/examples/pagesPlugin.tsx +++ b/packages/app-next/src/examples/pagesPlugin.tsx @@ -27,7 +27,9 @@ import { Route, Routes } from 'react-router-dom'; const indexRouteRef = createRouteRef(); const page1RouteRef = createRouteRef(); -export const externalPageXRouteRef = createExternalRouteRef(); +export const externalPageXRouteRef = createExternalRouteRef({ + defaultTarget: 'pages.pageX', +}); export const pageXRouteRef = createRouteRef(); // const page2RouteRef = createSubRouteRef({ // id: 'page2', diff --git a/packages/core-compat-api/src/convertLegacyRouteRef.ts b/packages/core-compat-api/src/convertLegacyRouteRef.ts index 36311dc081..e6a52932ce 100644 --- a/packages/core-compat-api/src/convertLegacyRouteRef.ts +++ b/packages/core-compat-api/src/convertLegacyRouteRef.ts @@ -199,6 +199,9 @@ export function convertLegacyRouteRef( getDescription() { return legacyRefStr; }, + getDefaultTarget() { + return newRef.getDefaultTarget(); + }, setId(id: string) { newRef.setId(id); }, diff --git a/packages/frontend-app-api/src/routing/resolveRouteBindings.test.ts b/packages/frontend-app-api/src/routing/resolveRouteBindings.test.ts index d80a02e34d..9b150eb127 100644 --- a/packages/frontend-app-api/src/routing/resolveRouteBindings.test.ts +++ b/packages/frontend-app-api/src/routing/resolveRouteBindings.test.ts @@ -117,4 +117,48 @@ describe('resolveRouteBindings', () => { "Invalid config at app.routes.bindings['mySource'], 'myTarget' is not a valid route", ); }); + + it('can have default targets, but at the lowest priority', () => { + const source = createExternalRouteRef({ defaultTarget: 'target1' }); + const target1 = createRouteRef(); + const target2 = createRouteRef(); + const routesById = { + routes: new Map([ + ['target1', target1], + ['target2', target2], + ]), + externalRoutes: new Map([['source', source]]), + }; + + // defaultTarget wins only if no bind or config matches + let result = resolveRouteBindings( + () => {}, + new ConfigReader({}), + routesById, + ); + + expect(result.get(source)).toBe(target1); + + // config wins over defaultTarget + result = resolveRouteBindings( + () => {}, + new ConfigReader({ + app: { routes: { bindings: { source: 'target2' } } }, + }), + routesById, + ); + + expect(result.get(source)).toBe(target2); + + // bind wins over defaultTarget + result = resolveRouteBindings( + ({ bind }) => { + bind({ a: source }, { a: target2 }); + }, + new ConfigReader({}), + routesById, + ); + + expect(result.get(source)).toBe(target2); + }); }); diff --git a/packages/frontend-app-api/src/routing/resolveRouteBindings.ts b/packages/frontend-app-api/src/routing/resolveRouteBindings.ts index 18ea9f9bd8..49e3b1a1be 100644 --- a/packages/frontend-app-api/src/routing/resolveRouteBindings.ts +++ b/packages/frontend-app-api/src/routing/resolveRouteBindings.ts @@ -22,6 +22,8 @@ import { import { RouteRefsById } from './collectRouteIds'; import { Config } from '@backstage/config'; import { JsonObject } from '@backstage/types'; +// eslint-disable-next-line @backstage/no-relative-monorepo-imports +import { toInternalExternalRouteRef } from '../../../frontend-plugin-api/src/routing/ExternalRouteRef'; /** * Extracts a union of the keys in a map whose value extends the given type @@ -82,6 +84,7 @@ export function resolveRouteBindings( ): Map { const result = new Map(); + // Perform callback bindings first with highest priority if (bindRoutes) { const bind: CreateAppRouteBinder = ( externalRoutes, @@ -105,37 +108,50 @@ export function resolveRouteBindings( bindRoutes({ bind }); } - const bindingsConfig = config.getOptionalConfig('app.routes.bindings'); - if (!bindingsConfig) { - return result; + // Then perform config based bindings with lower priority + const bindings = config + .getOptionalConfig('app.routes.bindings') + ?.get(); + if (bindings) { + for (const [externalRefId, targetRefId] of Object.entries(bindings)) { + if (typeof targetRefId !== 'string' || targetRefId === '') { + throw new Error( + `Invalid config at app.routes.bindings['${externalRefId}'], value must be a non-empty string`, + ); + } + + const externalRef = routesById.externalRoutes.get(externalRefId); + if (!externalRef) { + throw new Error( + `Invalid config at app.routes.bindings, '${externalRefId}' is not a valid external route`, + ); + } + if (result.has(externalRef)) { + continue; + } + const targetRef = routesById.routes.get(targetRefId); + if (!targetRef) { + throw new Error( + `Invalid config at app.routes.bindings['${externalRefId}'], '${targetRefId}' is not a valid route`, + ); + } + + result.set(externalRef, targetRef); + } } - const bindings = bindingsConfig.get(); - for (const [externalRefId, targetRefId] of Object.entries(bindings)) { - if (typeof targetRefId !== 'string' || targetRefId === '') { - throw new Error( - `Invalid config at app.routes.bindings['${externalRefId}'], value must be a non-empty string`, - ); + // Finally fall back to attempting to map defaults, at lowest priority + for (const externalRef of routesById.externalRoutes.values()) { + if (!result.has(externalRef)) { + const defaultRefId = + toInternalExternalRouteRef(externalRef).getDefaultTarget(); + if (defaultRefId) { + const defaultRef = routesById.routes.get(defaultRefId); + if (defaultRef) { + result.set(externalRef, defaultRef); + } + } } - - const externalRef = routesById.externalRoutes.get(externalRefId); - if (!externalRef) { - throw new Error( - `Invalid config at app.routes.bindings, '${externalRefId}' is not a valid external route`, - ); - } - // Route bindings defined in config have lower priority than those defined in code - if (result.has(externalRef)) { - continue; - } - const targetRef = routesById.routes.get(targetRefId); - if (!targetRef) { - throw new Error( - `Invalid config at app.routes.bindings['${externalRefId}'], '${targetRefId}' is not a valid route`, - ); - } - - result.set(externalRef, targetRef); } return result; diff --git a/packages/frontend-plugin-api/api-report.md b/packages/frontend-plugin-api/api-report.md index 4116469c2a..28c8ea321c 100644 --- a/packages/frontend-plugin-api/api-report.md +++ b/packages/frontend-plugin-api/api-report.md @@ -573,6 +573,7 @@ export function createExternalRouteRef< ? (keyof TParams)[] : TParamKeys[]; optional?: TOptional; + defaultTarget?: string; }): ExternalRouteRef< keyof TParams extends never ? undefined diff --git a/packages/frontend-plugin-api/src/routing/ExternalRouteRef.ts b/packages/frontend-plugin-api/src/routing/ExternalRouteRef.ts index c9aabdef27..7a754ffa7b 100644 --- a/packages/frontend-plugin-api/src/routing/ExternalRouteRef.ts +++ b/packages/frontend-plugin-api/src/routing/ExternalRouteRef.ts @@ -44,6 +44,7 @@ export interface InternalExternalRouteRef< readonly version: 'v1'; getParams(): string[]; getDescription(): string; + getDefaultTarget(): string | undefined; setId(id: string): void; } @@ -80,10 +81,15 @@ class ExternalRouteRefImpl constructor( readonly optional: boolean, readonly params: string[] = [], + readonly defaultTarget: string | undefined, creationSite: string, ) { super(params, creationSite); } + + getDefaultTarget() { + return this.defaultTarget; + } } /** @@ -115,6 +121,14 @@ export function createExternalRouteRef< * if they aren't, `useExternalRouteRef` will return `undefined`. */ optional?: TOptional; + + /** + * The route (typically in another plugin) that this should map to by default. + * + * The string is expected to be on the standard `.` form, + * for example `techdocs.docRoot`. + */ + defaultTarget?: string; }): ExternalRouteRef< keyof TParams extends never ? undefined @@ -126,6 +140,7 @@ export function createExternalRouteRef< return new ExternalRouteRefImpl( Boolean(options?.optional), options?.params as string[] | undefined, + options?.defaultTarget, describeParentCallSite(), ) as ExternalRouteRef; }