From 074dfe37b04c9c693d432082176642e0f7ed051b Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 12 Jan 2024 17:11:34 +0100 Subject: [PATCH 1/2] frontend-app-api: switch undeclared inputs to be a warning instead of error Signed-off-by: Patrik Oldsberg --- .changeset/silver-countries-notice.md | 5 ++++ .../src/tree/instantiateAppNodeTree.test.ts | 24 +++++++++------ .../src/tree/instantiateAppNodeTree.ts | 30 +++++++++---------- 3 files changed, 35 insertions(+), 24 deletions(-) create mode 100644 .changeset/silver-countries-notice.md diff --git a/.changeset/silver-countries-notice.md b/.changeset/silver-countries-notice.md new file mode 100644 index 0000000000..457042487b --- /dev/null +++ b/.changeset/silver-countries-notice.md @@ -0,0 +1,5 @@ +--- +'@backstage/frontend-app-api': minor +--- + +Attaching extensions to an input that does not exist is now a warning rather than an error. diff --git a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts index 7855b87df2..ec8712cf59 100644 --- a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts +++ b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts @@ -30,6 +30,7 @@ import { AppNodeSpec } from '@backstage/frontend-plugin-api'; import { resolveAppTree } from './resolveAppTree'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { resolveExtensionDefinition } from '../../../frontend-plugin-api/src/wiring/resolveExtensionDefinition'; +import { withLogCollector } from '@backstage/test-utils'; const testDataRef = createExtensionDataRef('test'); const otherDataRef = createExtensionDataRef('other'); @@ -433,8 +434,8 @@ describe('createAppNodeInstance', () => { ); }); - it('should refuse to create an instance with undeclared inputs', () => { - expect(() => + it('should warn when creating an instance with undeclared inputs', () => { + const { warn } = withLogCollector(['warn'], () => createAppNodeInstance({ attachments: new Map([ [ @@ -458,7 +459,7 @@ describe('createAppNodeInstance', () => { resolveExtensionDefinition( createExtension({ namespace: 'app', - name: 'test', + name: 'parent', attachTo: { id: 'ignored', input: 'ignored' }, inputs: { declared: createExtensionInput({ @@ -471,13 +472,15 @@ describe('createAppNodeInstance', () => { ), ), }), - ).toThrow( - "Failed to instantiate extension 'app/test', received undeclared input 'undeclared' from extension 'app/test'", ); + + expect(warn).toEqual([ + "The extension 'app/test' is attached to the input 'undeclared' of 'app/parent', but the extension 'app/parent' noes not declare a 'undeclared' input", + ]); }); it('should refuse to create an instance with multiple undeclared inputs', () => { - expect(() => + const { warn } = withLogCollector(['warn'], () => createAppNodeInstance({ attachments: new Map([ [ @@ -496,7 +499,7 @@ describe('createAppNodeInstance', () => { resolveExtensionDefinition( createExtension({ namespace: 'app', - name: 'test', + name: 'parent', attachTo: { id: 'ignored', input: 'ignored' }, output: {}, factory: () => ({}), @@ -504,9 +507,12 @@ describe('createAppNodeInstance', () => { ), ), }), - ).toThrow( - "Failed to instantiate extension 'app/test', received undeclared inputs 'undeclared1' from extension 'app/test' and 'undeclared2' from extensions 'app/test', 'app/test'", ); + + expect(warn).toEqual([ + "The extension 'app/test' is attached to the input 'undeclared1' of 'app/parent', but the extension 'app/parent' noes not declare a 'undeclared1' input", + "The extensions 'app/test', 'app/test' are attached to the input 'undeclared2' of 'app/parent', but the extension 'app/parent' noes not declare a 'undeclared2' input", + ]); }); it('should refuse to create an instance with multiple inputs for required singleton', () => { diff --git a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts index f72fcdd0cb..b1fa9eae39 100644 --- a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts +++ b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts @@ -46,26 +46,26 @@ function resolveInputData( } function resolveInputs( + id: string, inputMap: AnyExtensionInputMap, attachments: ReadonlyMap, ): ResolvedExtensionInputs { const undeclaredAttachments = Array.from(attachments.entries()).filter( ([inputName]) => inputMap[inputName] === undefined, ); - // TODO: Make this a warning rather than an error - if (undeclaredAttachments.length > 0) { - throw new Error( - `received undeclared input${ - undeclaredAttachments.length > 1 ? 's' : '' - } ${undeclaredAttachments - .map( - ([k, exts]) => - `'${k}' from extension${exts.length > 1 ? 's' : ''} '${exts - .map(e => e.spec.id) - .join("', '")}'`, - ) - .join(' and ')}`, - ); + + if (process.env.NODE_ENV !== 'production') { + for (const [name, nodes] of undeclaredAttachments) { + const pl = nodes.length > 1; + // eslint-disable-next-line no-console + console.warn( + `The extension${pl ? 's' : ''} '${nodes + .map(n => n.spec.id) + .join("', '")}' ${ + pl ? 'are' : 'is' + } attached to the input '${name}' of '${id}', but the extension '${id}' noes not declare a '${name}' input`, + ); + } } return mapValues(inputMap, (input, inputName) => { @@ -129,7 +129,7 @@ export function createAppNodeInstance(options: { const namedOutputs = internalExtension.factory({ node, config: parsedConfig, - inputs: resolveInputs(internalExtension.inputs, attachments), + inputs: resolveInputs(id, internalExtension.inputs, attachments), }); for (const [name, output] of Object.entries(namedOutputs)) { From 4780af8e153de5686c61b3cda58da5c7b73d227f Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 12 Jan 2024 17:49:22 +0100 Subject: [PATCH 2/2] frontend-app-api: list candidate inputs in undeclared input warning Signed-off-by: Patrik Oldsberg --- .../src/tree/instantiateAppNodeTree.test.ts | 6 +++--- .../src/tree/instantiateAppNodeTree.ts | 16 +++++++++++----- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts index ec8712cf59..766cb4804e 100644 --- a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts +++ b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts @@ -475,7 +475,7 @@ describe('createAppNodeInstance', () => { ); expect(warn).toEqual([ - "The extension 'app/test' is attached to the input 'undeclared' of 'app/parent', but the extension 'app/parent' noes not declare a 'undeclared' input", + "The extension 'app/test' is attached to the input 'undeclared' of the extension 'app/parent', but it has no such input (candidates are 'declared')", ]); }); @@ -510,8 +510,8 @@ describe('createAppNodeInstance', () => { ); expect(warn).toEqual([ - "The extension 'app/test' is attached to the input 'undeclared1' of 'app/parent', but the extension 'app/parent' noes not declare a 'undeclared1' input", - "The extensions 'app/test', 'app/test' are attached to the input 'undeclared2' of 'app/parent', but the extension 'app/parent' noes not declare a 'undeclared2' input", + "The extension 'app/test' is attached to the input 'undeclared1' of the extension 'app/parent', but it has no inputs", + "The extensions 'app/test', 'app/test' are attached to the input 'undeclared2' of the extension 'app/parent', but it has no inputs", ]); }); diff --git a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts index b1fa9eae39..90a256fea6 100644 --- a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts +++ b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts @@ -55,15 +55,21 @@ function resolveInputs( ); if (process.env.NODE_ENV !== 'production') { + const inputNames = Object.keys(inputMap); + for (const [name, nodes] of undeclaredAttachments) { const pl = nodes.length > 1; // eslint-disable-next-line no-console console.warn( - `The extension${pl ? 's' : ''} '${nodes - .map(n => n.spec.id) - .join("', '")}' ${ - pl ? 'are' : 'is' - } attached to the input '${name}' of '${id}', but the extension '${id}' noes not declare a '${name}' input`, + [ + `The extension${pl ? 's' : ''} '${nodes + .map(n => n.spec.id) + .join("', '")}' ${pl ? 'are' : 'is'}`, + `attached to the input '${name}' of the extension '${id}', but it`, + inputNames.length === 0 + ? 'has no inputs' + : `has no such input (candidates are '${inputNames.join("', '")}')`, + ].join(' '), ); } }