From c1e9ca65004954350565fa055b4a6cfecc173184 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 9 Oct 2023 13:37:41 +0200 Subject: [PATCH 1/3] frontend-plugin-api: add createExtensionOverrides Signed-off-by: Patrik Oldsberg --- .changeset/silly-chefs-allow.md | 5 ++ packages/frontend-plugin-api/api-report.md | 17 ++++ .../wiring/createExtensionOverrides.test.ts | 82 +++++++++++++++++++ .../src/wiring/createExtensionOverrides.ts | 62 ++++++++++++++ .../frontend-plugin-api/src/wiring/index.ts | 5 ++ 5 files changed, 171 insertions(+) create mode 100644 .changeset/silly-chefs-allow.md create mode 100644 packages/frontend-plugin-api/src/wiring/createExtensionOverrides.test.ts create mode 100644 packages/frontend-plugin-api/src/wiring/createExtensionOverrides.ts diff --git a/.changeset/silly-chefs-allow.md b/.changeset/silly-chefs-allow.md new file mode 100644 index 0000000000..6a1ff5dc09 --- /dev/null +++ b/.changeset/silly-chefs-allow.md @@ -0,0 +1,5 @@ +--- +'@backstage/frontend-plugin-api': patch +--- + +Added `createExtensionOverrides` which can be used to install a collection of extensions in an app that will replace any existing ones. diff --git a/packages/frontend-plugin-api/api-report.md b/packages/frontend-plugin-api/api-report.md index 18baf88e5d..36de6114e6 100644 --- a/packages/frontend-plugin-api/api-report.md +++ b/packages/frontend-plugin-api/api-report.md @@ -159,6 +159,11 @@ export interface CreateExtensionOptions< output: TOutput; } +// @public (undocumented) +export function createExtensionOverrides( + options: ExtensionOverridesOptions, +): ExtensionOverrides; + // @public export function createNavItemExtension(options: { id: string; @@ -313,6 +318,18 @@ export type ExtensionInputValues< >; }; +// @public (undocumented) +export interface ExtensionOverrides { + // (undocumented) + $$type: '@backstage/ExtensionOverrides'; +} + +// @public (undocumented) +export interface ExtensionOverridesOptions { + // (undocumented) + extensions: Extension[]; +} + // @public (undocumented) export type NavTarget = { title: string; diff --git a/packages/frontend-plugin-api/src/wiring/createExtensionOverrides.test.ts b/packages/frontend-plugin-api/src/wiring/createExtensionOverrides.test.ts new file mode 100644 index 0000000000..fa430a7834 --- /dev/null +++ b/packages/frontend-plugin-api/src/wiring/createExtensionOverrides.test.ts @@ -0,0 +1,82 @@ +/* + * Copyright 2023 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 { createExtension } from './createExtension'; +import { + createExtensionOverrides, + toInternalExtensionOverrides, +} from './createExtensionOverrides'; + +describe('createExtensionOverrides', () => { + it('should create overrides without extensions', () => { + expect(createExtensionOverrides({ extensions: [] })).toMatchInlineSnapshot(` + { + "$$type": "@backstage/ExtensionOverrides", + "extensions": [], + "version": "v1", + } + `); + }); + + it('should create overrides with extensions', () => { + expect( + createExtensionOverrides({ + extensions: [ + createExtension({ + id: 'a', + attachTo: { id: 'core', input: 'apis' }, + output: {}, + factory() {}, + }), + ], + }), + ).toMatchInlineSnapshot(` + { + "$$type": "@backstage/ExtensionOverrides", + "extensions": [ + { + "$$type": "@backstage/Extension", + "attachTo": { + "id": "core", + "input": "apis", + }, + "disabled": false, + "factory": [Function], + "id": "a", + "inputs": {}, + "output": {}, + }, + ], + "version": "v1", + } + `); + }); + + it('should convert to internal overrides', () => { + const overrides = createExtensionOverrides({ + extensions: [ + createExtension({ + id: 'a', + attachTo: { id: 'core', input: 'apis' }, + output: {}, + factory() {}, + }), + ], + }); + const internal = toInternalExtensionOverrides(overrides); + expect(internal).toBe(overrides); + }); +}); diff --git a/packages/frontend-plugin-api/src/wiring/createExtensionOverrides.ts b/packages/frontend-plugin-api/src/wiring/createExtensionOverrides.ts new file mode 100644 index 0000000000..2c64229eb9 --- /dev/null +++ b/packages/frontend-plugin-api/src/wiring/createExtensionOverrides.ts @@ -0,0 +1,62 @@ +/* + * Copyright 2023 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 { Extension } from './createExtension'; + +/** @public */ +export interface ExtensionOverridesOptions { + extensions: Extension[]; +} + +/** @public */ +export interface ExtensionOverrides { + $$type: '@backstage/ExtensionOverrides'; +} + +/** @internal */ +export interface InternalExtensionOverrides extends ExtensionOverrides { + version: string; + extensions: Extension[]; +} + +/** @public */ +export function createExtensionOverrides( + options: ExtensionOverridesOptions, +): ExtensionOverrides { + return { + $$type: '@backstage/ExtensionOverrides', + version: 'v1', + extensions: options.extensions, + } as InternalExtensionOverrides; +} + +/** @internal */ +export function toInternalExtensionOverrides( + overrides: ExtensionOverrides, +): InternalExtensionOverrides { + const internal = overrides as InternalExtensionOverrides; + if (internal.$$type !== '@backstage/ExtensionOverrides') { + throw new Error( + `Invalid translation resource, bad type '${internal.$$type}'`, + ); + } + if (internal.version !== 'v1') { + throw new Error( + `Invalid translation resource, bad version '${internal.version}'`, + ); + } + return internal; +} diff --git a/packages/frontend-plugin-api/src/wiring/index.ts b/packages/frontend-plugin-api/src/wiring/index.ts index b7f0acefd9..4e4104de97 100644 --- a/packages/frontend-plugin-api/src/wiring/index.ts +++ b/packages/frontend-plugin-api/src/wiring/index.ts @@ -38,3 +38,8 @@ export { type BackstagePlugin, type PluginOptions, } from './createPlugin'; +export { + createExtensionOverrides, + type ExtensionOverrides, + type ExtensionOverridesOptions, +} from './createExtensionOverrides'; From d920b8c34300b1ff32a05f1aa3da475d476cdf85 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 9 Oct 2023 19:23:12 +0200 Subject: [PATCH 2/3] frontend-app-api: add support for installing extension overrides Signed-off-by: Patrik Oldsberg --- .changeset/tender-experts-yell.md | 5 ++ packages/app-next/src/App.tsx | 2 +- packages/frontend-app-api/api-report.md | 7 +- .../extractRouteInfoFromInstanceTree.test.ts | 2 +- .../src/wiring/createApp.test.tsx | 18 ++-- .../frontend-app-api/src/wiring/createApp.tsx | 37 +++++--- .../src/wiring/discovery.test.ts | 26 +++--- .../frontend-app-api/src/wiring/discovery.ts | 21 +++-- .../src/wiring/parameters.test.ts | 10 +-- .../frontend-app-api/src/wiring/parameters.ts | 85 ++++++++++++++++--- .../src/wiring/createPlugin.test.ts | 10 +-- plugins/search-react/src/alpha.test.tsx | 2 +- 12 files changed, 159 insertions(+), 66 deletions(-) create mode 100644 .changeset/tender-experts-yell.md diff --git a/.changeset/tender-experts-yell.md b/.changeset/tender-experts-yell.md new file mode 100644 index 0000000000..a4d321073b --- /dev/null +++ b/.changeset/tender-experts-yell.md @@ -0,0 +1,5 @@ +--- +'@backstage/frontend-app-api': minor +--- + +Added support for installing `ExtensionOverrides` via `createApp` options. As part of this change the `plugins` option has been renamed to `features`, and the `pluginLoader` has been renamed to `featureLoader`. diff --git a/packages/app-next/src/App.tsx b/packages/app-next/src/App.tsx index d844939cf2..01353feb4f 100644 --- a/packages/app-next/src/App.tsx +++ b/packages/app-next/src/App.tsx @@ -49,7 +49,7 @@ TODO: /* app.tsx */ const app = createApp({ - plugins: [graphiqlPlugin, pagesPlugin, techRadarPlugin], + features: [graphiqlPlugin, pagesPlugin, techRadarPlugin], // bindRoutes({ bind }) { // bind(catalogPlugin.externalRoutes, { // createComponent: scaffolderPlugin.routes.root, diff --git a/packages/frontend-app-api/api-report.md b/packages/frontend-app-api/api-report.md index 2e77ef904b..a576d4cfe4 100644 --- a/packages/frontend-app-api/api-report.md +++ b/packages/frontend-app-api/api-report.md @@ -7,13 +7,16 @@ import { BackstagePlugin } from '@backstage/frontend-plugin-api'; import { Config } from '@backstage/config'; import { ConfigApi } from '@backstage/core-plugin-api'; import { ExtensionDataRef } from '@backstage/frontend-plugin-api'; +import { ExtensionOverrides } from '@backstage/frontend-plugin-api'; import { JSX as JSX_2 } from 'react'; // @public (undocumented) export function createApp(options: { - plugins: BackstagePlugin[]; + features?: (BackstagePlugin | ExtensionOverrides)[]; configLoader?: () => Promise; - pluginLoader?: (ctx: { config: ConfigApi }) => Promise; + featureLoader?: (ctx: { + config: ConfigApi; + }) => Promise<(BackstagePlugin | ExtensionOverrides)[]>; }): { createRoot(): JSX_2.Element; }; diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts index a510adf1b7..446be51b5b 100644 --- a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts @@ -76,7 +76,7 @@ function routeInfoFromExtensions(extensions: Extension[]) { }); const { rootInstances } = createInstances({ config: new MockConfigApi({}), - plugins: [plugin], + features: [plugin], }); return extractRouteInfoFromInstanceTree(rootInstances); diff --git a/packages/frontend-app-api/src/wiring/createApp.test.tsx b/packages/frontend-app-api/src/wiring/createApp.test.tsx index abd0fe9b00..1a5aca673a 100644 --- a/packages/frontend-app-api/src/wiring/createApp.test.tsx +++ b/packages/frontend-app-api/src/wiring/createApp.test.tsx @@ -38,20 +38,20 @@ describe('createInstances', () => { ], }, }); - const plugins = [ + const features = [ createPlugin({ id: 'plugin', extensions: [], }), ]; - expect(() => createInstances({ config, plugins })).toThrow( + expect(() => createInstances({ config, features })).toThrow( "A 'root' extension configuration was detected, but the root extension is not configurable", ); }); it('throws an error when a root extension is overridden', () => { const config = new MockConfigApi({}); - const plugins = [ + const features = [ createPlugin({ id: 'plugin', extensions: [ @@ -65,7 +65,7 @@ describe('createInstances', () => { ], }), ]; - expect(() => createInstances({ config, plugins })).toThrow( + expect(() => createInstances({ config, features })).toThrow( "The following plugin(s) are overriding the 'root' extension which is forbidden: plugin", ); }); @@ -97,9 +97,9 @@ describe('createInstances', () => { extensions: [ExtensionA, ExtensionB, ExtensionB], }); - const plugins = [PluginA, PluginB]; + const features = [PluginA, PluginB]; - expect(() => createInstances({ config, plugins })).toThrow( + expect(() => createInstances({ config, features })).toThrow( "The following extensions are duplicated: The extension 'A' was provided 2 time(s) by the plugin 'A' and 1 time(s) by the plugin 'B', The extension 'B' was provided 2 time(s) by the plugin 'B'", ); }); @@ -114,7 +114,7 @@ describe('createApp', () => { extensions: [{ 'themes.light': false }, { 'themes.dark': false }], }, }), - plugins: [ + features: [ createPlugin({ id: 'test', extensions: [ @@ -137,7 +137,7 @@ describe('createApp', () => { it('should log an app', () => { const { rootInstances } = createInstances({ config: new MockConfigApi({}), - plugins: [], + features: [], }); const root = createExtensionInstance({ extension: createExtension({ @@ -175,7 +175,7 @@ describe('createApp', () => { it('should serialize an app as JSON', () => { const { rootInstances } = createInstances({ config: new MockConfigApi({}), - plugins: [], + features: [], }); const root = createExtensionInstance({ extension: createExtension({ diff --git a/packages/frontend-app-api/src/wiring/createApp.tsx b/packages/frontend-app-api/src/wiring/createApp.tsx index 02f3d17d7c..873e774ad8 100644 --- a/packages/frontend-app-api/src/wiring/createApp.tsx +++ b/packages/frontend-app-api/src/wiring/createApp.tsx @@ -20,6 +20,7 @@ import { BackstagePlugin, coreExtensionData, ExtensionDataRef, + ExtensionOverrides, } from '@backstage/frontend-plugin-api'; import { Core } from '../extensions/Core'; import { CoreRoutes } from '../extensions/CoreRoutes'; @@ -51,7 +52,7 @@ import { identityApiRef, AppTheme, } from '@backstage/core-plugin-api'; -import { getAvailablePlugins } from './discovery'; +import { getAvailableFeatures } from './discovery'; import { ApiFactoryRegistry, ApiProvider, @@ -104,9 +105,9 @@ export interface ExtensionTree { export function createExtensionTree(options: { config: Config; }): ExtensionTree { - const plugins = getAvailablePlugins(); + const features = getAvailableFeatures(); const { instances } = createInstances({ - plugins, + features, config: options.config, }); @@ -172,7 +173,7 @@ export function createExtensionTree(options: { * @internal */ export function createInstances(options: { - plugins: BackstagePlugin[]; + features: (BackstagePlugin | ExtensionOverrides)[]; config: Config; }) { const builtinExtensions = [ @@ -187,7 +188,7 @@ export function createInstances(options: { // pull in default extension instance from discovered packages // apply config to adjust default extension instances and add more const extensionParams = mergeExtensionParameters({ - sources: options.plugins, + features: options.features, builtinExtensions, parameters: readAppExtensionParameters(options.config), }); @@ -260,9 +261,11 @@ export function createInstances(options: { /** @public */ export function createApp(options: { - plugins: BackstagePlugin[]; + features?: (BackstagePlugin | ExtensionOverrides)[]; configLoader?: () => Promise; - pluginLoader?: (ctx: { config: ConfigApi }) => Promise; + featureLoader?: (ctx: { + config: ConfigApi; + }) => Promise<(BackstagePlugin | ExtensionOverrides)[]>; }): { createRoot(): JSX.Element; } { @@ -273,14 +276,18 @@ export function createApp(options: { overrideBaseUrlConfigs(defaultConfigLoaderSync()), ); - const discoveredPlugins = getAvailablePlugins(); - const loadedPlugins = (await options.pluginLoader?.({ config })) ?? []; - const allPlugins = Array.from( - new Set([...discoveredPlugins, ...options.plugins, ...loadedPlugins]), + const discoveredFeatures = getAvailableFeatures(); + const loadedFeatures = (await options.featureLoader?.({ config })) ?? []; + const allFeatures = Array.from( + new Set([ + ...discoveredFeatures, + ...(options.features ?? []), + ...loadedFeatures, + ]), ); const { rootInstances } = createInstances({ - plugins: allPlugins, + features: allFeatures, config, }); @@ -293,7 +300,11 @@ export function createApp(options: { const apiHolder = createApiHolder(coreInstance, config); - const appContext = createLegacyAppContext(allPlugins); + const appContext = createLegacyAppContext( + allFeatures.filter( + (f): f is BackstagePlugin => f.$$type === '@backstage/BackstagePlugin', + ), + ); const rootElements = rootInstances .map(e => ( diff --git a/packages/frontend-app-api/src/wiring/discovery.test.ts b/packages/frontend-app-api/src/wiring/discovery.test.ts index 5f1c4df718..54294bbd45 100644 --- a/packages/frontend-app-api/src/wiring/discovery.test.ts +++ b/packages/frontend-app-api/src/wiring/discovery.test.ts @@ -15,25 +15,25 @@ */ import { createPlugin } from '@backstage/frontend-plugin-api'; -import { getAvailablePlugins } from './discovery'; +import { getAvailableFeatures } from './discovery'; const globalSpy = jest.fn(); Object.defineProperty(global, '__@backstage/discovered__', { get: globalSpy, }); -describe('getAvailablePlugins', () => { +describe('getAvailableFeatures', () => { afterEach(jest.resetAllMocks); it('should discover nothing with undefined global', () => { - expect(getAvailablePlugins()).toEqual([]); + expect(getAvailableFeatures()).toEqual([]); }); it('should discover nothing with empty global', () => { globalSpy.mockReturnValue({ modules: [], }); - expect(getAvailablePlugins()).toEqual([]); + expect(getAvailableFeatures()).toEqual([]); }); it('should discover a plugin', () => { @@ -41,24 +41,24 @@ describe('getAvailablePlugins', () => { globalSpy.mockReturnValue({ modules: [{ default: testPlugin }], }); - expect(getAvailablePlugins()).toEqual([testPlugin]); + expect(getAvailableFeatures()).toEqual([testPlugin]); }); it('should ignore garbage', () => { globalSpy.mockReturnValueOnce({ modules: [{ default: null }] }); - expect(getAvailablePlugins()).toEqual([]); + expect(getAvailableFeatures()).toEqual([]); globalSpy.mockReturnValueOnce({ modules: [{ default: undefined }] }); - expect(getAvailablePlugins()).toEqual([]); + expect(getAvailableFeatures()).toEqual([]); globalSpy.mockReturnValueOnce({ modules: [{ default: Symbol() }] }); - expect(getAvailablePlugins()).toEqual([]); + expect(getAvailableFeatures()).toEqual([]); globalSpy.mockReturnValueOnce({ modules: [{ default: () => {} }] }); - expect(getAvailablePlugins()).toEqual([]); + expect(getAvailableFeatures()).toEqual([]); globalSpy.mockReturnValueOnce({ modules: [{ default: 0 }] }); - expect(getAvailablePlugins()).toEqual([]); + expect(getAvailableFeatures()).toEqual([]); globalSpy.mockReturnValueOnce({ modules: [{ default: false }] }); - expect(getAvailablePlugins()).toEqual([]); + expect(getAvailableFeatures()).toEqual([]); globalSpy.mockReturnValueOnce({ modules: [{ default: true }] }); - expect(getAvailablePlugins()).toEqual([]); + expect(getAvailableFeatures()).toEqual([]); }); it('should discover multiple plugins', () => { @@ -72,7 +72,7 @@ describe('getAvailablePlugins', () => { { default: test3Plugin }, ], }); - expect(getAvailablePlugins()).toEqual([ + expect(getAvailableFeatures()).toEqual([ test1Plugin, test2Plugin, test3Plugin, diff --git a/packages/frontend-app-api/src/wiring/discovery.ts b/packages/frontend-app-api/src/wiring/discovery.ts index 6d0b9282ea..14786c93d9 100644 --- a/packages/frontend-app-api/src/wiring/discovery.ts +++ b/packages/frontend-app-api/src/wiring/discovery.ts @@ -14,7 +14,10 @@ * limitations under the License. */ -import { BackstagePlugin } from '@backstage/frontend-plugin-api'; +import { + BackstagePlugin, + ExtensionOverrides, +} from '@backstage/frontend-plugin-api'; interface DiscoveryGlobal { modules: Array<{ name: string; default: unknown }>; @@ -23,19 +26,27 @@ interface DiscoveryGlobal { /** * @public */ -export function getAvailablePlugins(): BackstagePlugin[] { +export function getAvailableFeatures(): ( + | BackstagePlugin + | ExtensionOverrides +)[] { const discovered = ( window as { '__@backstage/discovered__'?: DiscoveryGlobal } )['__@backstage/discovered__']; return ( - discovered?.modules.map(m => m.default).filter(isBackstagePlugin) ?? [] + discovered?.modules.map(m => m.default).filter(isBackstageFeature) ?? [] ); } -function isBackstagePlugin(obj: unknown): obj is BackstagePlugin { +function isBackstageFeature( + obj: unknown, +): obj is BackstagePlugin | ExtensionOverrides { if (obj !== null && typeof obj === 'object' && '$$type' in obj) { - return obj.$$type === '@backstage/BackstagePlugin'; + return ( + obj.$$type === '@backstage/BackstagePlugin' || + obj.$$type === '@backstage/ExtensionOverrides' + ); } return false; } diff --git a/packages/frontend-app-api/src/wiring/parameters.test.ts b/packages/frontend-app-api/src/wiring/parameters.test.ts index 97ae941744..24d68301f1 100644 --- a/packages/frontend-app-api/src/wiring/parameters.test.ts +++ b/packages/frontend-app-api/src/wiring/parameters.test.ts @@ -35,7 +35,7 @@ describe('mergeExtensionParameters', () => { it('should filter out disabled extension instances', () => { expect( mergeExtensionParameters({ - sources: [], + features: [], builtinExtensions: [makeExt('a', 'disabled')], parameters: [], }), @@ -47,7 +47,7 @@ describe('mergeExtensionParameters', () => { const b = makeExt('b'); expect( mergeExtensionParameters({ - sources: [], + features: [], builtinExtensions: [a, b], parameters: [], }), @@ -63,7 +63,7 @@ describe('mergeExtensionParameters', () => { const pluginA = createPlugin({ id: 'test', extensions: [a] }); expect( mergeExtensionParameters({ - sources: [pluginA], + features: [pluginA], builtinExtensions: [b], parameters: [ { @@ -88,7 +88,7 @@ describe('mergeExtensionParameters', () => { const plugin = createPlugin({ id: 'test', extensions: [a, b] }); expect( mergeExtensionParameters({ - sources: [plugin], + features: [plugin], builtinExtensions: [], parameters: [ { @@ -126,7 +126,7 @@ describe('mergeExtensionParameters', () => { const b = makeExt('b', 'disabled'); expect( mergeExtensionParameters({ - sources: [createPlugin({ id: 'empty', extensions: [] })], + features: [createPlugin({ id: 'empty', extensions: [] })], builtinExtensions: [a, b], parameters: [ { diff --git a/packages/frontend-app-api/src/wiring/parameters.ts b/packages/frontend-app-api/src/wiring/parameters.ts index 9474a01d79..e81d5f74cb 100644 --- a/packages/frontend-app-api/src/wiring/parameters.ts +++ b/packages/frontend-app-api/src/wiring/parameters.ts @@ -15,7 +15,13 @@ */ import { Config } from '@backstage/config'; -import { BackstagePlugin, Extension } from '@backstage/frontend-plugin-api'; +import { + BackstagePlugin, + Extension, + ExtensionOverrides, +} from '@backstage/frontend-plugin-api'; +// eslint-disable-next-line @backstage/no-relative-monorepo-imports +import { toInternalExtensionOverrides } from '../../../frontend-plugin-api/src/wiring/createExtensionOverrides'; import { JsonValue } from '@backstage/types'; export interface ExtensionParameters { @@ -208,15 +214,26 @@ export interface ExtensionInstanceParameters { /** @internal */ export function mergeExtensionParameters(options: { - sources: BackstagePlugin[]; + features: (BackstagePlugin | ExtensionOverrides)[]; builtinExtensions: Extension[]; parameters: Array; }): ExtensionInstanceParameters[] { - const { sources, builtinExtensions, parameters } = options; + const { builtinExtensions, parameters } = options; - const pluginExtensions = sources.flatMap(source => { + const plugins = options.features.filter( + (f): f is BackstagePlugin => f.$$type === '@backstage/BackstagePlugin', + ); + const overrides = options.features.filter( + (f): f is ExtensionOverrides => + f.$$type === '@backstage/ExtensionOverrides', + ); + + const pluginExtensions = plugins.flatMap(source => { return source.extensions.map(extension => ({ ...extension, source })); }); + const overrideExtensions = overrides.flatMap( + override => toInternalExtensionOverrides(override).extensions, + ); // Prevent root override if (pluginExtensions.some(({ id }) => id === 'root')) { @@ -230,7 +247,28 @@ export function mergeExtensionParameters(options: { ); } - const overrides = [ + if (overrideExtensions.some(({ id }) => id === 'root')) { + throw new Error( + `An extension override is overriding the 'root' extension which is forbidden`, + ); + } + const overrideExtensionIds = overrideExtensions.map(({ id }) => id); + if (overrideExtensionIds.length !== new Set(overrideExtensionIds).size) { + const counts = new Map(); + for (const id of overrideExtensionIds) { + counts.set(id, (counts.get(id) ?? 0) + 1); + } + const duplicated = Array.from(counts.entries()) + .filter(([, count]) => count > 1) + .map(([id]) => id); + throw new Error( + `The following extensions had duplicate overrides: ${duplicated.join( + ', ', + )}`, + ); + } + + const configuredExtensions = [ ...pluginExtensions.map(({ source, ...extension }) => ({ extension, params: { @@ -251,8 +289,33 @@ export function mergeExtensionParameters(options: { })), ]; + // Install all extension overrides + for (const extension of overrideExtensions) { + // 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 = extension; + configuredExtensions[index].params.attachTo = extension.attachTo; + configuredExtensions[index].params.disabled = extension.disabled; + } else { + // Add the extension as a new one when not overriding an existing one + configuredExtensions.push({ + extension, + params: { + source: undefined, + attachTo: extension.attachTo, + disabled: extension.disabled, + config: undefined, + }, + }); + } + } + const duplicatedExtensionIds = new Set(); - const duplicatedExtensionData = overrides.reduce< + const duplicatedExtensionData = configuredExtensions.reduce< Record> >((data, { extension, params }) => { const extensionId = extension.id; @@ -296,11 +359,11 @@ export function mergeExtensionParameters(options: { ); } - const existingIndex = overrides.findIndex( + const existingIndex = configuredExtensions.findIndex( e => e.extension.id === extensionId, ); if (existingIndex !== -1) { - const existing = overrides[existingIndex]; + const existing = configuredExtensions[existingIndex]; if (overrideParam.attachTo) { existing.params.attachTo = overrideParam.attachTo; } @@ -314,8 +377,8 @@ export function mergeExtensionParameters(options: { existing.params.disabled = Boolean(overrideParam.disabled); if (!existing.params.disabled) { // bump - overrides.splice(existingIndex, 1); - overrides.push(existing); + configuredExtensions.splice(existingIndex, 1); + configuredExtensions.push(existing); } } } else { @@ -323,7 +386,7 @@ export function mergeExtensionParameters(options: { } } - return overrides + return configuredExtensions .filter(override => !override.params.disabled) .map(param => ({ extension: param.extension, diff --git a/packages/frontend-plugin-api/src/wiring/createPlugin.test.ts b/packages/frontend-plugin-api/src/wiring/createPlugin.test.ts index 589ac59d75..07841b3a9a 100644 --- a/packages/frontend-plugin-api/src/wiring/createPlugin.test.ts +++ b/packages/frontend-plugin-api/src/wiring/createPlugin.test.ts @@ -104,14 +104,14 @@ const outputExtension = createExtension({ }); function createTestAppRoot({ - plugins, + features, config = {}, }: { - plugins: BackstagePlugin[]; + features: BackstagePlugin[]; config: JsonObject; }) { return createApp({ - plugins: plugins, + features, configLoader: async () => new MockConfigApi(config), }).createRoot(); } @@ -132,7 +132,7 @@ describe('createPlugin', () => { await renderWithEffects( createTestAppRoot({ - plugins: [plugin], + features: [plugin], config: { app: { extensions: [{ 'core.layout': false }] } }, }), ); @@ -157,7 +157,7 @@ describe('createPlugin', () => { await renderWithEffects( createTestAppRoot({ - plugins: [plugin], + features: [plugin], config: { app: { extensions: [ diff --git a/plugins/search-react/src/alpha.test.tsx b/plugins/search-react/src/alpha.test.tsx index 8a88c9cc5d..5222e4dad5 100644 --- a/plugins/search-react/src/alpha.test.tsx +++ b/plugins/search-react/src/alpha.test.tsx @@ -168,7 +168,7 @@ describe('createSearchResultListItemExtension', () => { }); const app = createApp({ - plugins: [SearchPlugin], + features: [SearchPlugin], configLoader: async () => new MockConfigApi({ app: { From 85840e0a1f3de29aaa6e4d163c6d614015fcb938 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 9 Oct 2023 19:42:52 +0200 Subject: [PATCH 3/3] frontend-app-api: add tests for extension overrides Signed-off-by: Patrik Oldsberg --- .../src/wiring/createApp.test.tsx | 28 ++++++++ .../src/wiring/parameters.test.ts | 69 ++++++++++++++++++- 2 files changed, 94 insertions(+), 3 deletions(-) diff --git a/packages/frontend-app-api/src/wiring/createApp.test.tsx b/packages/frontend-app-api/src/wiring/createApp.test.tsx index 1a5aca673a..b4107f0249 100644 --- a/packages/frontend-app-api/src/wiring/createApp.test.tsx +++ b/packages/frontend-app-api/src/wiring/createApp.test.tsx @@ -16,6 +16,7 @@ import { createExtension, + createExtensionOverrides, createPageExtension, createPlugin, createThemeExtension, @@ -27,6 +28,13 @@ import React from 'react'; import { createRouteRef } from '@backstage/core-plugin-api'; import { createExtensionInstance } from './createExtensionInstance'; +const extBaseConfig = { + id: 'test', + attachTo: { id: 'root', input: 'default' }, + output: {}, + factory() {}, +}; + describe('createInstances', () => { it('throws an error when a root extension is parametrized', () => { const config = new MockConfigApi({ @@ -103,6 +111,26 @@ describe('createInstances', () => { "The following extensions are duplicated: The extension 'A' was provided 2 time(s) by the plugin 'A' and 1 time(s) by the plugin 'B', The extension 'B' was provided 2 time(s) by the plugin 'B'", ); }); + + it('throws an error when duplicated extension overrides are detected', () => { + expect(() => + createInstances({ + config: new MockConfigApi({}), + features: [ + createExtensionOverrides({ + extensions: [ + createExtension({ ...extBaseConfig, id: 'a' }), + createExtension({ ...extBaseConfig, id: 'a' }), + createExtension({ ...extBaseConfig, id: 'b' }), + ], + }), + createExtensionOverrides({ + extensions: [createExtension({ ...extBaseConfig, id: 'b' })], + }), + ], + }), + ).toThrow('The following extensions had duplicate overrides: a, b'); + }); }); describe('createApp', () => { diff --git a/packages/frontend-app-api/src/wiring/parameters.test.ts b/packages/frontend-app-api/src/wiring/parameters.test.ts index 24d68301f1..4ee420f667 100644 --- a/packages/frontend-app-api/src/wiring/parameters.test.ts +++ b/packages/frontend-app-api/src/wiring/parameters.test.ts @@ -15,7 +15,11 @@ */ import { ConfigReader } from '@backstage/config'; -import { createPlugin, Extension } from '@backstage/frontend-plugin-api'; +import { + createExtensionOverrides, + createPlugin, + Extension, +} from '@backstage/frontend-plugin-api'; import { JsonValue } from '@backstage/types'; import { expandShorthandExtensionParameters, @@ -23,10 +27,14 @@ import { readAppExtensionParameters, } from './parameters'; -function makeExt(id: string, status: 'disabled' | 'enabled' = 'enabled') { +function makeExt( + id: string, + status: 'disabled' | 'enabled' = 'enabled', + attachId: string = 'root', +) { return { id, - attachTo: { id: 'root', input: 'default' }, + attachTo: { id: attachId, input: 'default' }, disabled: status === 'disabled', } as Extension; } @@ -144,6 +152,61 @@ describe('mergeExtensionParameters', () => { { extension: a, attachTo: { id: 'root', input: 'default' } }, ]); }); + + it('should apply extension overrides', () => { + const a = makeExt('a'); + const b = makeExt('b'); + const plugin = createPlugin({ id: 'test', extensions: [a, b] }); + const aOverride = makeExt('a', 'enabled', 'other'); + const bOverride = makeExt('b', 'disabled', 'other'); + const cOverride = makeExt('c'); + + const result = mergeExtensionParameters({ + features: [ + plugin, + createExtensionOverrides({ + extensions: [aOverride, bOverride, cOverride], + }), + ], + builtinExtensions: [], + parameters: [], + }); + + expect(result.length).toBe(2); + expect(result[0].extension).toBe(aOverride); + expect(result[0].attachTo).toEqual({ id: 'other', input: 'default' }); + expect(result[0].config).toEqual(undefined); + expect(result[0].source).toBe(plugin); + + expect(result[1]).toEqual({ + extension: cOverride, + attachTo: { id: 'root', input: 'default' }, + config: undefined, + source: undefined, + }); + }); + + it('should use order from configuration when rather than overrides', () => { + const a = makeExt('a', 'disabled'); + const b = makeExt('b', 'disabled'); + const c = makeExt('c', 'disabled'); + const aOverride = makeExt('c', 'disabled'); + const bOverride = makeExt('b', 'disabled'); + const cOverride = makeExt('a', 'disabled'); + + const result = mergeExtensionParameters({ + features: [ + createPlugin({ id: 'test', extensions: [a, b, c] }), + createExtensionOverrides({ + extensions: [cOverride, bOverride, aOverride], + }), + ], + builtinExtensions: [], + parameters: ['b', 'c', 'a'].map(id => ({ id, disabled: false })), + }); + + expect(result.map(r => r.extension.id)).toEqual(['b', 'c', 'a']); + }); }); describe('readAppExtensionParameters', () => {