diff --git a/.changeset/violet-jokes-tap.md b/.changeset/violet-jokes-tap.md new file mode 100644 index 0000000000..5b79ced6cd --- /dev/null +++ b/.changeset/violet-jokes-tap.md @@ -0,0 +1,21 @@ +--- +'@backstage/frontend-plugin-api': patch +'@backstage/frontend-app-api': patch +--- + +Added support for defining `replaces` in `createExtensionInput` which will allow extensions to redirect missing `attachTo` points to an input of the created extension. + +```ts +export const AppThemeApi = ApiBlueprint.makeWithOverrides({ + name: 'app-theme', + inputs: { + themes: createExtensionInput([ThemeBlueprint.dataRefs.theme], { + // attachTo: { id: 'app', input: 'themes'} will be redirected to this input instead + replaces: [{ id: 'app', input: 'themes' }], + }), + }, + factory: () { + ... + } +}); +``` diff --git a/packages/frontend-app-api/src/extensions/AppThemeApi.tsx b/packages/frontend-app-api/src/extensions/AppThemeApi.tsx index d0dcad34e8..fbbb9957ff 100644 --- a/packages/frontend-app-api/src/extensions/AppThemeApi.tsx +++ b/packages/frontend-app-api/src/extensions/AppThemeApi.tsx @@ -36,7 +36,9 @@ import { AppThemeSelector } from '@backstage/core-app-api'; export const AppThemeApi = ApiBlueprint.makeWithOverrides({ name: 'app-theme', inputs: { - themes: createExtensionInput([ThemeBlueprint.dataRefs.theme]), + themes: createExtensionInput([ThemeBlueprint.dataRefs.theme], { + replaces: [{ id: 'app', input: 'themes' }], + }), }, factory: (originalFactory, { inputs }) => { return originalFactory({ diff --git a/packages/frontend-app-api/src/extensions/ComponentsApi.tsx b/packages/frontend-app-api/src/extensions/ComponentsApi.tsx index 5803aacf95..5489aa35a2 100644 --- a/packages/frontend-app-api/src/extensions/ComponentsApi.tsx +++ b/packages/frontend-app-api/src/extensions/ComponentsApi.tsx @@ -29,9 +29,10 @@ import { DefaultComponentsApi } from '../apis/implementations/ComponentsApi'; export const ComponentsApi = ApiBlueprint.makeWithOverrides({ name: 'components', inputs: { - components: createExtensionInput([ - createComponentExtension.componentDataRef, - ]), + components: createExtensionInput( + [createComponentExtension.componentDataRef], + { replaces: [{ id: 'app', input: 'components' }] }, + ), }, factory: (originalFactory, { inputs }) => { return originalFactory({ diff --git a/packages/frontend-app-api/src/extensions/IconsApi.ts b/packages/frontend-app-api/src/extensions/IconsApi.ts index d770000197..dc2acac629 100644 --- a/packages/frontend-app-api/src/extensions/IconsApi.ts +++ b/packages/frontend-app-api/src/extensions/IconsApi.ts @@ -31,7 +31,9 @@ import { icons as defaultIcons } from '../../../app-defaults/src/defaults'; export const IconsApi = ApiBlueprint.makeWithOverrides({ name: 'icons', inputs: { - icons: createExtensionInput([IconBundleBlueprint.dataRefs.icons]), + icons: createExtensionInput([IconBundleBlueprint.dataRefs.icons], { + replaces: [{ id: 'app', input: 'icons' }], + }), }, factory: (originalFactory, { inputs }) => { return originalFactory({ diff --git a/packages/frontend-app-api/src/extensions/Root.ts b/packages/frontend-app-api/src/extensions/Root.ts index e33f794cd6..a613a8dcfa 100644 --- a/packages/frontend-app-api/src/extensions/Root.ts +++ b/packages/frontend-app-api/src/extensions/Root.ts @@ -28,7 +28,9 @@ export const Root = createExtension({ app: createExtensionInput([coreExtensionData.reactElement], { singleton: true, }), - apis: createExtensionInput([ApiBlueprint.dataRefs.factory]), + apis: createExtensionInput([ApiBlueprint.dataRefs.factory], { + replaces: [{ id: 'app', input: 'apis' }], + }), }, output: [], factory: () => [], diff --git a/packages/frontend-app-api/src/extensions/TranslationsApi.tsx b/packages/frontend-app-api/src/extensions/TranslationsApi.tsx index 9c13c3b063..1fa4fadaae 100644 --- a/packages/frontend-app-api/src/extensions/TranslationsApi.tsx +++ b/packages/frontend-app-api/src/extensions/TranslationsApi.tsx @@ -33,9 +33,10 @@ import { I18nextTranslationApi } from '../../../core-app-api/src/apis/implementa export const TranslationsApi = ApiBlueprint.makeWithOverrides({ name: 'translations', inputs: { - translations: createExtensionInput([ - TranslationBlueprint.dataRefs.translation, - ]), + translations: createExtensionInput( + [TranslationBlueprint.dataRefs.translation], + { replaces: [{ id: 'app', input: 'translations' }] }, + ), }, factory: (originalFactory, { inputs }) => { return originalFactory({ diff --git a/packages/frontend-app-api/src/tree/resolveAppTree.test.ts b/packages/frontend-app-api/src/tree/resolveAppTree.test.ts index 740f05d5f7..0517e943aa 100644 --- a/packages/frontend-app-api/src/tree/resolveAppTree.test.ts +++ b/packages/frontend-app-api/src/tree/resolveAppTree.test.ts @@ -14,7 +14,12 @@ * limitations under the License. */ -import { createExtension, Extension } from '@backstage/frontend-plugin-api'; +import { + coreExtensionData, + createExtension, + createExtensionInput, + Extension, +} from '@backstage/frontend-plugin-api'; import { resolveAppTree } from './resolveAppTree'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { resolveExtensionDefinition } from '../../../frontend-plugin-api/src/wiring/resolveExtensionDefinition'; @@ -165,4 +170,146 @@ describe('buildAppTree', () => { ]), ).toThrow("Unexpected duplicate extension id 'a'"); }); + + describe('redirects', () => { + it('should throw an error when theres a duplicate redirect target', () => { + const e1 = resolveExtensionDefinition( + createExtension({ + name: 'test', + attachTo: { id: 'nonexistent', input: 'nonexistent' }, + inputs: { + test: createExtensionInput([coreExtensionData.reactElement], { + replaces: [{ id: 'a', input: 'test' }], + }), + }, + output: [], + factory: () => [], + }), + ) as Extension; + + const e2 = resolveExtensionDefinition( + createExtension({ + name: 'test-2', + attachTo: { id: 'nonexistent', input: 'nonexistent' }, + inputs: { + test: createExtensionInput([coreExtensionData.reactElement], { + replaces: [{ id: 'a', input: 'test' }], + }), + }, + output: [], + factory: () => [], + }), + ) as Extension; + + expect(() => + resolveAppTree('a', [ + { ...baseSpec, id: 'a', extension: e1 }, + { ...baseSpec, id: 'b', extension: e2 }, + ]), + ).toThrow("Duplicate redirect target for input 'test' in extension 'b'"); + }); + + it('should set the correct attachment point for a redirect', () => { + const e1 = resolveExtensionDefinition( + createExtension({ + name: 'test', + attachTo: { id: 'nonexistent', input: 'nonexistent' }, + inputs: { + test: createExtensionInput([coreExtensionData.reactElement], { + replaces: [{ id: 'replace', input: 'me' }], + }), + }, + output: [], + factory: () => [], + }), + ) as Extension; + + const e2 = resolveExtensionDefinition( + createExtension({ + name: 'test-2', + attachTo: { id: 'replace', input: 'me' }, + output: [], + factory: () => [], + }), + ) as Extension; + + const tree = resolveAppTree('a', [ + { attachTo: e1.attachTo, id: 'a', extension: e1, disabled: false }, + { attachTo: e2.attachTo, id: 'b', extension: e2, disabled: false }, + ]); + + expect(tree.root).toMatchInlineSnapshot(` + { + "attachments": { + "test": [ + { + "attachments": undefined, + "id": "b", + "output": undefined, + }, + ], + }, + "id": "a", + "output": undefined, + } + `); + + expect(tree.orphans).toMatchInlineSnapshot(`[]`); + + expect(String(tree.root)).toMatchInlineSnapshot(` + " + test [ + + ] + " + `); + }); + + it('should not allow redirects for attachment points that already exist', () => { + const e1 = resolveExtensionDefinition( + createExtension({ + name: 'test', + attachTo: { id: 'a', input: 'a' }, + inputs: { + test: createExtensionInput([coreExtensionData.reactElement], { + replaces: [{ id: 'test-2', input: 'test' }], + }), + }, + output: [], + factory: () => [], + }), + ) as Extension; + + const e2 = resolveExtensionDefinition( + createExtension({ + name: 'test-2', + attachTo: { id: 'b', input: 'b' }, + inputs: { + test: createExtensionInput([coreExtensionData.reactElement]), + }, + output: [], + factory: () => [], + }), + ) as Extension; + + const e3 = resolveExtensionDefinition( + createExtension({ + name: 'test-3', + attachTo: { id: 'test-2', input: 'test' }, + output: [], + factory: () => [], + }), + ) as Extension; + + const tree = resolveAppTree('test-2', [ + { attachTo: e1.attachTo, id: e1.id, extension: e1, disabled: false }, + { attachTo: e2.attachTo, id: e2.id, extension: e2, disabled: false }, + { attachTo: e3.attachTo, id: e3.id, extension: e3, disabled: false }, + ]); + + expect(tree.nodes.get('test-3')?.edges.attachedTo?.node).toBe( + tree.nodes.get('test-2'), + ); + }); + }); }); diff --git a/packages/frontend-app-api/src/tree/resolveAppTree.ts b/packages/frontend-app-api/src/tree/resolveAppTree.ts index 24948373bf..dc9985da30 100644 --- a/packages/frontend-app-api/src/tree/resolveAppTree.ts +++ b/packages/frontend-app-api/src/tree/resolveAppTree.ts @@ -21,6 +21,9 @@ import { AppNodeSpec, } from '@backstage/frontend-plugin-api'; +// eslint-disable-next-line @backstage/no-relative-monorepo-imports +import { toInternalExtension } from '../../../frontend-plugin-api/src/wiring/resolveExtensionDefinition'; + function indent(str: string) { return str.replace(/^/gm, ' '); } @@ -38,9 +41,7 @@ class SerializableAppNode implements AppNode { this.spec = spec; } - setParent(parent: SerializableAppNode) { - const input = this.spec.attachTo.input; - + setParent(parent: SerializableAppNode, input: string) { this.edges.attachedTo = { node: parent, input }; const parentInputEdges = parent.edges.attachments.get(input); @@ -87,6 +88,24 @@ class SerializableAppNode implements AppNode { } } +function makeRedirectKey(attachTo: { id: string; input: string }) { + return `${attachTo.id}%${attachTo.input}`; +} + +const isValidAttachmentPoint = ( + attachTo: { id: string; input: string }, + nodes: Map, +) => { + if (!nodes.has(attachTo.id)) { + return false; + } + + return ( + attachTo.input in + toInternalExtension(nodes.get(attachTo.id)!.spec.extension).inputs + ); +}; + /** * Build the app tree by iterating through all node specs and constructing the app * tree with all attachments in the same order as they appear in the input specs array. @@ -98,17 +117,7 @@ export function resolveAppTree( ): AppTree { const nodes = new Map(); - // A node with the provided rootNodeId must be found in the tree, and it must not be attached to anything - let rootNode: AppNode | undefined = undefined; - - // While iterating through the inputs specs we keep track of all nodes that were created - // before their parent, and attach them later when the parent is created. - // As we find the parents and attach the children, we remove them from this map. This means - // that after iterating through all input specs, this will be a map for each root node. - const orphansByParent = new Map< - string /* parentId */, - SerializableAppNode[] - >(); + const redirectTargetsByKey = new Map(); for (const spec of specs) { // The main check with a more helpful error message happens in resolveAppNodeSpecs @@ -119,28 +128,46 @@ export function resolveAppTree( const node = new SerializableAppNode(spec); nodes.set(spec.id, node); + const internal = toInternalExtension(spec.extension); + for (const [inputName, input] of Object.entries(internal.inputs)) { + if (input.replaces) { + for (const replace of input.replaces) { + const key = makeRedirectKey(replace); + if (redirectTargetsByKey.has(key)) { + throw new Error( + `Duplicate redirect target for input '${inputName}' in extension '${spec.id}'`, + ); + } + redirectTargetsByKey.set(key, { id: spec.id, input: inputName }); + } + } + } + } + + const orphans = new Array(); + + // A node with the provided rootNodeId must be found in the tree, and it must not be attached to anything + let rootNode: AppNode | undefined = undefined; + + for (const node of nodes.values()) { + const spec = node.spec; + // TODO: For now we simply ignore the attachTo spec of the root node, but it'd be cleaner if we could avoid defining it if (spec.id === rootNodeId) { rootNode = node; } else { - const parent = nodes.get(spec.attachTo.id); - if (parent) { - node.setParent(parent); - } else { - const orphanNodesForParent = orphansByParent.get(spec.attachTo.id); - if (orphanNodesForParent) { - orphanNodesForParent.push(node); - } else { - orphansByParent.set(spec.attachTo.id, [node]); - } - } - } + let attachTo = node.spec.attachTo; - const orphanedChildren = orphansByParent.get(spec.id); - if (orphanedChildren) { - orphansByParent.delete(spec.id); - for (const orphan of orphanedChildren) { - orphan.setParent(node); + if (!isValidAttachmentPoint(attachTo, nodes)) { + attachTo = + redirectTargetsByKey.get(makeRedirectKey(attachTo)) ?? attachTo; + } + + const parent = nodes.get(attachTo.id); + if (parent) { + node.setParent(parent, attachTo.input); + } else { + orphans.push(node); } } } @@ -152,6 +179,6 @@ export function resolveAppTree( return { root: rootNode, nodes, - orphans: Array.from(orphansByParent.values()).flat(), + orphans, }; } diff --git a/packages/frontend-plugin-api/api-report.md b/packages/frontend-plugin-api/api-report.md index c32d5b4edb..5b0d7d4432 100644 --- a/packages/frontend-plugin-api/api-report.md +++ b/packages/frontend-plugin-api/api-report.md @@ -687,7 +687,12 @@ export function createExtensionInput< }, >( extensionData: Array, - config?: TConfig, + config?: TConfig & { + replaces?: Array<{ + id: string; + input: string; + }>; + }, ): ExtensionInput< UExtensionData, { @@ -1183,7 +1188,7 @@ export interface ExtensionDefinition< // @public (undocumented) export interface ExtensionInput< - TExtensionData extends ExtensionDataRef< + UExtensionData extends ExtensionDataRef< unknown, string, { @@ -1200,7 +1205,12 @@ export interface ExtensionInput< // (undocumented) config: TConfig; // (undocumented) - extensionData: Array; + extensionData: Array; + // (undocumented) + replaces?: Array<{ + id: string; + input: string; + }>; } // @public (undocumented) diff --git a/packages/frontend-plugin-api/src/blueprints/ApiBlueprint.test.ts b/packages/frontend-plugin-api/src/blueprints/ApiBlueprint.test.ts index f937b556ed..3571a666b0 100644 --- a/packages/frontend-plugin-api/src/blueprints/ApiBlueprint.test.ts +++ b/packages/frontend-plugin-api/src/blueprints/ApiBlueprint.test.ts @@ -116,6 +116,7 @@ describe('ApiBlueprint', () => { "extensionData": [ [Function], ], + "replaces": undefined, }, }, "kind": "api", diff --git a/packages/frontend-plugin-api/src/wiring/createExtensionInput.ts b/packages/frontend-plugin-api/src/wiring/createExtensionInput.ts index 8d293fceb3..7209e6f647 100644 --- a/packages/frontend-plugin-api/src/wiring/createExtensionInput.ts +++ b/packages/frontend-plugin-api/src/wiring/createExtensionInput.ts @@ -18,12 +18,13 @@ import { ExtensionDataRef } from './createExtensionDataRef'; /** @public */ export interface ExtensionInput< - TExtensionData extends ExtensionDataRef, + UExtensionData extends ExtensionDataRef, TConfig extends { singleton: boolean; optional: boolean }, > { $$type: '@backstage/ExtensionInput'; - extensionData: Array; + extensionData: Array; config: TConfig; + replaces?: Array<{ id: string; input: string }>; } /** @public */ @@ -32,7 +33,7 @@ export function createExtensionInput< TConfig extends { singleton?: boolean; optional?: boolean }, >( extensionData: Array, - config?: TConfig, + config?: TConfig & { replaces?: Array<{ id: string; input: string }> }, ): ExtensionInput< UExtensionData, { @@ -71,6 +72,7 @@ export function createExtensionInput< ? true : false, }, + replaces: config?.replaces, } as ExtensionInput< UExtensionData, {