diff --git a/.changeset/spicy-vans-eat.md b/.changeset/spicy-vans-eat.md new file mode 100644 index 0000000000..037a5f18e0 --- /dev/null +++ b/.changeset/spicy-vans-eat.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-app-api': patch +--- + +Fix feature loaders in CJS double-default nested builds diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.test.ts b/packages/backend-app-api/src/wiring/BackendInitializer.test.ts index cf8845fcec..c3c42ca423 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.test.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.test.ts @@ -576,4 +576,59 @@ describe('BackendInitializer', () => { "Illegal dependency: Module 'mod' for plugin 'test' attempted to depend on extension point 'a' for plugin 'test-a'. Extension points can only be used within their plugin's scope.", ); }); + + it('should properly load double-default CJS modules', async () => { + expect.assertions(3); + + const init = new BackendInitializer(baseFactories); + init.add( + createBackendFeatureLoader({ + loader() { + return [ + createBackendPlugin({ + pluginId: 'no-double-wrapping', + register(reg) { + reg.registerInit({ + deps: {}, + async init() { + expect(true).toBeTruthy(); + }, + }); + }, + }), + { + default: createBackendPlugin({ + pluginId: 'single-wrapping', + register(reg) { + reg.registerInit({ + deps: {}, + async init() { + expect(true).toBeTruthy(); + }, + }); + }, + }), + }, + { + default: { + default: createBackendPlugin({ + pluginId: 'double-wrapping', + register(reg) { + reg.registerInit({ + deps: {}, + async init() { + expect(true).toBeTruthy(); + }, + }); + }, + }), + }, + } as any, // not typescript valid, but can happen at runtime + ]; + }, + }), + ); + + await init.start(); + }); }); diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.ts b/packages/backend-app-api/src/wiring/BackendInitializer.ts index c0da9eda62..b2173fb105 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.ts @@ -38,6 +38,7 @@ import { featureDiscoveryServiceRef } from '@backstage/backend-plugin-api/alpha' import { DependencyGraph } from '../lib/DependencyGraph'; import { ServiceRegistry } from './ServiceRegistry'; import { createInitializationLogger } from './createInitializationLogger'; +import { unwrapFeature } from './helpers'; export interface BackendRegisterInit { consumes: Set; @@ -163,7 +164,7 @@ export class BackendInitializer { if (featureDiscovery) { const { features } = await featureDiscovery.getBackendFeatures(); for (const feature of features) { - this.#addFeature(feature); + this.#addFeature(unwrapFeature(feature)); } this.#serviceRegistry.checkForCircularDeps(); } @@ -417,6 +418,7 @@ export class BackendInitializer { const result = await loader .loader(Object.fromEntries(deps)) + .then(features => features.map(unwrapFeature)) .catch(error => { throw new ForwardedError( `Feature loader ${loader.description} failed`, diff --git a/packages/backend-app-api/src/wiring/BackstageBackend.ts b/packages/backend-app-api/src/wiring/BackstageBackend.ts index 9df7549bcb..49a8bbbbfb 100644 --- a/packages/backend-app-api/src/wiring/BackstageBackend.ts +++ b/packages/backend-app-api/src/wiring/BackstageBackend.ts @@ -16,6 +16,7 @@ import { BackendFeature, ServiceFactory } from '@backstage/backend-plugin-api'; import { BackendInitializer } from './BackendInitializer'; +import { unwrapFeature } from './helpers'; import { Backend } from './types'; export class BackstageBackend implements Backend { @@ -50,21 +51,3 @@ function isPromise(value: unknown | Promise): value is Promise { typeof value.then === 'function' ); } - -function unwrapFeature( - feature: BackendFeature | { default: BackendFeature }, -): BackendFeature { - if ('$$type' in feature) { - return feature; - } - - // This is a workaround where default exports get transpiled to `exports['default'] = ...` - // in CommonJS modules, which in turn results in a double `{ default: { default: ... } }` nesting - // when importing using a dynamic import. - // TODO: This is a broader issue than just this piece of code, and should move away from CommonJS. - if ('default' in feature) { - return feature.default; - } - - return feature; -} diff --git a/packages/backend-app-api/src/wiring/helpers.ts b/packages/backend-app-api/src/wiring/helpers.ts new file mode 100644 index 0000000000..ffb7e3b079 --- /dev/null +++ b/packages/backend-app-api/src/wiring/helpers.ts @@ -0,0 +1,36 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { BackendFeature } from '@backstage/backend-plugin-api'; + +/** @internal */ +export function unwrapFeature( + feature: BackendFeature | { default: BackendFeature }, +): BackendFeature { + if ('$$type' in feature) { + return feature; + } + + // This is a workaround where default exports get transpiled to `exports['default'] = ...` + // in CommonJS modules, which in turn results in a double `{ default: { default: ... } }` nesting + // when importing using a dynamic import. + // TODO: This is a broader issue than just this piece of code, and should move away from CommonJS. + if ('default' in feature) { + return feature.default; + } + + return feature; +}