diff --git a/packages/frontend-app-api/src/createApp.tsx b/packages/frontend-app-api/src/createApp.tsx index 46cd0a7aed..ecc86a5ed7 100644 --- a/packages/frontend-app-api/src/createApp.tsx +++ b/packages/frontend-app-api/src/createApp.tsx @@ -15,9 +15,9 @@ */ import React from 'react'; -import { Config, ConfigReader } from '@backstage/config'; +import { ConfigReader } from '@backstage/config'; import { - ExtensionInstanceConfig, + ExtensionInstanceParameters, BackstagePlugin, coreExtensionData, } from '@backstage/frontend-plugin-api'; @@ -26,65 +26,10 @@ import { createExtensionInstance, ExtensionInstance, } from './createExtensionInstance'; - -// Since we'll never merge arrays in config the config reader context -// isn't too much of a help. Fall back to manual config reading logic -// as the Config interface makes it quite hard for us otherwise. -function readAppExtensionConfigs( - rootConfig: Config, -): Partial[] { - const arr = rootConfig.getOptional('app.extensions'); - if (!Array.isArray(arr)) { - if (arr === undefined) { - return []; - } - // This will throw, and show which part of config had the wrong type - rootConfig.getConfigArray('app.extensions'); - return []; - } - - return arr.map((value, index) => { - function errorMsg(msg: string, key?: string, prop?: string) { - return `Invalid extension configuration at app.extensions[${index}]${ - key ? `[${key}]` : '' - }${prop ? `.${prop}` : ''}, ${msg}`; - } - - if (typeof value === 'string') { - return { id: value }; - } else if ( - typeof value !== 'object' || - value === null || - Array.isArray(value) - ) { - throw new Error(errorMsg('must be a string or an object')); - } - - const keys = Object.keys(value); - if (keys.length !== 1) { - const joinedKeys = `"${keys.join('", "')}"`; - throw new Error(errorMsg(`must have exactly one key, got ${joinedKeys}`)); - } - - const key = keys[0]; - const obj = value[key]; - if (typeof obj !== 'object' || obj === null || Array.isArray(obj)) { - throw new Error(errorMsg('must be an object', key)); - } - const at = obj.at; - if (at !== undefined && typeof at !== 'string') { - throw new Error(errorMsg('must be a string', key, 'at')); - } - const extension = obj.extension; - if (extension !== undefined && typeof extension !== 'string') { - throw new Error(errorMsg('must be a string', key, 'extension')); - } - if (extension) { - throw new Error('TODO: implement extension resolution'); - } - return { id: key, at, config: obj.config /* validate later */ }; - }); -} +import { + mergeExtensionParameters, + readAppExtensionParameters, +} from './wiring/parameters'; /** @public */ export function createApp(options: { plugins: BackstagePlugin[] }): { @@ -92,10 +37,7 @@ export function createApp(options: { plugins: BackstagePlugin[] }): { } { const appConfig = ConfigReader.fromConfigs(process.env.APP_CONFIG as any); - // pull in default extension instance from discovered packages - // apply config to adjust default extension instances and add more - const extensionInstanceConfigs = [ - ...options.plugins.flatMap(plugin => plugin.defaultExtensionInstances), + const builtinExtensionInstanceParams = [ { id: 'core.router', at: 'root/default', @@ -104,40 +46,23 @@ export function createApp(options: { plugins: BackstagePlugin[] }): { }, ]; - const appExtensionConfigs = readAppExtensionConfigs(appConfig); - for (const appExtensionConfig of appExtensionConfigs) { - const existingConfig = extensionInstanceConfigs.find( - e => e.id === appExtensionConfig.id, - ); - if (existingConfig) { - if (appExtensionConfig.at) { - existingConfig.at = appExtensionConfig.at; - } - if (appExtensionConfig.extension) { - // TODO: do we want to reset config here? it might be completely - // unrelated to the previous one - existingConfig.extension = appExtensionConfig.extension; - } - if (appExtensionConfig.config) { - // TODO: merge config? - existingConfig.config = appExtensionConfig.config; - } - } else if (appExtensionConfig.id) { - const { id, at, extension, config } = appExtensionConfig; - if (!at || !extension) { - throw new Error(`Extension ${appExtensionConfig.id} is incomplete`); - } - extensionInstanceConfigs.push({ id, at, extension, config }); - } - } + // pull in default extension instance from discovered packages + // apply config to adjust default extension instances and add more + const extensionInstanceParams = mergeExtensionParameters( + [ + ...options.plugins.flatMap(plugin => plugin.defaultExtensionInstances), + ...builtinExtensionInstanceParams, + ], + readAppExtensionParameters(appConfig), + ); // Create attachment map so that we can look attachments up during instance creation const attachmentMap = new Map< string, - Map + Map >(); - for (const instanceConfig of extensionInstanceConfigs) { - const [extensionId, pointId = 'default'] = instanceConfig.at.split('/'); + for (const instanceParams of extensionInstanceParams) { + const [extensionId, pointId = 'default'] = instanceParams.at.split('/'); let pointMap = attachmentMap.get(extensionId); if (!pointMap) { @@ -151,21 +76,21 @@ export function createApp(options: { plugins: BackstagePlugin[] }): { pointMap.set(pointId, instances); } - instances.push(instanceConfig); + instances.push(instanceParams); } const instances = new Map(); function createInstance( - instanceConfig: ExtensionInstanceConfig, + instanceParams: ExtensionInstanceParameters, ): ExtensionInstance { - const existingInstance = instances.get(instanceConfig.id); + const existingInstance = instances.get(instanceParams.id); if (existingInstance) { return existingInstance; } const attachments = Object.fromEntries( - Array.from(attachmentMap.get(instanceConfig.id)?.entries() ?? []).map( + Array.from(attachmentMap.get(instanceParams.id)?.entries() ?? []).map( ([inputName, attachmentConfigs]) => [ inputName, attachmentConfigs.map(createInstance), @@ -174,16 +99,16 @@ export function createApp(options: { plugins: BackstagePlugin[] }): { ); return createExtensionInstance({ - id: instanceConfig.id, - config: instanceConfig.config, - extension: instanceConfig.extension, + id: instanceParams.id, + config: instanceParams.config, + extension: instanceParams.extension, attachments, }); } const rootConfigs = attachmentMap.get('root')?.get('default') ?? []; - const rootInstances = rootConfigs.map(instanceConfig => - createInstance(instanceConfig), + const rootInstances = rootConfigs.map(instanceParams => + createInstance(instanceParams), ); return { diff --git a/packages/frontend-app-api/src/wiring/parameters.test.ts b/packages/frontend-app-api/src/wiring/parameters.test.ts new file mode 100644 index 0000000000..2f9ca97999 --- /dev/null +++ b/packages/frontend-app-api/src/wiring/parameters.test.ts @@ -0,0 +1,175 @@ +/* + * 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 { ConfigReader } from '@backstage/config'; +import { Extension } from '@backstage/frontend-plugin-api'; +import { + mergeExtensionParameters, + readAppExtensionParameters, +} from './parameters'; + +function makeExt(id: string, status: 'disabled' | 'enabled' = 'enabled') { + return { + id, + at: 'foo/bar', + extension: {} as Extension, + disabled: status === 'disabled', + }; +} + +describe('mergeExtensionParameters', () => { + it('should filter out disabled extension instances', () => { + expect(mergeExtensionParameters([makeExt('a', 'disabled')], [])).toEqual( + [], + ); + }); + + it('should pass through extension instances', () => { + expect(mergeExtensionParameters([makeExt('a'), makeExt('b')], [])).toEqual([ + makeExt('a'), + makeExt('b'), + ]); + }); + + it('should override extension instances', () => { + expect( + mergeExtensionParameters( + [makeExt('a'), makeExt('b')], + [ + { + id: 'b', + extension: { ext: 'other' } as unknown as Extension, + }, + ], + ), + ).toEqual([makeExt('a'), { ...makeExt('b'), extension: { ext: 'other' } }]); + }); + + it('should override attachment points', () => { + expect( + mergeExtensionParameters( + [makeExt('a'), makeExt('b')], + [ + { + id: 'b', + at: 'derp', + }, + ], + ), + ).toEqual([makeExt('a'), { ...makeExt('b'), at: 'derp' }]); + }); + + it('should fully override configuration', () => { + expect( + mergeExtensionParameters( + [ + { ...makeExt('a'), config: { foo: { bar: 1 } } }, + { ...makeExt('b'), config: { foo: { bar: 2 } } }, + ], + [ + { + id: 'b', + config: { foo: { qux: 3 } }, + }, + ], + ), + ).toEqual([ + { ...makeExt('a'), config: { foo: { bar: 1 } } }, + { ...makeExt('b'), config: { foo: { qux: 3 } } }, + ]); + }); + + it('should place enabled instances in the order that they were enabled', () => { + expect( + mergeExtensionParameters( + [makeExt('a', 'disabled'), makeExt('b', 'disabled')], + [ + { + id: 'b', + disabled: false, + }, + { + id: 'a', + disabled: false, + }, + ], + ), + ).toEqual([makeExt('b'), makeExt('a')]); + }); +}); + +describe('readAppExtensionParameters', () => { + it('should disable extension with shorthand notation', () => { + expect( + readAppExtensionParameters( + new ConfigReader({ app: { extensions: [{ 'core.router': false }] } }), + ), + ).toEqual([ + { + id: 'core.router', + disabled: true, + }, + ]); + expect( + readAppExtensionParameters( + new ConfigReader({ + app: { extensions: [{ 'core.router': { disabled: true } }] }, + }), + ), + ).toEqual([ + { + at: undefined, + config: undefined, + disabled: true, + id: 'core.router', + }, + ]); + }); + + it('should enable extension with shorthand notation', () => { + expect( + readAppExtensionParameters( + new ConfigReader({ app: { extensions: ['core.router'] } }), + ), + ).toEqual([ + { + id: 'core.router', + }, + ]); + expect( + readAppExtensionParameters( + new ConfigReader({ app: { extensions: [{ 'core.router': true }] } }), + ), + ).toEqual([ + { + id: 'core.router', + disabled: false, + }, + ]); + expect( + readAppExtensionParameters( + new ConfigReader({ + app: { extensions: [{ 'core.router': { disabled: false } }] }, + }), + ), + ).toEqual([ + { + id: 'core.router', + disabled: false, + }, + ]); + }); +}); diff --git a/packages/frontend-app-api/src/wiring/parameters.ts b/packages/frontend-app-api/src/wiring/parameters.ts new file mode 100644 index 0000000000..8705eb42dd --- /dev/null +++ b/packages/frontend-app-api/src/wiring/parameters.ts @@ -0,0 +1,131 @@ +/* + * 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 { Config } from '@backstage/config'; +import { ExtensionInstanceParameters } from '@backstage/frontend-plugin-api'; + +// Since we'll never merge arrays in config the config reader context +// isn't too much of a help. Fall back to manual config reading logic +// as the Config interface makes it quite hard for us otherwise. +/** @internal */ +export function readAppExtensionParameters( + rootConfig: Config, +): Partial[] { + const arr = rootConfig.getOptional('app.extensions'); + if (!Array.isArray(arr)) { + if (arr === undefined) { + return []; + } + // This will throw, and show which part of config had the wrong type + rootConfig.getConfigArray('app.extensions'); + return []; + } + + return arr.map((value, index) => { + function errorMsg(msg: string, key?: string, prop?: string) { + return `Invalid extension configuration at app.extensions[${index}]${ + key ? `[${key}]` : '' + }${prop ? `.${prop}` : ''}, ${msg}`; + } + + if (typeof value === 'string') { + return { id: value }; + } else if ( + typeof value !== 'object' || + value === null || + Array.isArray(value) + ) { + throw new Error(errorMsg('must be a string or an object')); + } + + const keys = Object.keys(value); + if (keys.length !== 1) { + const joinedKeys = `"${keys.join('", "')}"`; + throw new Error(errorMsg(`must have exactly one key, got ${joinedKeys}`)); + } + + const key = keys[0]; + const obj = value[key]; + if (typeof obj === 'boolean') { + return { id: key, disabled: !obj }; + } + + if (typeof obj !== 'object' || obj === null || Array.isArray(obj)) { + throw new Error(errorMsg('must be an object', key)); + } + const at = obj.at; + if (at !== undefined && typeof at !== 'string') { + throw new Error(errorMsg('must be a string', key, 'at')); + } + const disabled = obj.disabled; + if (disabled !== undefined && typeof disabled !== 'boolean') { + throw new Error(errorMsg('must be a boolean', key, 'disabled')); + } + const extension = obj.extension; + if (extension !== undefined && typeof extension !== 'string') { + throw new Error(errorMsg('must be a string', key, 'extension')); + } + if (extension) { + throw new Error('TODO: implement extension resolution'); + } + return { id: key, at, disabled, config: obj.config /* validate later */ }; + }); +} + +/** @internal */ +export function mergeExtensionParameters( + base: ExtensionInstanceParameters[], + overrides: Array>, +): ExtensionInstanceParameters[] { + const extensionInstanceParams = base.slice(); + + for (const overrideParam of overrides) { + const existingParamIndex = extensionInstanceParams.findIndex( + e => e.id === overrideParam.id, + ); + if (existingParamIndex !== -1) { + const existingParam = extensionInstanceParams[existingParamIndex]; + if (overrideParam.at) { + existingParam.at = overrideParam.at; + } + if (overrideParam.extension) { + // TODO: do we want to reset config here? it might be completely + // unrelated to the previous one + existingParam.extension = overrideParam.extension; + } + if (overrideParam.config) { + // TODO: merge config? + existingParam.config = overrideParam.config; + } + if (Boolean(existingParam.disabled) !== Boolean(overrideParam.disabled)) { + existingParam.disabled = Boolean(overrideParam.disabled); + if (!existingParam.disabled) { + // bump + extensionInstanceParams.splice(existingParamIndex, 1); + extensionInstanceParams.push(existingParam); + } + } + } else if (overrideParam.id) { + const { id, at, extension, config } = overrideParam; + if (!at || !extension) { + throw new Error(`Extension ${overrideParam.id} is incomplete`); + } + extensionInstanceParams.push({ id, at, extension, config }); + } + } + + return extensionInstanceParams.filter(param => !param.disabled); +} diff --git a/packages/frontend-plugin-api/api-report.md b/packages/frontend-plugin-api/api-report.md index a256a4e6e5..a86597d7bd 100644 --- a/packages/frontend-plugin-api/api-report.md +++ b/packages/frontend-plugin-api/api-report.md @@ -17,7 +17,7 @@ export interface BackstagePlugin { // (undocumented) $$type: 'backstage-plugin'; // (undocumented) - defaultExtensionInstances: ExtensionInstanceConfig[]; + defaultExtensionInstances: ExtensionInstanceParameters[]; // (undocumented) id: string; } @@ -25,7 +25,7 @@ export interface BackstagePlugin { // @public (undocumented) export interface BackstagePluginOptions { // (undocumented) - defaultExtensionInstances?: ExtensionInstanceConfig[]; + defaultExtensionInstances?: ExtensionInstanceParameters[]; // (undocumented) id: string; } @@ -126,12 +126,14 @@ export type ExtensionDataValue = { }; // @public (undocumented) -export interface ExtensionInstanceConfig { +export interface ExtensionInstanceParameters { // (undocumented) at: string; // (undocumented) config?: unknown; // (undocumented) + disabled?: boolean; + // (undocumented) extension: Extension; // (undocumented) id: string; diff --git a/packages/frontend-plugin-api/src/index.ts b/packages/frontend-plugin-api/src/index.ts index 95083bcc73..f70c77abcc 100644 --- a/packages/frontend-plugin-api/src/index.ts +++ b/packages/frontend-plugin-api/src/index.ts @@ -24,7 +24,7 @@ export { createExtension, coreExtensionData, createPlugin, - type ExtensionInstanceParameters as ExtensionInstanceConfig, + type ExtensionInstanceParameters, type BackstagePlugin, type Extension, type AnyExtensionDataMap, diff --git a/packages/frontend-plugin-api/src/types.ts b/packages/frontend-plugin-api/src/types.ts index 3d70a58a0f..8822cf044d 100644 --- a/packages/frontend-plugin-api/src/types.ts +++ b/packages/frontend-plugin-api/src/types.ts @@ -97,6 +97,7 @@ export interface ExtensionInstanceParameters { at: string; extension: Extension; config?: unknown; + disabled?: boolean; } /** @public */