From 2a6142234f131adbc90258b0aa041e8e5455c9f2 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 30 Aug 2024 00:42:37 +0200 Subject: [PATCH 1/2] frontend-plugin-api: no longer require factory for extension overrides Signed-off-by: Patrik Oldsberg --- .changeset/breezy-guests-scream.md | 5 +++ .changeset/three-socks-call.md | 5 +++ packages/frontend-plugin-api/api-report.md | 2 +- .../src/wiring/createExtension.test.ts | 35 +++++++++++++++++++ .../src/wiring/createExtension.ts | 2 +- .../src/app/createExtensionTester.tsx | 16 ++++++--- 6 files changed, 58 insertions(+), 7 deletions(-) create mode 100644 .changeset/breezy-guests-scream.md create mode 100644 .changeset/three-socks-call.md diff --git a/.changeset/breezy-guests-scream.md b/.changeset/breezy-guests-scream.md new file mode 100644 index 0000000000..e33344f992 --- /dev/null +++ b/.changeset/breezy-guests-scream.md @@ -0,0 +1,5 @@ +--- +'@backstage/frontend-test-utils': patch +--- + +The extension tester will no longer unconditionally enable any additional extensions that have been added. diff --git a/.changeset/three-socks-call.md b/.changeset/three-socks-call.md new file mode 100644 index 0000000000..39b75f2fea --- /dev/null +++ b/.changeset/three-socks-call.md @@ -0,0 +1,5 @@ +--- +'@backstage/frontend-plugin-api': patch +--- + +The `factory` option is no longer required when overriding an extension. diff --git a/packages/frontend-plugin-api/api-report.md b/packages/frontend-plugin-api/api-report.md index b42efddffa..e694f1d8c8 100644 --- a/packages/frontend-plugin-api/api-report.md +++ b/packages/frontend-plugin-api/api-report.md @@ -1108,7 +1108,7 @@ export type ExtensionDefinition< string}' is already defined in parent schema`; }; }; - factory( + factory?( originalFactory: (context?: { config?: T['config']; inputs?: ResolveInputValueOverrides>; diff --git a/packages/frontend-plugin-api/src/wiring/createExtension.test.ts b/packages/frontend-plugin-api/src/wiring/createExtension.test.ts index 68478a756e..ee79c277d0 100644 --- a/packages/frontend-plugin-api/src/wiring/createExtension.test.ts +++ b/packages/frontend-plugin-api/src/wiring/createExtension.test.ts @@ -705,6 +705,41 @@ describe('createExtension', () => { ).toBe('foo-hello-override-world'); }); + it('should be able to disable extension with override', () => { + const subject = createExtension({ + name: 'root', + attachTo: { id: 'ignored', input: 'ignored' }, + inputs: { + input: createExtensionInput([stringDataRef], { + singleton: true, + optional: true, + }), + }, + output: [stringDataRef.optional()], + factory({ inputs }) { + return inputs.input ?? []; + }, + }); + + const attached = createExtension({ + attachTo: { id: 'root', input: 'input' }, + output: [stringDataRef], + factory() { + return [stringDataRef('test')]; + }, + }); + + expect( + createExtensionTester(subject).add(attached).get(stringDataRef), + ).toBe('test'); + + expect( + createExtensionTester(subject) + .add(attached.override({ disabled: true })) + .get(stringDataRef), + ).toBe(undefined); + }); + it('should be able to override input values', () => { const outputRef = createExtensionDataRef().with({ id: 'output', diff --git a/packages/frontend-plugin-api/src/wiring/createExtension.ts b/packages/frontend-plugin-api/src/wiring/createExtension.ts index 445bbd8a68..1dcc2fef6f 100644 --- a/packages/frontend-plugin-api/src/wiring/createExtension.ts +++ b/packages/frontend-plugin-api/src/wiring/createExtension.ts @@ -191,7 +191,7 @@ export type ExtensionDefinition< string}' is already defined in parent schema`; }; }; - factory( + factory?( originalFactory: (context?: { config?: T['config']; inputs?: ResolveInputValueOverrides>; diff --git a/packages/frontend-test-utils/src/app/createExtensionTester.tsx b/packages/frontend-test-utils/src/app/createExtensionTester.tsx index f9c1b3cb89..232b41b435 100644 --- a/packages/frontend-test-utils/src/app/createExtensionTester.tsx +++ b/packages/frontend-test-utils/src/app/createExtensionTester.tsx @@ -213,11 +213,17 @@ export class ExtensionTester { const [subject, ...rest] = this.#extensions; const extensionsConfig: JsonArray = [ - ...rest.map(extension => ({ - [extension.id]: { - config: extension.config, - }, - })), + ...rest.flatMap(extension => + extension.config + ? [ + { + [extension.id]: { + config: extension.config, + }, + }, + ] + : [], + ), { [subject.id]: { config: subject.config, From 7d028e271d359f09d1b27b9122845cf1da47a4f9 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 10 Sep 2024 00:45:52 +0200 Subject: [PATCH 2/2] frontend-plugin-api: refuse output override without factory Signed-off-by: Patrik Oldsberg --- .../src/wiring/createExtension.test.ts | 30 ++++++++++++++++++- .../src/wiring/createExtension.ts | 9 ++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/packages/frontend-plugin-api/src/wiring/createExtension.test.ts b/packages/frontend-plugin-api/src/wiring/createExtension.test.ts index ee79c277d0..271415fda4 100644 --- a/packages/frontend-plugin-api/src/wiring/createExtension.test.ts +++ b/packages/frontend-plugin-api/src/wiring/createExtension.test.ts @@ -588,7 +588,7 @@ describe('createExtension', () => { }, }); - // @ts-expect-error - this should fail because string output should be merged? + // @ts-expect-error const override2 = testExtension.override({ output: [numberDataRef], factory(_, { inputs }) { @@ -740,6 +740,34 @@ describe('createExtension', () => { ).toBe(undefined); }); + it('should complain when overriding with incompatible output', () => { + const testExtension = createExtension({ + namespace: 'test', + attachTo: { id: 'root', input: 'blob' }, + output: [stringDataRef], + factory() { + return [stringDataRef('0')]; + }, + }); + + // @ts-expect-error - override output is incompatible with factory + const override = testExtension.override({ + output: [numberDataRef], + factory() { + return [stringDataRef('1')]; + }, + }); + expect(override).toBeDefined(); + + expect(() => + testExtension.override({ + output: [numberDataRef], + }), + ).toThrowErrorMatchingInlineSnapshot( + `"Refused to override output without also overriding factory"`, + ); + }); + it('should be able to override input values', () => { const outputRef = createExtensionDataRef().with({ id: 'output', diff --git a/packages/frontend-plugin-api/src/wiring/createExtension.ts b/packages/frontend-plugin-api/src/wiring/createExtension.ts index 1dcc2fef6f..d6f339ef6d 100644 --- a/packages/frontend-plugin-api/src/wiring/createExtension.ts +++ b/packages/frontend-plugin-api/src/wiring/createExtension.ts @@ -430,6 +430,15 @@ export function createExtension< UFactoryOutput >; + // TODO(Rugvip): Making this a type check would be optimal, but it seems + // like it's tricky to add that and still have the type + // inference work correctly for the factory output. + if (overrideOptions.output && !overrideOptions.factory) { + throw new Error( + 'Refused to override output without also overriding factory', + ); + } + return createExtension({ kind: newOptions.kind, namespace: newOptions.namespace,