diff --git a/.changeset/famous-dodos-crash.md b/.changeset/famous-dodos-crash.md new file mode 100644 index 0000000000..5442c1b378 --- /dev/null +++ b/.changeset/famous-dodos-crash.md @@ -0,0 +1,15 @@ +--- +'@backstage/core-app-api': minor +'@backstage/frontend-app-api': patch +--- + +Allow for the disabling of external routes through config, which was rendered impossible after the introduction of default targets. + +```yaml +app: + routes: + bindings: + # This has the effect of removing the button for registering new + # catalog entities in the scaffolder template list view + scaffolder.registerComponent: false +``` diff --git a/packages/core-app-api/src/app/resolveRouteBindings.test.ts b/packages/core-app-api/src/app/resolveRouteBindings.test.ts index 9dfbbfd452..7d5c649a34 100644 --- a/packages/core-app-api/src/app/resolveRouteBindings.test.ts +++ b/packages/core-app-api/src/app/resolveRouteBindings.test.ts @@ -194,4 +194,35 @@ describe('collectRouteIds', () => { 'test.extRef': extRef, }); }); + + it('can disable external routes that have defaults', () => { + const source = createExternalRouteRef({ + id: 'test', + defaultTarget: 'test.target1', + }); + const target1 = createRouteRef({ id: 'test' }); + const plugin = createPlugin({ + id: 'test', + routes: { target1 }, + externalRoutes: { source }, + }); + + // resolves normally with no config + let result = resolveRouteBindings(() => {}, new MockConfigApi({}), [ + plugin, + ]); + + expect(result.get(source)).toBe(target1); + + // can be disabled + result = resolveRouteBindings( + () => {}, + new MockConfigApi({ + app: { routes: { bindings: { 'test.source': false } } }, + }), + [plugin], + ); + + expect(result.get(source)).toBe(undefined); + }); }); diff --git a/packages/core-app-api/src/app/resolveRouteBindings.ts b/packages/core-app-api/src/app/resolveRouteBindings.ts index 55e1139b29..07dfea43ee 100644 --- a/packages/core-app-api/src/app/resolveRouteBindings.ts +++ b/packages/core-app-api/src/app/resolveRouteBindings.ts @@ -102,11 +102,12 @@ export function resolveRouteBindings( const bindings = config .getOptionalConfig('app.routes.bindings') ?.get(); + const disabledExternalRefs = new Set(); if (bindings) { for (const [externalRefId, targetRefId] of Object.entries(bindings)) { - if (typeof targetRefId !== 'string' || targetRefId === '') { + if (!isValidTargetRefId(targetRefId)) { throw new Error( - `Invalid config at app.routes.bindings['${externalRefId}'], value must be a non-empty string`, + `Invalid config at app.routes.bindings['${externalRefId}'], value must be a non-empty string or false`, ); } @@ -116,23 +117,27 @@ export function resolveRouteBindings( `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); + if (targetRefId === false) { + disabledExternalRefs.add(externalRef); + + result.delete(externalRef); + } else if (!result.has(externalRef)) { + 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); + } } } // Finally fall back to attempting to map defaults, at lowest priority for (const externalRef of routesById.externalRoutes.values()) { - if (!result.has(externalRef)) { + if (!result.has(externalRef) && !disabledExternalRefs.has(externalRef)) { const defaultRefId = 'getDefaultTarget' in externalRef ? (externalRef.getDefaultTarget as () => string | undefined)() @@ -148,3 +153,15 @@ export function resolveRouteBindings( return result; } + +function isValidTargetRefId(value: unknown): value is string | false { + if (value === false) { + return true; + } + + if (typeof value === 'string' && value) { + return true; + } + + return false; +} diff --git a/packages/frontend-app-api/src/routing/resolveRouteBindings.test.ts b/packages/frontend-app-api/src/routing/resolveRouteBindings.test.ts index 9b150eb127..908a99d3aa 100644 --- a/packages/frontend-app-api/src/routing/resolveRouteBindings.test.ts +++ b/packages/frontend-app-api/src/routing/resolveRouteBindings.test.ts @@ -161,4 +161,31 @@ describe('resolveRouteBindings', () => { expect(result.get(source)).toBe(target2); }); + + it('can disable external routes that have defaults', () => { + const source = createExternalRouteRef({ defaultTarget: 'target1' }); + const target1 = createRouteRef(); + const routesById = { + routes: new Map([['target1', target1]]), + externalRoutes: new Map([['source', source]]), + }; + + // resolves normally with no config + let result = resolveRouteBindings( + () => {}, + new ConfigReader({}), + routesById, + ); + + expect(result.get(source)).toBe(target1); + + // can be disabled + result = resolveRouteBindings( + () => {}, + new ConfigReader({ app: { routes: { bindings: { source: false } } } }), + routesById, + ); + + expect(result.get(source)).toBe(undefined); + }); }); diff --git a/packages/frontend-app-api/src/routing/resolveRouteBindings.ts b/packages/frontend-app-api/src/routing/resolveRouteBindings.ts index 49e3b1a1be..81d9eddef3 100644 --- a/packages/frontend-app-api/src/routing/resolveRouteBindings.ts +++ b/packages/frontend-app-api/src/routing/resolveRouteBindings.ts @@ -112,11 +112,12 @@ export function resolveRouteBindings( const bindings = config .getOptionalConfig('app.routes.bindings') ?.get(); + const disabledExternalRefs = new Set(); if (bindings) { for (const [externalRefId, targetRefId] of Object.entries(bindings)) { - if (typeof targetRefId !== 'string' || targetRefId === '') { + if (!isValidTargetRefId(targetRefId)) { throw new Error( - `Invalid config at app.routes.bindings['${externalRefId}'], value must be a non-empty string`, + `Invalid config at app.routes.bindings['${externalRefId}'], value must be a non-empty string or false`, ); } @@ -126,23 +127,27 @@ export function resolveRouteBindings( `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); + if (targetRefId === false) { + disabledExternalRefs.add(externalRef); + + result.delete(externalRef); + } else if (!result.has(externalRef)) { + 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); + } } } // Finally fall back to attempting to map defaults, at lowest priority for (const externalRef of routesById.externalRoutes.values()) { - if (!result.has(externalRef)) { + if (!result.has(externalRef) && !disabledExternalRefs.has(externalRef)) { const defaultRefId = toInternalExternalRouteRef(externalRef).getDefaultTarget(); if (defaultRefId) { @@ -156,3 +161,15 @@ export function resolveRouteBindings( return result; } + +function isValidTargetRefId(value: unknown): value is string | false { + if (value === false) { + return true; + } + + if (typeof value === 'string' && value) { + return true; + } + + return false; +}