From 4c099673179ad1ce77c540d921ecbfe373813094 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 13 Apr 2026 13:58:59 +0200 Subject: [PATCH] Deduplicate frontend plugin/module extension collection logic (#33869) Extract the shared extension resolution and duplicate-check logic from createFrontendPlugin and createFrontendModule into a new resolveExtensionDefinitions helper. Also fixes the duplicate extension error message in createFrontendModule to say "Module" instead of "Plugin". Made-with: Cursor Signed-off-by: Patrik Oldsberg --- .../deduplicate-extension-collection.md | 5 ++ .../src/wiring/createFrontendModule.ts | 37 ++----------- .../src/wiring/createFrontendPlugin.ts | 42 +++------------ .../src/wiring/resolveExtensionDefinition.ts | 52 +++++++++++++++++++ 4 files changed, 70 insertions(+), 66 deletions(-) create mode 100644 .changeset/deduplicate-extension-collection.md diff --git a/.changeset/deduplicate-extension-collection.md b/.changeset/deduplicate-extension-collection.md new file mode 100644 index 0000000000..7df9ed61fb --- /dev/null +++ b/.changeset/deduplicate-extension-collection.md @@ -0,0 +1,5 @@ +--- +'@backstage/frontend-plugin-api': patch +--- + +Fixed the duplicate extension error message in `createFrontendModule` to correctly say "Module" instead of "Plugin". diff --git a/packages/frontend-plugin-api/src/wiring/createFrontendModule.ts b/packages/frontend-plugin-api/src/wiring/createFrontendModule.ts index 1d77ce2167..5d435102d9 100644 --- a/packages/frontend-plugin-api/src/wiring/createFrontendModule.ts +++ b/packages/frontend-plugin-api/src/wiring/createFrontendModule.ts @@ -14,11 +14,10 @@ * limitations under the License. */ -import { OpaqueExtensionDefinition } from '@internal/frontend'; import { ExtensionDefinition } from './createExtension'; import { Extension, - resolveExtensionDefinition, + resolveExtensionDefinitions, } from './resolveExtensionDefinition'; import { FeatureFlagConfig } from './types'; import { FilterPredicate } from '@backstage/filter-predicates'; @@ -93,36 +92,10 @@ export function createFrontendModule< >(options: CreateFrontendModuleOptions): FrontendModule { const { pluginId } = options; - const extensions = new Array>(); - const extensionDefinitionsById = new Map< - string, - typeof OpaqueExtensionDefinition.TInternal - >(); - - for (const def of options.extensions ?? []) { - const internal = OpaqueExtensionDefinition.toInternal(def); - const ext = resolveExtensionDefinition(def, { namespace: pluginId }); - extensions.push(ext); - extensionDefinitionsById.set(ext.id, { - ...internal, - namespace: pluginId, - }); - } - - if (extensions.length !== extensionDefinitionsById.size) { - const extensionIds = extensions.map(e => e.id); - const duplicates = Array.from( - new Set( - extensionIds.filter((id, index) => extensionIds.indexOf(id) !== index), - ), - ); - // TODO(Rugvip): This could provide some more information about the kind + name of the extensions - throw new Error( - `Plugin '${pluginId}' provided duplicate extensions: ${duplicates.join( - ', ', - )}`, - ); - } + const { extensions } = resolveExtensionDefinitions(options.extensions ?? [], { + namespace: pluginId, + featureType: 'Module', + }); return { $$type: '@backstage/FrontendModule', diff --git a/packages/frontend-plugin-api/src/wiring/createFrontendPlugin.ts b/packages/frontend-plugin-api/src/wiring/createFrontendPlugin.ts index 9e9c58c4b6..9c6095e732 100644 --- a/packages/frontend-plugin-api/src/wiring/createFrontendPlugin.ts +++ b/packages/frontend-plugin-api/src/wiring/createFrontendPlugin.ts @@ -14,17 +14,14 @@ * limitations under the License. */ -import { - OpaqueExtensionDefinition, - OpaqueFrontendPlugin, -} from '@internal/frontend'; +import { OpaqueFrontendPlugin } from '@internal/frontend'; import { ExtensionDefinition, OverridableExtensionDefinition, } from './createExtension'; import { - Extension, resolveExtensionDefinition, + resolveExtensionDefinitions, } from './resolveExtensionDefinition'; import { FeatureFlagConfig } from './types'; import { MakeSortedExtensionsMap } from './MakeSortedExtensionsMap'; @@ -272,36 +269,13 @@ export function createFrontendPlugin< ); } - const extensions = new Array>(); - const extensionDefinitionsById = new Map< - string, - typeof OpaqueExtensionDefinition.TInternal - >(); - - for (const def of options.extensions ?? []) { - const internal = OpaqueExtensionDefinition.toInternal(def); - const ext = resolveExtensionDefinition(def, { namespace: pluginId }); - extensions.push(ext); - extensionDefinitionsById.set(ext.id, { - ...internal, + const { extensions, extensionDefinitionsById } = resolveExtensionDefinitions( + options.extensions ?? [], + { namespace: pluginId, - }); - } - - if (extensions.length !== extensionDefinitionsById.size) { - const extensionIds = extensions.map(e => e.id); - const duplicates = Array.from( - new Set( - extensionIds.filter((id, index) => extensionIds.indexOf(id) !== index), - ), - ); - // TODO(Rugvip): This could provide some more information about the kind + name of the extensions - throw new Error( - `Plugin '${pluginId}' provided duplicate extensions: ${duplicates.join( - ', ', - )}`, - ); - } + featureType: 'Plugin', + }, + ); return OpaqueFrontendPlugin.createInstance('v1', { pluginId, diff --git a/packages/frontend-plugin-api/src/wiring/resolveExtensionDefinition.ts b/packages/frontend-plugin-api/src/wiring/resolveExtensionDefinition.ts index a14c4e7863..55ecf314d3 100644 --- a/packages/frontend-plugin-api/src/wiring/resolveExtensionDefinition.ts +++ b/packages/frontend-plugin-api/src/wiring/resolveExtensionDefinition.ts @@ -184,6 +184,58 @@ function resolveAttachTo( return resolveSpec(attachTo); } +/** + * Resolves a list of extension definitions into extensions, returning both the + * resolved extensions and a map of extension definitions keyed by resolved ID. + * Throws if any two definitions resolve to the same ID. + * + * @internal + */ +export function resolveExtensionDefinitions( + definitions: Iterable, + context: { namespace: string; featureType: string }, +): { + extensions: Extension[]; + extensionDefinitionsById: Map< + string, + typeof OpaqueExtensionDefinition.TInternal + >; +} { + const extensions = new Array>(); + const extensionDefinitionsById = new Map< + string, + typeof OpaqueExtensionDefinition.TInternal + >(); + + for (const def of definitions) { + const internal = OpaqueExtensionDefinition.toInternal(def); + const ext = resolveExtensionDefinition(def, { + namespace: context.namespace, + }); + extensions.push(ext); + extensionDefinitionsById.set(ext.id, { + ...internal, + namespace: context.namespace, + }); + } + + if (extensions.length !== extensionDefinitionsById.size) { + const extensionIds = extensions.map(e => e.id); + const duplicates = Array.from( + new Set( + extensionIds.filter((id, index) => extensionIds.indexOf(id) !== index), + ), + ); + throw new Error( + `${context.featureType} '${ + context.namespace + }' provided duplicate extensions: ${duplicates.join(', ')}`, + ); + } + + return { extensions, extensionDefinitionsById }; +} + /** @internal */ export function resolveExtensionDefinition< T extends ExtensionDefinitionParameters,