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 <poldsberg@gmail.com>

.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".

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<TId, TExtensions>): FrontendModule {
   const { pluginId } = options;
 
-  const extensions = new Array<Extension<any>>();
-  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',

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<Extension<any>>();
-  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,

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<ExtensionDefinition>,
+  context: { namespace: string; featureType: string },
+): {
+  extensions: Extension<any>[];
+  extensionDefinitionsById: Map<
+    string,
+    typeof OpaqueExtensionDefinition.TInternal
+  >;
+} {
+  const extensions = new Array<Extension<any>>();
+  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,