From 5cdf2b3d94d0a30b3d5c39045ce289c12c1710bc Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 13 Dec 2023 16:28:40 +0100 Subject: [PATCH] frontend-plugin-api: switch Extension and ExtensionDefinition to be opaque types Signed-off-by: Patrik Oldsberg --- .changeset/sweet-days-fail.md | 5 ++ .changeset/sweet-waves-do.md | 5 ++ .../src/tree/instantiateAppNodeTree.ts | 10 +++- .../src/tree/resolveAppNodeSpecs.test.ts | 2 + .../src/tree/resolveAppNodeSpecs.ts | 58 ++++++++++-------- packages/frontend-plugin-api/api-report.md | 40 ++++--------- .../src/extensions/createApiExtension.test.ts | 2 + .../createNavLogoExtension.test.tsx | 1 + .../extensions/createPageExtension.test.tsx | 3 + .../createTranslationExtension.test.ts | 7 ++- .../src/wiring/createExtension.ts | 55 ++++++++++------- .../wiring/createExtensionOverrides.test.ts | 3 + .../src/wiring/createExtensionOverrides.ts | 7 ++- .../src/wiring/createPlugin.ts | 7 ++- .../frontend-plugin-api/src/wiring/index.ts | 2 +- .../wiring/resolveExtensionDefinition.test.ts | 17 ++++-- .../src/wiring/resolveExtensionDefinition.ts | 60 +++++++++++++++++-- 17 files changed, 188 insertions(+), 96 deletions(-) create mode 100644 .changeset/sweet-days-fail.md create mode 100644 .changeset/sweet-waves-do.md diff --git a/.changeset/sweet-days-fail.md b/.changeset/sweet-days-fail.md new file mode 100644 index 0000000000..b3b2e60b76 --- /dev/null +++ b/.changeset/sweet-days-fail.md @@ -0,0 +1,5 @@ +--- +'@backstage/frontend-plugin-api': minor +--- + +Changed `Extension` and `ExtensionDefinition` to use opaque types. diff --git a/.changeset/sweet-waves-do.md b/.changeset/sweet-waves-do.md new file mode 100644 index 0000000000..42d07eb603 --- /dev/null +++ b/.changeset/sweet-waves-do.md @@ -0,0 +1,5 @@ +--- +'@backstage/frontend-app-api': patch +--- + +Updated usage of `Extension` and `ExtensionDefinition` as they are now opaque. diff --git a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts index dee1c83e3f..f72fcdd0cb 100644 --- a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts +++ b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts @@ -22,6 +22,8 @@ import { } from '@backstage/frontend-plugin-api'; import mapValues from 'lodash/mapValues'; import { AppNode, AppNodeInstance } from '@backstage/frontend-plugin-api'; +// eslint-disable-next-line @backstage/no-relative-monorepo-imports +import { toInternalExtension } from '../../../frontend-plugin-api/src/wiring/resolveExtensionDefinition'; type Mutable = { -readonly [P in keyof T]: T[P]; @@ -122,14 +124,16 @@ export function createAppNodeInstance(options: { } try { - const namedOutputs = extension.factory({ + const internalExtension = toInternalExtension(extension); + + const namedOutputs = internalExtension.factory({ node, config: parsedConfig, - inputs: resolveInputs(extension.inputs, attachments), + inputs: resolveInputs(internalExtension.inputs, attachments), }); for (const [name, output] of Object.entries(namedOutputs)) { - const ref = extension.output[name]; + const ref = internalExtension.output[name]; if (!ref) { throw new Error(`unknown output provided via '${name}'`); } diff --git a/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.test.ts b/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.test.ts index 2e9f73d87b..8dfa7aeea9 100644 --- a/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.test.ts +++ b/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.test.ts @@ -29,6 +29,7 @@ function makeExt( ) { return { $$type: '@backstage/Extension', + version: 'v1', id, attachTo: { id: attachId, input: 'default' }, disabled: status === 'disabled', @@ -42,6 +43,7 @@ function makeExtDef( ) { return { $$type: '@backstage/ExtensionDefinition', + version: 'v1', name, attachTo: { id: attachId, input: 'default' }, disabled: status === 'disabled', diff --git a/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.ts b/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.ts index 34ec29b8f7..870e1f69f8 100644 --- a/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.ts +++ b/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.ts @@ -25,6 +25,8 @@ import { ExtensionParameters } from './readAppExtensionsConfig'; import { AppNodeSpec } from '@backstage/frontend-plugin-api'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { toInternalBackstagePlugin } from '../../../frontend-plugin-api/src/wiring/createPlugin'; +// eslint-disable-next-line @backstage/no-relative-monorepo-imports +import { toInternalExtension } from '../../../frontend-plugin-api/src/wiring/resolveExtensionDefinition'; /** @internal */ export function resolveAppNodeSpecs(options: { @@ -88,45 +90,53 @@ export function resolveAppNodeSpecs(options: { } const configuredExtensions = [ - ...pluginExtensions.map(({ source, ...extension }) => ({ - extension, - params: { - source, - attachTo: extension.attachTo, - disabled: extension.disabled, - config: undefined as unknown, - }, - })), - ...builtinExtensions.map(extension => ({ - extension, - params: { - source: undefined, - attachTo: extension.attachTo, - disabled: extension.disabled, - config: undefined as unknown, - }, - })), + ...pluginExtensions.map(({ source, ...extension }) => { + const internalExtension = toInternalExtension(extension); + return { + extension: internalExtension, + params: { + source, + attachTo: internalExtension.attachTo, + disabled: internalExtension.disabled, + config: undefined as unknown, + }, + }; + }), + ...builtinExtensions.map(extension => { + const internalExtension = toInternalExtension(extension); + return { + extension: internalExtension, + params: { + source: undefined, + attachTo: internalExtension.attachTo, + disabled: internalExtension.disabled, + config: undefined as unknown, + }, + }; + }), ]; // Install all extension overrides for (const extension of overrideExtensions) { + 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 = extension; - configuredExtensions[index].params.attachTo = extension.attachTo; - configuredExtensions[index].params.disabled = extension.disabled; + 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, + extension: internalExtension, params: { source: undefined, - attachTo: extension.attachTo, - disabled: extension.disabled, + attachTo: internalExtension.attachTo, + disabled: internalExtension.disabled, config: undefined, }, }); diff --git a/packages/frontend-plugin-api/api-report.md b/packages/frontend-plugin-api/api-report.md index 6bd3000867..2e6cc1d8cb 100644 --- a/packages/frontend-plugin-api/api-report.md +++ b/packages/frontend-plugin-api/api-report.md @@ -670,26 +670,16 @@ export interface Extension { // (undocumented) $$type: '@backstage/Extension'; // (undocumented) - attachTo: { + readonly attachTo: { id: string; input: string; }; // (undocumented) - configSchema?: PortableSchema; + readonly configSchema?: PortableSchema; // (undocumented) - disabled: boolean; + readonly disabled: boolean; // (undocumented) - factory(options: { - node: AppNode; - config: TConfig; - inputs: ResolvedExtensionInputs; - }): ExtensionDataValues; - // (undocumented) - id: string; - // (undocumented) - inputs: AnyExtensionInputMap; - // (undocumented) - output: AnyExtensionDataMap; + readonly id: string; } // @public (undocumented) @@ -740,30 +730,20 @@ export interface ExtensionDefinition { // (undocumented) $$type: '@backstage/ExtensionDefinition'; // (undocumented) - attachTo: { + readonly attachTo: { id: string; input: string; }; // (undocumented) - configSchema?: PortableSchema; + readonly configSchema?: PortableSchema; // (undocumented) - disabled: boolean; + readonly disabled: boolean; // (undocumented) - factory(options: { - node: AppNode; - config: TConfig; - inputs: ResolvedExtensionInputs; - }): ExtensionDataValues; + readonly kind?: string; // (undocumented) - inputs: AnyExtensionInputMap; + readonly name?: string; // (undocumented) - kind?: string; - // (undocumented) - name?: string; - // (undocumented) - namespace?: string; - // (undocumented) - output: AnyExtensionDataMap; + readonly namespace?: string; } // @public (undocumented) diff --git a/packages/frontend-plugin-api/src/extensions/createApiExtension.test.ts b/packages/frontend-plugin-api/src/extensions/createApiExtension.test.ts index 2585b0d694..d3bd84bc7f 100644 --- a/packages/frontend-plugin-api/src/extensions/createApiExtension.test.ts +++ b/packages/frontend-plugin-api/src/extensions/createApiExtension.test.ts @@ -32,6 +32,7 @@ describe('createApiExtension', () => { }), ).toEqual({ $$type: '@backstage/ExtensionDefinition', + version: 'v1', kind: 'api', namespace: 'test', attachTo: { id: 'core', input: 'apis' }, @@ -67,6 +68,7 @@ describe('createApiExtension', () => { // boo expect(extension).toEqual({ $$type: '@backstage/ExtensionDefinition', + version: 'v1', kind: 'api', namespace: 'test', attachTo: { id: 'core', input: 'apis' }, diff --git a/packages/frontend-plugin-api/src/extensions/createNavLogoExtension.test.tsx b/packages/frontend-plugin-api/src/extensions/createNavLogoExtension.test.tsx index ca95fad7bc..265b381094 100644 --- a/packages/frontend-plugin-api/src/extensions/createNavLogoExtension.test.tsx +++ b/packages/frontend-plugin-api/src/extensions/createNavLogoExtension.test.tsx @@ -31,6 +31,7 @@ describe('createNavLogoExtension', () => { }), ).toEqual({ $$type: '@backstage/ExtensionDefinition', + version: 'v1', kind: 'nav-logo', name: 'test', attachTo: { id: 'core/nav', input: 'logos' }, diff --git a/packages/frontend-plugin-api/src/extensions/createPageExtension.test.tsx b/packages/frontend-plugin-api/src/extensions/createPageExtension.test.tsx index 525f10ee61..28f74d544c 100644 --- a/packages/frontend-plugin-api/src/extensions/createPageExtension.test.tsx +++ b/packages/frontend-plugin-api/src/extensions/createPageExtension.test.tsx @@ -42,6 +42,7 @@ describe('createPageExtension', () => { }), ).toEqual({ $$type: '@backstage/ExtensionDefinition', + version: 'v1', name: 'test', kind: 'page', attachTo: { id: 'core/routes', input: 'routes' }, @@ -71,6 +72,7 @@ describe('createPageExtension', () => { }), ).toEqual({ $$type: '@backstage/ExtensionDefinition', + version: 'v1', name: 'test', kind: 'page', attachTo: { id: 'other', input: 'place' }, @@ -97,6 +99,7 @@ describe('createPageExtension', () => { }), ).toEqual({ $$type: '@backstage/ExtensionDefinition', + version: 'v1', name: 'test', kind: 'page', attachTo: { id: 'core/routes', input: 'routes' }, diff --git a/packages/frontend-plugin-api/src/extensions/createTranslationExtension.test.ts b/packages/frontend-plugin-api/src/extensions/createTranslationExtension.test.ts index 3a0eee91b5..ea57fdbc8f 100644 --- a/packages/frontend-plugin-api/src/extensions/createTranslationExtension.test.ts +++ b/packages/frontend-plugin-api/src/extensions/createTranslationExtension.test.ts @@ -41,6 +41,7 @@ describe('createTranslationExtension', () => { expect(extension).toEqual({ $$type: '@backstage/ExtensionDefinition', + version: 'v1', kind: 'translation', namespace: 'test', attachTo: { id: 'core', input: 'translations' }, @@ -52,7 +53,7 @@ describe('createTranslationExtension', () => { factory: expect.any(Function), }); - expect(extension.factory({} as any)).toEqual({ + expect((extension as any).factory({} as any)).toEqual({ resource: messages, }); }); @@ -77,6 +78,7 @@ describe('createTranslationExtension', () => { expect(extension).toEqual({ $$type: '@backstage/ExtensionDefinition', + version: 'v1', kind: 'translation', namespace: 'test', attachTo: { id: 'core', input: 'translations' }, @@ -88,7 +90,7 @@ describe('createTranslationExtension', () => { factory: expect.any(Function), }); - expect(extension.factory({} as any)).toEqual({ resource }); + expect((extension as any).factory({} as any)).toEqual({ resource }); }); it('creates a translation resource extension with a name', () => { @@ -113,6 +115,7 @@ describe('createTranslationExtension', () => { }), ).toEqual({ $$type: '@backstage/ExtensionDefinition', + version: 'v1', kind: 'translation', namespace: 'test', name: 'sv', diff --git a/packages/frontend-plugin-api/src/wiring/createExtension.ts b/packages/frontend-plugin-api/src/wiring/createExtension.ts index ccb93f44ea..0d6806fe49 100644 --- a/packages/frontend-plugin-api/src/wiring/createExtension.ts +++ b/packages/frontend-plugin-api/src/wiring/createExtension.ts @@ -101,14 +101,20 @@ export interface CreateExtensionOptions< /** @public */ export interface ExtensionDefinition { $$type: '@backstage/ExtensionDefinition'; - kind?: string; - namespace?: string; - name?: string; - attachTo: { id: string; input: string }; - disabled: boolean; - inputs: AnyExtensionInputMap; - output: AnyExtensionDataMap; - configSchema?: PortableSchema; + readonly kind?: string; + readonly namespace?: string; + readonly name?: string; + readonly attachTo: { id: string; input: string }; + readonly disabled: boolean; + readonly configSchema?: PortableSchema; +} + +/** @internal */ +export interface InternalExtensionDefinition + extends ExtensionDefinition { + readonly version: 'v1'; + readonly inputs: AnyExtensionInputMap; + readonly output: AnyExtensionDataMap; factory(options: { node: AppNode; config: TConfig; @@ -116,20 +122,22 @@ export interface ExtensionDefinition { }): ExtensionDataValues; } -/** @public */ -export interface Extension { - $$type: '@backstage/Extension'; - id: string; - attachTo: { id: string; input: string }; - disabled: boolean; - inputs: AnyExtensionInputMap; - output: AnyExtensionDataMap; - configSchema?: PortableSchema; - factory(options: { - node: AppNode; - config: TConfig; - inputs: ResolvedExtensionInputs; - }): ExtensionDataValues; +/** @internal */ +export function toInternalExtensionDefinition( + overrides: ExtensionDefinition, +): InternalExtensionDefinition { + const internal = overrides as InternalExtensionDefinition; + if (internal.$$type !== '@backstage/ExtensionDefinition') { + throw new Error( + `Invalid extension definition instance, bad type '${internal.$$type}'`, + ); + } + if (internal.version !== 'v1') { + throw new Error( + `Invalid extension definition instance, bad version '${internal.version}'`, + ); + } + return internal; } /** @public */ @@ -142,6 +150,7 @@ export function createExtension< ): ExtensionDefinition { return { $$type: '@backstage/ExtensionDefinition', + version: 'v1', kind: options.kind, namespace: options.namespace, name: options.name, @@ -157,5 +166,5 @@ export function createExtension< ...rest, }); }, - }; + } as InternalExtensionDefinition; } diff --git a/packages/frontend-plugin-api/src/wiring/createExtensionOverrides.test.ts b/packages/frontend-plugin-api/src/wiring/createExtensionOverrides.test.ts index 772a214490..6f5f19e29e 100644 --- a/packages/frontend-plugin-api/src/wiring/createExtensionOverrides.test.ts +++ b/packages/frontend-plugin-api/src/wiring/createExtensionOverrides.test.ts @@ -74,6 +74,7 @@ describe('createExtensionOverrides', () => { "id": "a", "inputs": {}, "output": {}, + "version": "v1", }, { "$$type": "@backstage/Extension", @@ -87,6 +88,7 @@ describe('createExtensionOverrides', () => { "id": "b", "inputs": {}, "output": {}, + "version": "v1", }, { "$$type": "@backstage/Extension", @@ -100,6 +102,7 @@ describe('createExtensionOverrides', () => { "id": "k:c/n", "inputs": {}, "output": {}, + "version": "v1", }, ], "featureFlags": [], diff --git a/packages/frontend-plugin-api/src/wiring/createExtensionOverrides.ts b/packages/frontend-plugin-api/src/wiring/createExtensionOverrides.ts index 6c368a0e8b..a60e655ff9 100644 --- a/packages/frontend-plugin-api/src/wiring/createExtensionOverrides.ts +++ b/packages/frontend-plugin-api/src/wiring/createExtensionOverrides.ts @@ -14,8 +14,11 @@ * limitations under the License. */ -import { Extension, ExtensionDefinition } from './createExtension'; -import { resolveExtensionDefinition } from './resolveExtensionDefinition'; +import { ExtensionDefinition } from './createExtension'; +import { + Extension, + resolveExtensionDefinition, +} from './resolveExtensionDefinition'; import { FeatureFlagConfig } from './types'; /** @public */ diff --git a/packages/frontend-plugin-api/src/wiring/createPlugin.ts b/packages/frontend-plugin-api/src/wiring/createPlugin.ts index ecc4d08f9a..06c216a37f 100644 --- a/packages/frontend-plugin-api/src/wiring/createPlugin.ts +++ b/packages/frontend-plugin-api/src/wiring/createPlugin.ts @@ -14,10 +14,13 @@ * limitations under the License. */ -import { Extension, ExtensionDefinition } from './createExtension'; +import { ExtensionDefinition } from './createExtension'; import { ExternalRouteRef, RouteRef } from '../routing'; import { FeatureFlagConfig } from './types'; -import { resolveExtensionDefinition } from './resolveExtensionDefinition'; +import { + Extension, + resolveExtensionDefinition, +} from './resolveExtensionDefinition'; /** @public */ export type AnyRoutes = { [name in string]: RouteRef }; diff --git a/packages/frontend-plugin-api/src/wiring/index.ts b/packages/frontend-plugin-api/src/wiring/index.ts index 3f33910ca7..b19669c518 100644 --- a/packages/frontend-plugin-api/src/wiring/index.ts +++ b/packages/frontend-plugin-api/src/wiring/index.ts @@ -21,7 +21,6 @@ export { } from './coreExtensionData'; export { createExtension, - type Extension, type ExtensionDefinition, type CreateExtensionOptions, type ExtensionDataValues, @@ -51,4 +50,5 @@ export { type ExtensionOverrides, type ExtensionOverridesOptions, } from './createExtensionOverrides'; +export { type Extension } from './resolveExtensionDefinition'; export type { FeatureFlagConfig } from './types'; diff --git a/packages/frontend-plugin-api/src/wiring/resolveExtensionDefinition.test.ts b/packages/frontend-plugin-api/src/wiring/resolveExtensionDefinition.test.ts index 8f3755ac5d..9db9539ebb 100644 --- a/packages/frontend-plugin-api/src/wiring/resolveExtensionDefinition.test.ts +++ b/packages/frontend-plugin-api/src/wiring/resolveExtensionDefinition.test.ts @@ -17,6 +17,13 @@ import { ExtensionDefinition } from './createExtension'; import { resolveExtensionDefinition } from './resolveExtensionDefinition'; +const baseDef = { + $$type: '@backstage/ExtensionDefinition', + version: 'v1', + attachTo: { id: '', input: '' }, + disabled: false, +}; + describe('resolveExtensionDefinition', () => { it.each([ [{ namespace: 'ns' }, 'ns'], @@ -25,22 +32,24 @@ describe('resolveExtensionDefinition', () => { [{ kind: 'k', namespace: 'ns' }, 'k:ns'], [{ kind: 'k', namespace: 'ns', name: 'n' }, 'k:ns/n'], ])(`should resolve extension IDs %s`, (definition, expected) => { - const resolved = resolveExtensionDefinition( - definition as ExtensionDefinition, - ); + const resolved = resolveExtensionDefinition({ + ...baseDef, + ...definition, + } as ExtensionDefinition); expect(resolved.id).toBe(expected); }); it('should fail to resolve extension ID without namespace', () => { expect(() => resolveExtensionDefinition({ + ...baseDef, kind: 'k', } as ExtensionDefinition), ).toThrow( 'Extension must declare an explicit namespace or name as it could not be resolved from context, kind=k namespace=undefined name=undefined', ); expect(() => - resolveExtensionDefinition({} as ExtensionDefinition), + resolveExtensionDefinition(baseDef as ExtensionDefinition), ).toThrow( 'Extension must declare an explicit namespace or name as it could not be resolved from context, kind=undefined namespace=undefined name=undefined', ); diff --git a/packages/frontend-plugin-api/src/wiring/resolveExtensionDefinition.ts b/packages/frontend-plugin-api/src/wiring/resolveExtensionDefinition.ts index 42f3a75246..d702120447 100644 --- a/packages/frontend-plugin-api/src/wiring/resolveExtensionDefinition.ts +++ b/packages/frontend-plugin-api/src/wiring/resolveExtensionDefinition.ts @@ -14,15 +14,64 @@ * limitations under the License. */ -import { Extension, ExtensionDefinition } from './createExtension'; +import { AppNode } from '../apis'; +import { + AnyExtensionDataMap, + AnyExtensionInputMap, + ExtensionDataValues, + ExtensionDefinition, + ResolvedExtensionInputs, + toInternalExtensionDefinition, +} from './createExtension'; +import { PortableSchema } from '../schema'; + +/** @public */ +export interface Extension { + $$type: '@backstage/Extension'; + readonly id: string; + readonly attachTo: { id: string; input: string }; + readonly disabled: boolean; + readonly configSchema?: PortableSchema; +} + +/** @internal */ +export interface InternalExtension extends Extension { + readonly version: 'v1'; + readonly inputs: AnyExtensionInputMap; + readonly output: AnyExtensionDataMap; + factory(options: { + node: AppNode; + config: TConfig; + inputs: ResolvedExtensionInputs; + }): ExtensionDataValues; +} + +/** @internal */ +export function toInternalExtension( + overrides: Extension, +): InternalExtension { + const internal = overrides as InternalExtension; + if (internal.$$type !== '@backstage/Extension') { + throw new Error( + `Invalid extension instance, bad type '${internal.$$type}'`, + ); + } + if (internal.version !== 'v1') { + throw new Error( + `Invalid extension instance, bad version '${internal.version}'`, + ); + } + return internal; +} /** @internal */ export function resolveExtensionDefinition( definition: ExtensionDefinition, context?: { namespace?: string }, ): Extension { - const { name, kind, namespace: _, ...rest } = definition; - const namespace = definition.namespace ?? context?.namespace; + const internalDefinition = toInternalExtensionDefinition(definition); + const { name, kind, namespace: _, ...rest } = internalDefinition; + const namespace = internalDefinition.namespace ?? context?.namespace; const namePart = name && namespace ? `${namespace}/${name}` : namespace || name; @@ -34,7 +83,8 @@ export function resolveExtensionDefinition( return { ...rest, - id: kind ? `${kind}:${namePart}` : namePart, $$type: '@backstage/Extension', - }; + version: 'v1', + id: kind ? `${kind}:${namePart}` : namePart, + } as InternalExtension; }