diff --git a/packages/frontend-app-api/src/wiring/createApp.test.tsx b/packages/frontend-app-api/src/wiring/createApp.test.tsx index 6fc294131c..903b044ed3 100644 --- a/packages/frontend-app-api/src/wiring/createApp.test.tsx +++ b/packages/frontend-app-api/src/wiring/createApp.test.tsx @@ -16,8 +16,6 @@ import { createExtension, - createExtensionDataRef, - createExtensionInput, createPageExtension, createPlugin, } from '@backstage/frontend-plugin-api'; @@ -72,228 +70,37 @@ describe('createInstances', () => { ); }); - describe('should deduplicated plugins', () => { - it('in an immediate dependency level (e.g., A -> A)', () => { - const config = new MockConfigApi({}); + it('throws an error when duplicated extensions are detected', () => { + const config = new MockConfigApi({}); - const addonExtensionData = - createExtensionDataRef('plugin.page.addon'); - - const CyclicalA = createExtension({ - id: 'A', // Same id as ancestor A - at: 'A/addons', - inputs: {}, - output: { - element: addonExtensionData, - }, - factory({ bind }) { - bind({ - element:
Cyclical A
, - }); - }, - }); - - const A = createPageExtension({ - id: 'A', - defaultPath: '/', - routeRef: createRouteRef({ id: 'A.route' }), - inputs: { - addons: createExtensionInput({ - element: addonExtensionData, - }), - }, - loader: async ({ inputs }) => ( -
A {inputs.addons.map(({ element }) => element)}
- ), - }); - - const plugins = [ - createPlugin({ - id: 'plugin', - extensions: [A, CyclicalA], - }), - ]; - - // It should not create an infinite loop - expect(() => createInstances({ config, plugins })).not.toThrow( - 'Maximum call stack size exceeded', - ); + const ExtensionA = createPageExtension({ + id: 'A', + defaultPath: '/', + routeRef: createRouteRef({ id: 'A.route' }), + loader: async () =>
Extension A
, }); - it('in an intermediate level (e.g., A -> B -> A)', () => { - const config = new MockConfigApi({}); - - const addonExtensionData = - createExtensionDataRef('plugin.page.addon'); - - const CyclicalA = createExtension({ - id: 'A', // Same id as ancestor A - at: 'B/addons', - inputs: { - addons: createExtensionInput({ - element: addonExtensionData, - }), - }, - output: { - element: addonExtensionData, - }, - factory({ bind }) { - bind({ - element:
Cyclical A
, - }); - }, - }); - - const B = createExtension({ - id: 'B', - at: 'A/addons', - inputs: { - addons: createExtensionInput({ - element: addonExtensionData, - }), - }, - output: { - element: addonExtensionData, - }, - factory({ bind }) { - bind({ - element:
B
, - }); - }, - }); - - const A = createPageExtension({ - id: 'A', - defaultPath: '/', - routeRef: createRouteRef({ id: 'A.route' }), - inputs: { - addons: createExtensionInput({ - element: addonExtensionData, - }), - }, - loader: async ({ inputs }) => ( -
A {inputs.addons.map(({ element }) => element)}
- ), - }); - - const plugins = [ - createPlugin({ - id: 'plugin', - extensions: [A, B, CyclicalA], - }), - ]; - - // It should not create an infinite loop - expect(() => createInstances({ config, plugins })).not.toThrow( - 'Maximum call stack size exceeded', - ); + const ExtensionB = createPageExtension({ + id: 'B', + defaultPath: '/', + routeRef: createRouteRef({ id: 'B.route' }), + loader: async () =>
Extension B
, }); - it('in an deep level (e.g., A -> B -> C -> D -> B)', () => { - const config = new MockConfigApi({}); - - const addonRendererInput = - createExtensionDataRef<() => JSX.Element>('A.addon.renderer'); - - const addonElementInput = - createExtensionDataRef('A.addon.element'); - - const CyclicalB = createExtension({ - id: 'B', // Same id as ancestor B - at: 'D/addons', - inputs: {}, - output: { - element: addonElementInput, - }, - factory({ bind }) { - bind({ - element:
Cyclical B
, - }); - }, - }); - - const D = createExtension({ - id: 'D', - at: 'C/addons', - inputs: { - addons: createExtensionInput({ - element: addonElementInput, - }), - }, - output: { - element: addonElementInput, - }, - factory({ bind }) { - bind({ - element:
D
, - }); - }, - }); - - const C = createExtension({ - id: 'C', - at: 'B/renderer', - inputs: { - addons: createExtensionInput({ - element: addonElementInput, - }), - }, - output: { - renderer: addonRendererInput, - }, - factory({ bind }) { - bind({ - renderer: () =>
C
, - }); - }, - }); - - const B = createExtension({ - id: 'B', - at: 'A/addons', - inputs: { - renderer: createExtensionInput( - { - element: addonRendererInput, - }, - { singleton: true, required: false }, - ), - }, - output: { - element: addonElementInput, - }, - factory({ bind, inputs }) { - bind({ - element: inputs.renderer?.element() ??
B
, - }); - }, - }); - - const A = createPageExtension({ - id: 'A', - defaultPath: '/', - routeRef: createRouteRef({ id: 'A.route' }), - inputs: { - addons: createExtensionInput({ - element: addonElementInput, - }), - }, - loader: async ({ inputs }) => ( -
A {inputs.addons.map(({ element }) => element)}
- ), - }); - - const plugins = [ - createPlugin({ - id: 'plugin', - extensions: [A, B, C, D, CyclicalB], - }), - ]; - - // It should not create an infinite loop - expect(() => createInstances({ config, plugins })).not.toThrow( - 'Maximum call stack size exceeded', - ); + const PluginA = createPlugin({ + id: 'A', + extensions: [ExtensionA, ExtensionA], }); + + const PluginB = createPlugin({ + id: 'B', + extensions: [ExtensionA, ExtensionB, ExtensionB], + }); + + const plugins = [PluginA, PluginB]; + + expect(() => createInstances({ config, plugins })).toThrow( + "The following extensions are duplicated: The extension 'A' was provided 2 time(s) by the plugin 'A' and 1 time(s) by the plugin 'B', The extension 'B' was provided 2 time(s) by the plugin 'B'", + ); }); }); diff --git a/packages/frontend-app-api/src/wiring/parameters.ts b/packages/frontend-app-api/src/wiring/parameters.ts index f2fba520c3..efe0fbf678 100644 --- a/packages/frontend-app-api/src/wiring/parameters.ts +++ b/packages/frontend-app-api/src/wiring/parameters.ts @@ -196,19 +196,8 @@ export function mergeExtensionParameters(options: { }): ExtensionInstanceParameters[] { const { sources, builtinExtensions, parameters } = options; - const pluginExtensionIds = new Set(); const pluginExtensions = sources.flatMap(source => { - return source.extensions - .filter(extension => { - // Filter out duplicated extensions - const id = extension.id; - if (!pluginExtensionIds.has(id)) { - pluginExtensionIds.add(id); - return true; - } - return false; - }) - .map(extension => ({ ...extension, source })); + return source.extensions.map(extension => ({ ...extension, source })); }); // Prevent root override @@ -244,6 +233,41 @@ export function mergeExtensionParameters(options: { })), ]; + const duplicatedExtensionIds = new Set(); + const duplicatedExtensionData = overrides.reduce< + Record> + >((data, { extension, params }) => { + const extensionId = extension.id; + const extensionData = data?.[extensionId]; + if (extensionData) duplicatedExtensionIds.add(extensionId); + const pluginId = params.source?.id ?? 'internal'; + const pluginCount = extensionData?.[pluginId] ?? 0; + return { + ...data, + [extensionId]: { ...extensionData, [pluginId]: pluginCount + 1 }, + }; + }, {}); + + if (duplicatedExtensionIds.size > 0) { + throw new Error( + `The following extensions are duplicated: ${Array.from( + duplicatedExtensionIds, + ) + .map( + extensionId => + `The extension '${extensionId}' was provided ${Object.keys( + duplicatedExtensionData[extensionId], + ) + .map( + pluginId => + `${duplicatedExtensionData[extensionId][pluginId]} time(s) by the plugin '${pluginId}'`, + ) + .join(' and ')}`, + ) + .join(', ')}`, + ); + } + for (const overrideParam of parameters) { const extensionId = overrideParam.id;