From a5ee952079f53e9d16807ea9c272dc1b9a3a14d5 Mon Sep 17 00:00:00 2001 From: blam Date: Fri, 23 Aug 2024 17:03:40 +0200 Subject: [PATCH] feat: simplify the extension redirects Signed-off-by: blam --- .../src/extensions/AppThemeApi.tsx | 4 +- .../src/extensions/ComponentsApi.tsx | 7 +- .../src/extensions/IconsApi.ts | 4 +- .../src/extensions/TranslationsApi.tsx | 7 +- .../src/tree/resolveAppTree.test.ts | 108 +++++++++++++----- .../src/tree/resolveAppTree.ts | 22 +++- .../src/blueprints/ApiBlueprint.test.ts | 1 + 7 files changed, 113 insertions(+), 40 deletions(-) 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/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 ac12d6284b..c8535365aa 100644 --- a/packages/frontend-app-api/src/tree/resolveAppTree.test.ts +++ b/packages/frontend-app-api/src/tree/resolveAppTree.test.ts @@ -15,6 +15,7 @@ */ import { + AppNodeSpec, coreExtensionData, createExtension, createExtensionInput, @@ -208,38 +209,37 @@ describe('buildAppTree', () => { ]), ).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; + 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 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 }, - ]); + 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(` + expect(tree.root).toMatchInlineSnapshot(` { "attachments": { "test": [ @@ -255,14 +255,62 @@ describe('buildAppTree', () => { } `); - expect(tree.orphans).toMatchInlineSnapshot(`[]`); + expect(tree.orphans).toMatchInlineSnapshot(`[]`); - expect(String(tree.root)).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 054ad26f4f..72f34e8695 100644 --- a/packages/frontend-app-api/src/tree/resolveAppTree.ts +++ b/packages/frontend-app-api/src/tree/resolveAppTree.ts @@ -92,6 +92,22 @@ function makeRedirectKey(attachTo: { id: string; input: string }) { return `${attachTo.id}%${attachTo.input}`; } +const MAX_REDIRECT_DEPTH = 100; + +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. @@ -143,8 +159,10 @@ export function resolveAppTree( rootNode = node; } else { let attachTo = node.spec.attachTo; - while (redirectTargetsByKey.has(makeRedirectKey(attachTo))) { - attachTo = redirectTargetsByKey.get(makeRedirectKey(attachTo))!; + + if (!isValidAttachmentPoint(attachTo, nodes)) { + attachTo = + redirectTargetsByKey.get(makeRedirectKey(attachTo)) ?? attachTo; } const parent = nodes.get(attachTo.id); 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",