From 7041414b06ae580b07abda392e10568734516a53 Mon Sep 17 00:00:00 2001
From: Camila Belo <camilaibs@gmail.com>
Date: Thu, 21 Sep 2023 15:24:10 +0200
Subject: [PATCH] refactor(frontend-app-api): remove duplicated plugin
 extensions

Signed-off-by: Camila Belo <camilaibs@gmail.com>
---
 .changeset/wild-jobs-greet.md                 |  2 +-
 .../src/wiring/createApp.test.tsx             | 19 ++++++++++-------
 .../frontend-app-api/src/wiring/createApp.tsx | 21 +------------------
 .../frontend-app-api/src/wiring/parameters.ts | 17 ++++++++++++---
 4 files changed, 27 insertions(+), 32 deletions(-)

diff --git a/.changeset/wild-jobs-greet.md b/.changeset/wild-jobs-greet.md
index be0ddb98be..fdbe6d56ac 100644
--- a/.changeset/wild-jobs-greet.md
+++ b/.changeset/wild-jobs-greet.md
@@ -2,4 +2,4 @@
 '@backstage/frontend-app-api': patch
 ---
 
-Prevents root extension configuration and cyclic dependency between extensions when creating extension instances.
+Prevents root extension override and duplicated plugin extensions.
diff --git a/packages/frontend-app-api/src/wiring/createApp.test.tsx b/packages/frontend-app-api/src/wiring/createApp.test.tsx
index 5702077c89..6fc294131c 100644
--- a/packages/frontend-app-api/src/wiring/createApp.test.tsx
+++ b/packages/frontend-app-api/src/wiring/createApp.test.tsx
@@ -72,8 +72,8 @@ describe('createInstances', () => {
     );
   });
 
-  describe('throws an error when immediate cyclical dependencies are found', () => {
-    it('in an immediate level (e.g., A -> A)', () => {
+  describe('should deduplicated plugins', () => {
+    it('in an immediate dependency level (e.g., A -> A)', () => {
       const config = new MockConfigApi({});
 
       const addonExtensionData =
@@ -114,8 +114,9 @@ describe('createInstances', () => {
         }),
       ];
 
-      expect(() => createInstances({ config, plugins })).toThrow(
-        /There is a cyclical dependency with the extension "A": (.*) → A → A/,
+      // It should not create an infinite loop
+      expect(() => createInstances({ config, plugins })).not.toThrow(
+        'Maximum call stack size exceeded',
       );
     });
 
@@ -182,8 +183,9 @@ describe('createInstances', () => {
         }),
       ];
 
-      expect(() => createInstances({ config, plugins })).toThrow(
-        /There is a cyclical dependency with the extension "A": (.*) → A → B → A/,
+      // It should not create an infinite loop
+      expect(() => createInstances({ config, plugins })).not.toThrow(
+        'Maximum call stack size exceeded',
       );
     });
 
@@ -288,8 +290,9 @@ describe('createInstances', () => {
         }),
       ];
 
-      expect(() => createInstances({ config, plugins })).toThrow(
-        /There is a cyclical dependency with the extension "B": (.*) → A → B → C → D → B/,
+      // It should not create an infinite loop
+      expect(() => createInstances({ config, plugins })).not.toThrow(
+        'Maximum call stack size exceeded',
       );
     });
   });
diff --git a/packages/frontend-app-api/src/wiring/createApp.tsx b/packages/frontend-app-api/src/wiring/createApp.tsx
index abd135cc5f..462bfcdccc 100644
--- a/packages/frontend-app-api/src/wiring/createApp.tsx
+++ b/packages/frontend-app-api/src/wiring/createApp.tsx
@@ -208,7 +208,6 @@ export function createInstances(options: {
 
   function createInstance(
     instanceParams: ExtensionInstanceParameters,
-    extensionAncestorIds: string[] = [],
   ): ExtensionInstance {
     const extensionId = instanceParams.extension.id;
     const existingInstance = instances.get(extensionId);
@@ -216,28 +215,10 @@ export function createInstances(options: {
       return existingInstance;
     }
 
-    // Prevent cyclical dependencies
-    if (extensionAncestorIds.includes(extensionId)) {
-      const extensionDependencyGraph = extensionAncestorIds
-        .concat(extensionId)
-        .join(' → ');
-      throw new Error(
-        `There is a cyclical dependency with the extension "${extensionId}": ${extensionDependencyGraph}`,
-      );
-    }
-
     const attachments = new Map(
       Array.from(attachmentMap.get(extensionId)?.entries() ?? []).map(
         ([inputName, attachmentConfigs]) => {
-          return [
-            inputName,
-            attachmentConfigs.map(attachmentConfig =>
-              createInstance(
-                attachmentConfig,
-                extensionAncestorIds.concat(extensionId),
-              ),
-            ),
-          ];
+          return [inputName, attachmentConfigs.map(createInstance)];
         },
       ),
     );
diff --git a/packages/frontend-app-api/src/wiring/parameters.ts b/packages/frontend-app-api/src/wiring/parameters.ts
index 0605d34001..f2fba520c3 100644
--- a/packages/frontend-app-api/src/wiring/parameters.ts
+++ b/packages/frontend-app-api/src/wiring/parameters.ts
@@ -196,9 +196,20 @@ export function mergeExtensionParameters(options: {
 }): ExtensionInstanceParameters[] {
   const { sources, builtinExtensions, parameters } = options;
 
-  const pluginExtensions = sources.flatMap(source =>
-    source.extensions.map(extension => ({ ...extension, source })),
-  );
+  const pluginExtensionIds = new Set<string>();
+  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 }));
+  });
 
   // Prevent root override
   if (pluginExtensions.some(({ id }) => id === 'root')) {