frontend-app-api: fix wiring of modules

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>

packages/app-next/src/App.tsx

@@ -26,8 +26,8 @@ import homePlugin, {
 import {
   coreExtensionData,
   createExtension,
-  createExtensionOverrides,
   ApiBlueprint,
+  createFrontendModule,
 } from '@backstage/frontend-plugin-api';
 import {
   techdocsPlugin,
@@ -45,7 +45,6 @@ import { createApiFactory, configApiRef } from '@backstage/core-plugin-api';
 import {
   ScmAuth,
   ScmIntegrationsApi,
-  scmAuthApiRef,
   scmIntegrationsApiRef,
 } from '@backstage/integration-react';
 import kubernetesPlugin from '@backstage/plugin-kubernetes/alpha';
@@ -97,34 +96,48 @@ const convertedTechdocsPlugin = convertLegacyPlugin(techdocsPlugin, {
   ],
 });
 
-const homePageExtension = createExtension({
-  name: 'myhomepage',
-  attachTo: { id: 'page:home', input: 'props' },
-  output: [coreExtensionData.reactElement, titleExtensionDataRef],
-  factory() {
-    return [
-      coreExtensionData.reactElement(homePage),
-      titleExtensionDataRef('just a title'),
-    ];
-  },
-});
-
-const scmAuthExtension = ApiBlueprint.make({
-  namespace: scmAuthApiRef.id,
-  params: {
-    factory: ScmAuth.createDefaultApiFactory(),
-  },
-});
-
-const scmIntegrationApi = ApiBlueprint.make({
-  namespace: scmIntegrationsApiRef.id,
-  params: {
-    factory: createApiFactory({
-      api: scmIntegrationsApiRef,
-      deps: { configApi: configApiRef },
-      factory: ({ configApi }) => ScmIntegrationsApi.fromConfig(configApi),
+const customHomePageModule = createFrontendModule({
+  pluginId: 'home',
+  extensions: [
+    createExtension({
+      name: 'my-home-page',
+      attachTo: { id: 'page:home', input: 'props' },
+      output: [coreExtensionData.reactElement, titleExtensionDataRef],
+      factory() {
+        return [
+          coreExtensionData.reactElement(homePage),
+          titleExtensionDataRef('just a title'),
+        ];
+      },
     }),
-  },
+  ],
+});
+
+const scmModule = createFrontendModule({
+  pluginId: 'app',
+  extensions: [
+    ApiBlueprint.make({
+      name: 'scm-auth',
+      params: {
+        factory: ScmAuth.createDefaultApiFactory(),
+      },
+    }),
+    ApiBlueprint.make({
+      name: 'scm-integrations',
+      params: {
+        factory: createApiFactory({
+          api: scmIntegrationsApiRef,
+          deps: { configApi: configApiRef },
+          factory: ({ configApi }) => ScmIntegrationsApi.fromConfig(configApi),
+        }),
+      },
+    }),
+  ],
+});
+
+const notFoundErrorPageModule = createFrontendModule({
+  pluginId: 'app',
+  extensions: [notFoundErrorPage],
 });
 
 const collectedLegacyPlugins = convertLegacyApp(
@@ -142,15 +155,10 @@ const app = createApp({
     appVisualizerPlugin,
     kubernetesPlugin,
     signInPageOverrides,
+    scmModule,
+    notFoundErrorPageModule,
+    customHomePageModule,
     ...collectedLegacyPlugins,
-    createExtensionOverrides({
-      extensions: [
-        homePageExtension,
-        scmAuthExtension,
-        scmIntegrationApi,
-        notFoundErrorPage,
-      ],
-    }),
   ],
   /* Handled through config instead */
   // bindRoutes({ bind }) {

packages/app-next/src/examples/notFoundErrorPageExtension.tsx

@@ -52,6 +52,7 @@ export function CustomNotFoundErrorPage() {
 }
 
 export default createComponentExtension({
+  name: 'not-found-error-page',
   ref: coreComponentRefs.notFoundErrorPage,
   loader: { sync: () => CustomNotFoundErrorPage },
 });

packages/frontend-app-api/src/tree/resolveAppNodeSpecs.test.ts

@@ -16,6 +16,7 @@
 
 import {
   createExtensionOverrides,
+  createFrontendModule,
   createFrontendPlugin,
   Extension,
   ExtensionDefinition,
@@ -38,7 +39,7 @@ function makeExt(
 }
 
 function makeExtDef(
-  name: string,
+  name: string | undefined = undefined,
   status: 'disabled' | 'enabled' = 'enabled',
   attachId: string = 'root',
 ) {
@@ -324,6 +325,56 @@ describe('resolveAppNodeSpecs', () => {
     ]);
   });
 
+  it('should apply module overrides', () => {
+    const plugin = createFrontendPlugin({
+      id: 'test',
+      extensions: [makeExtDef('a'), makeExtDef('b')],
+    });
+    const aOverride = makeExt('test/a', 'enabled', 'other');
+    const bOverride = makeExt('test/b', 'disabled', 'other');
+    const cOverride = makeExt('test/c');
+
+    expect(
+      resolveAppNodeSpecs({
+        features: [
+          plugin,
+          createFrontendModule({
+            pluginId: 'test',
+            extensions: [
+              makeExtDef('a', 'enabled', 'other'),
+              makeExtDef('b', 'disabled', 'other'),
+              makeExtDef('c'),
+            ],
+          }),
+        ],
+        builtinExtensions: [],
+        parameters: [],
+      }),
+    ).toEqual([
+      {
+        id: 'test/a',
+        extension: expect.objectContaining(aOverride),
+        attachTo: { id: 'other', input: 'default' },
+        source: plugin,
+        disabled: false,
+      },
+      {
+        id: 'test/b',
+        extension: expect.objectContaining(bOverride),
+        attachTo: { id: 'other', input: 'default' },
+        source: plugin,
+        disabled: true,
+      },
+      {
+        id: 'test/c',
+        extension: expect.objectContaining(cOverride),
+        attachTo: { id: 'root', input: 'default' },
+        source: plugin,
+        disabled: false,
+      },
+    ]);
+  });
+
   it('should use order from configuration when rather than overrides', () => {
     const result = resolveAppNodeSpecs({
       features: [
@@ -357,6 +408,40 @@ describe('resolveAppNodeSpecs', () => {
     ]);
   });
 
+  it('should use order from configuration when rather than modules', () => {
+    const result = resolveAppNodeSpecs({
+      features: [
+        createFrontendPlugin({
+          id: 'test',
+          extensions: [
+            makeExtDef('a', 'disabled'),
+            makeExtDef('b', 'disabled'),
+            makeExtDef('c', 'disabled'),
+          ],
+        }),
+        createFrontendModule({
+          pluginId: 'test',
+          extensions: [
+            makeExtDef('c', 'disabled'),
+            makeExtDef('b', 'disabled'),
+            makeExtDef('a', 'disabled'),
+          ],
+        }),
+      ],
+      builtinExtensions: [],
+      parameters: ['test/b', 'test/c', 'test/a'].map(id => ({
+        id,
+        disabled: false,
+      })),
+    });
+
+    expect(result.map(r => r.extension.id)).toEqual([
+      'test/b',
+      'test/c',
+      'test/a',
+    ]);
+  });
+
   it('throws an error when a forbidden extension is overridden by a plugin', () => {
     expect(() =>
       resolveAppNodeSpecs({
@@ -390,6 +475,28 @@ describe('resolveAppNodeSpecs', () => {
     );
   });
 
+  it('throws an error when a forbidden extension is overridden by module', () => {
+    expect(() =>
+      resolveAppNodeSpecs({
+        features: [
+          createFrontendPlugin({
+            id: 'forbidden',
+            extensions: [],
+          }),
+          createFrontendModule({
+            pluginId: 'forbidden',
+            extensions: [makeExtDef()],
+          }),
+        ],
+        builtinExtensions: [],
+        parameters: [],
+        forbidden: new Set(['forbidden']),
+      }),
+    ).toThrow(
+      "It is forbidden to override the following extension(s): 'forbidden', which is done by a module for the following plugin(s): 'forbidden'",
+    );
+  });
+
   it('throws an error when a forbidden extension is parametrized', () => {
     expect(() =>
       resolveAppNodeSpecs({

packages/frontend-app-api/src/tree/resolveAppNodeSpecs.ts

@@ -25,6 +25,11 @@ import {
   toInternalFrontendPlugin,
 } from '../../../frontend-plugin-api/src/wiring/createFrontendPlugin';
 // eslint-disable-next-line @backstage/no-relative-monorepo-imports
+import {
+  isInternalFrontendModule,
+  toInternalFrontendModule,
+} from '../../../frontend-plugin-api/src/wiring/createFrontendModule';
+// eslint-disable-next-line @backstage/no-relative-monorepo-imports
 import { toInternalExtension } from '../../../frontend-plugin-api/src/wiring/resolveExtensionDefinition';
 import { FrontendFeature } from '../wiring';
 
@@ -47,6 +52,7 @@ export function resolveAppNodeSpecs(options: {
     (f): f is ExtensionOverrides =>
       f.$$type === '@backstage/ExtensionOverrides',
   );
+  const modules = features.filter(isInternalFrontendModule);
 
   const pluginExtensions = plugins.flatMap(source => {
     return toInternalFrontendPlugin(source).extensions.map(extension => ({
@@ -54,6 +60,17 @@ export function resolveAppNodeSpecs(options: {
       source,
     }));
   });
+  const moduleExtensions = modules.flatMap(mod =>
+    toInternalFrontendModule(mod).extensions.flatMap(extension => {
+      // Modules for plugins that are not installed are ignored
+      const source = plugins.find(p => p.id === mod.pluginId);
+      if (!source) {
+        return [];
+      }
+
+      return [{ ...extension, source }];
+    }),
+  );
   const overrideExtensions = overrides.flatMap(
     override => toInternalExtensionOverrides(override).extensions,
   );
@@ -69,13 +86,23 @@ export function resolveAppNodeSpecs(options: {
       `It is forbidden to override the following extension(s): ${forbiddenStr}, which is done by the following plugin(s): ${pluginsStr}`,
     );
   }
-
+  if (moduleExtensions.some(({ id }) => forbidden.has(id))) {
+    const pluginsStr = moduleExtensions
+      .filter(({ id }) => forbidden.has(id))
+      .map(({ source }) => `'${source.id}'`)
+      .join(', ');
+    const forbiddenStr = [...forbidden].map(id => `'${id}'`).join(', ');
+    throw new Error(
+      `It is forbidden to override the following extension(s): ${forbiddenStr}, which is done by a module for the following plugin(s): ${pluginsStr}`,
+    );
+  }
   if (overrideExtensions.some(({ id }) => forbidden.has(id))) {
     const forbiddenStr = [...forbidden].map(id => `'${id}'`).join(', ');
     throw new Error(
       `It is forbidden to override the following extension(s): ${forbiddenStr}, which is done by one or more extension overrides`,
     );
   }
+
   const overrideExtensionIds = overrideExtensions.map(({ id }) => id);
   if (overrideExtensionIds.length !== new Set(overrideExtensionIds).size) {
     const counts = new Map<string, number>();
@@ -146,6 +173,33 @@ export function resolveAppNodeSpecs(options: {
     }
   }
 
+  // Install all module overrides
+  for (const extension of moduleExtensions) {
+    const internalExtension = toInternalExtension(extension);
+
+    // Check if our override is overriding an extension that already exists
+    const index = configuredExtensions.findIndex(
+      e => e.extension.id === extension.id,
+    );
+    if (index !== -1) {
+      // Only implementation, attachment point and default disabled status are overridden, the source is kept
+      configuredExtensions[index].extension = internalExtension;
+      configuredExtensions[index].params.attachTo = internalExtension.attachTo;
+      configuredExtensions[index].params.disabled = internalExtension.disabled;
+    } else {
+      // Add the extension as a new one when not overriding an existing one
+      configuredExtensions.push({
+        extension: internalExtension,
+        params: {
+          source: extension.source,
+          attachTo: internalExtension.attachTo,
+          disabled: internalExtension.disabled,
+          config: undefined,
+        },
+      });
+    }
+  }
+
   const duplicatedExtensionIds = new Set<string>();
   const duplicatedExtensionData = configuredExtensions.reduce<
     Record<string, Record<string, number>>