diff --git a/.changeset/spotty-zoos-sneeze.md b/.changeset/spotty-zoos-sneeze.md new file mode 100644 index 0000000000..9405e399e3 --- /dev/null +++ b/.changeset/spotty-zoos-sneeze.md @@ -0,0 +1,6 @@ +--- +'@backstage/core-app-api': minor +'@backstage/frontend-app-api': patch +--- + +Added the ability to explicitly disable routes through the `bindRoutes` option by passing `false` as the route target. This also fixes a bug where route bindings in config were incorrectly prioritized above the ones in code in certain situations. diff --git a/docs/features/software-templates/index.md b/docs/features/software-templates/index.md index d4fa6ac971..077090743f 100644 --- a/docs/features/software-templates/index.md +++ b/docs/features/software-templates/index.md @@ -91,19 +91,29 @@ There could be situations where you would like to disable the ![Disable Button](../../assets/software-templates/disable-register-existing-component-button.png) -To do so, you will un-register / remove the `catalogImportPlugin.routes.importPage` -from `backstage/packages/app/src/App.tsx`: +To do so, you need to explicitly disable the default route binding from the `scaffolderPlugin.registerComponent` to the Catalog Import page. + +This can be done in `backstage/packages/app/src/App.tsx`: ```diff const app = createApp({ apis, bindRoutes({ bind }) { -- bind(scaffolderPlugin.externalRoutes, { + bind(scaffolderPlugin.externalRoutes, { ++ registerComponent: false, - registerComponent: catalogImportPlugin.routes.importPage, -- }); - bind(orgPlugin.externalRoutes, { - catalogIndex: catalogPlugin.routes.catalogIndex, + viewTechDoc: techdocsPlugin.routes.docRoot, }); +}) +``` + +OR in `app-config.yaml`: + +```yaml +app: + routes: + bindings: + scaffolder.registerComponent: false ``` After the change, you should no longer see the button. diff --git a/docs/frontend-system/architecture/36-routes.md b/docs/frontend-system/architecture/36-routes.md index 2a9b372b1c..8ed3024edd 100644 --- a/docs/frontend-system/architecture/36-routes.md +++ b/docs/frontend-system/architecture/36-routes.md @@ -251,6 +251,8 @@ app: # point to the Catalog details page when the Scaffolder component details ref is used # highlight-next-line scaffolder.componentDetails: catalog.details + # explicitly disable the default route binding from the scaffolder to the catalog import page + scaffolder.registerComponent: false ``` We also have the ability to express this in code as an option to `createApp`, but you of course only need to use one of these two methods: @@ -268,6 +270,7 @@ const app = createApp({ }); bind(scaffolder.externalRoutes, { componentDetails: catalog.routes.details, + registerComponent: false, }); }, // highlight-end diff --git a/packages/core-app-api/src/app/resolveRouteBindings.test.ts b/packages/core-app-api/src/app/resolveRouteBindings.test.ts index 7d5c649a34..fdc1a9e456 100644 --- a/packages/core-app-api/src/app/resolveRouteBindings.test.ts +++ b/packages/core-app-api/src/app/resolveRouteBindings.test.ts @@ -75,6 +75,55 @@ describe('resolveRouteBindings', () => { expect(result.get(mySource)).toBe(myTarget); }); + it('prioritizes callback routes over config', () => { + const mySource = createExternalRouteRef({ id: 'test', optional: true }); + const myTarget = createRouteRef({ id: 'test' }); + + expect( + resolveRouteBindings( + ({ bind }) => { + bind({ mySource }, { mySource: false }); + }, + new MockConfigApi({ + app: { routes: { bindings: { 'test.mySource': 'myTarget' } } }, + }), + [ + createPlugin({ + id: 'test', + routes: { + myTarget, + }, + externalRoutes: { + mySource, + }, + }), + ], + ).get(mySource), + ).toBe(undefined); + + expect( + resolveRouteBindings( + ({ bind }) => { + bind({ mySource }, { mySource: myTarget }); + }, + new MockConfigApi({ + app: { routes: { bindings: { 'test.mySource': false } } }, + }), + [ + createPlugin({ + id: 'test', + routes: { + myTarget, + }, + externalRoutes: { + mySource, + }, + }), + ], + ).get(mySource), + ).toBe(myTarget); + }); + it('throws on invalid config', () => { expect(() => resolveRouteBindings( diff --git a/packages/core-app-api/src/app/resolveRouteBindings.ts b/packages/core-app-api/src/app/resolveRouteBindings.ts index 07dfea43ee..41b88f2674 100644 --- a/packages/core-app-api/src/app/resolveRouteBindings.ts +++ b/packages/core-app-api/src/app/resolveRouteBindings.ts @@ -73,6 +73,7 @@ export function resolveRouteBindings( ) { const routesById = collectRouteIds(plugins); const result = new Map(); + const disabledExternalRefs = new Set(); // Perform callback bindings first with highest priority if (bindRoutes) { @@ -87,11 +88,15 @@ export function resolveRouteBindings( } if (!value && !externalRoute.optional) { throw new Error( - `External route ${key} is required but was undefined`, + `External route ${key} is required but was ${ + value === false ? 'disabled' : 'not provided' + }`, ); } if (value) { result.set(externalRoute, value); + } else if (value === false) { + disabledExternalRefs.add(externalRoute); } } }; @@ -102,7 +107,6 @@ 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 (!isValidTargetRefId(targetRefId)) { @@ -118,11 +122,14 @@ export function resolveRouteBindings( ); } + // Skip if binding was already defined in code + if (result.has(externalRef) || disabledExternalRefs.has(externalRef)) { + continue; + } + if (targetRefId === false) { disabledExternalRefs.add(externalRef); - - result.delete(externalRef); - } else if (!result.has(externalRef)) { + } else { const targetRef = routesById.routes.get(targetRefId); if (!targetRef) { throw new Error( diff --git a/packages/core-app-api/src/app/types.ts b/packages/core-app-api/src/app/types.ts index 513063e974..1b33e9c05b 100644 --- a/packages/core-app-api/src/app/types.ts +++ b/packages/core-app-api/src/app/types.ts @@ -160,7 +160,7 @@ type TargetRouteMap< infer Params, any > - ? RouteRef | SubRouteRef + ? RouteRef | SubRouteRef | false : never; }; diff --git a/packages/frontend-app-api/src/routing/resolveRouteBindings.test.ts b/packages/frontend-app-api/src/routing/resolveRouteBindings.test.ts index 908a99d3aa..6618bef152 100644 --- a/packages/frontend-app-api/src/routing/resolveRouteBindings.test.ts +++ b/packages/frontend-app-api/src/routing/resolveRouteBindings.test.ts @@ -69,6 +69,41 @@ describe('resolveRouteBindings', () => { expect(result.get(mySource)).toBe(myTarget); }); + it('prioritizes callback routes over config', () => { + const mySource = createExternalRouteRef(); + const myTarget = createRouteRef(); + + expect( + resolveRouteBindings( + ({ bind }) => { + bind({ mySource }, { mySource: false }); + }, + new ConfigReader({ + app: { routes: { bindings: { mySource: 'myTarget' } } }, + }), + { + routes: new Map([['myTarget', myTarget]]), + externalRoutes: new Map([['mySource', mySource]]), + }, + ).get(mySource), + ).toBe(undefined); + + expect( + resolveRouteBindings( + ({ bind }) => { + bind({ mySource }, { mySource: myTarget }); + }, + new ConfigReader({ + app: { routes: { bindings: { mySource: false } } }, + }), + { + routes: new Map([['myTarget', myTarget]]), + externalRoutes: new Map([['mySource', mySource]]), + }, + ).get(mySource), + ).toBe(myTarget); + }); + it('throws on invalid config', () => { expect(() => resolveRouteBindings( diff --git a/packages/frontend-app-api/src/routing/resolveRouteBindings.ts b/packages/frontend-app-api/src/routing/resolveRouteBindings.ts index a8f641daff..433115ccec 100644 --- a/packages/frontend-app-api/src/routing/resolveRouteBindings.ts +++ b/packages/frontend-app-api/src/routing/resolveRouteBindings.ts @@ -55,7 +55,7 @@ type TargetRouteMap< [name in keyof ExternalRoutes]: ExternalRoutes[name] extends ExternalRouteRef< infer Params > - ? RouteRef | SubRouteRef + ? RouteRef | SubRouteRef | false : never; }; @@ -82,6 +82,7 @@ export function resolveRouteBindings( routesById: RouteRefsById, ): Map { const result = new Map(); + const disabledExternalRefs = new Set(); // Perform callback bindings first with highest priority if (bindRoutes) { @@ -96,6 +97,8 @@ export function resolveRouteBindings( } if (value) { result.set(externalRoute, value); + } else if (value === false) { + disabledExternalRefs.add(externalRoute); } } }; @@ -106,7 +109,6 @@ 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 (!isValidTargetRefId(targetRefId)) { @@ -122,11 +124,14 @@ export function resolveRouteBindings( ); } + // Skip if binding was already defined in code + if (result.has(externalRef) || disabledExternalRefs.has(externalRef)) { + continue; + } + if (targetRefId === false) { disabledExternalRefs.add(externalRef); - - result.delete(externalRef); - } else if (!result.has(externalRef)) { + } else { const targetRef = routesById.routes.get(targetRefId); if (!targetRef) { throw new Error(