Merge pull request #19699 from backstage/freben/nulls

allow nulls, relax id checks

packages/frontend-app-api/src/wiring/parameters.test.ts

@@ -210,8 +210,8 @@ describe('readAppExtensionParameters', () => {
           },
         }),
       ),
-    ).toThrow(
-      "Invalid extension configuration at app.extensions[0], extension ID must only contain letters, numbers, and dots; got 'core.router/routes'",
+    ).toThrowErrorMatchingInlineSnapshot(
+      `"Invalid extension configuration at app.extensions[0], extension ID must not contain slashes; got 'core.router/routes', did you mean 'core.router'?"`,
     );
   });
 });
@@ -243,9 +243,6 @@ describe('expandShorthandExtensionParameters', () => {
   });
 
   it('rejects unknown values', () => {
-    expect(() => run({ a: null })).toThrowErrorMatchingInlineSnapshot(
-      `"Invalid extension configuration at app.extensions[1][a], value must be a boolean or object"`,
-    );
     expect(() => run({ a: 1 })).toThrowErrorMatchingInlineSnapshot(
       `"Invalid extension configuration at app.extensions[1][a], value must be a boolean or object"`,
     );
@@ -260,13 +257,26 @@ describe('expandShorthandExtensionParameters', () => {
       disabled: false,
     });
     expect(() => run('')).toThrowErrorMatchingInlineSnapshot(
-      `"Invalid extension configuration at app.extensions[1], extension ID must only contain letters, numbers, and dots; got ''"`,
+      `"Invalid extension configuration at app.extensions[1], extension ID must not be empty or contain whitespace"`,
+    );
+    expect(() => run(' a')).toThrowErrorMatchingInlineSnapshot(
+      `"Invalid extension configuration at app.extensions[1], extension ID must not be empty or contain whitespace"`,
     );
     expect(() => run('core.router/routes')).toThrowErrorMatchingInlineSnapshot(
-      `"Invalid extension configuration at app.extensions[1], extension ID must only contain letters, numbers, and dots; got 'core.router/routes'"`,
+      `"Invalid extension configuration at app.extensions[1], extension ID must not contain slashes; got 'core.router/routes', did you mean 'core.router'?"`,
     );
   });
 
+  it('supports null value', () => {
+    // this is the result of typing:
+    // - core.router:
+    // The missing value is interpreted as null by the yaml parser so we deal with that
+    expect(run({ 'core.router': null })).toEqual({
+      id: 'core.router',
+      disabled: false,
+    });
+  });
+
   it('supports boolean value', () => {
     expect(run({ 'core.router': true })).toEqual({
       id: 'core.router',

packages/frontend-app-api/src/wiring/parameters.ts

@@ -60,14 +60,26 @@ export function expandShorthandExtensionParameters(
     }${prop ? `.${prop}` : ''}, ${msg}`;
   }
 
+  // NOTE(freben): This check is intentionally not complete and doesn't check
+  // whether letters and digits are used, etc. It's not up to the config reading
+  // logic to decide what constitutes a valid extension ID; that should be
+  // decided by the logic that loads and instantiates the extensions. This check
+  // is just here to catch real mistakes or truly conceptually wrong input.
   function assertValidId(id: string) {
-    if (!id.match(/^[\.a-zA-Z0-9]+$/)) {
+    if (!id || id !== id.trim()) {
       throw new Error(
-        errorMsg(
-          `extension ID must only contain letters, numbers, and dots; got '${id}'`,
-        ),
+        errorMsg('extension ID must not be empty or contain whitespace'),
       );
     }
+
+    if (id.includes('/')) {
+      let message = `extension ID must not contain slashes; got '${id}'`;
+      const good = id.split('/')[0];
+      if (good) {
+        message += `, did you mean '${good}'?`;
+      }
+      throw new Error(errorMsg(message));
+    }
   }
 
   // Example YAML:
@@ -98,6 +110,16 @@ export function expandShorthandExtensionParameters(
   const value = arrayEntry[id];
   assertValidId(id);
 
+  // This example covers a potentially common mistake in the syntax
+  // Example YAML:
+  // - entity.card.about:
+  if (value === null) {
+    return {
+      id,
+      disabled: false,
+    };
+  }
+
   // Example YAML:
   // - catalog.page.cicd: false
   if (typeof value === 'boolean') {
@@ -115,7 +137,9 @@ export function expandShorthandExtensionParameters(
   //        path: /tech-radar
   //        width: 1500
   //        height: 800
-  if (typeof value !== 'object' || value === null || Array.isArray(value)) {
+  if (typeof value !== 'object' || Array.isArray(value)) {
+    // We don't mention null here - we don't want people to explicitly enter
+    // - entity.card.about: null
     throw new Error(errorMsg('value must be a boolean or object', id));
   }