config: treat null as explicitly undefined

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>

Author: Rugvip

.changeset/clever-donkeys-fry.md

@@ -0,0 +1,5 @@
+---
+'@backstage/config': minor
+---
+
+The `ConfigReader` now treats `null` values as present but explicitly undefined, meaning it will not fall back to the next level of configuration.

packages/config/package.json

@@ -37,8 +37,7 @@
   },
   "dependencies": {
     "@backstage/errors": "workspace:^",
-    "@backstage/types": "workspace:^",
-    "lodash": "^4.17.21"
+    "@backstage/types": "workspace:^"
   },
   "devDependencies": {
     "@backstage/cli": "workspace:^",

packages/config/src/reader.test.ts

@@ -48,15 +48,15 @@ const DATA = {
 };
 
 function expectValidValues(config: ConfigReader) {
-  expect(config.keys()).toEqual(Object.keys(DATA));
+  expect(config.keys()).toEqual(Object.keys(DATA).filter(k => k !== 'null'));
   expect(config.get('zero')).toBe(0);
   expect(config.has('zero')).toBe(true);
   expect(config.has('false')).toBe(true);
-  expect(config.has('null')).toBe(true);
+  expect(config.has('null')).toBe(false);
   expect(config.has('missing')).toBe(false);
   expect(config.has('nested.one')).toBe(true);
   expect(config.has('nested.missing')).toBe(false);
-  expect(config.has('nested.null')).toBe(true);
+  expect(config.has('nested.null')).toBe(false);
   expect(config.getNumber('zero')).toBe(0);
   expect(config.getNumber('one')).toBe(1);
   expect(config.getNumber('zeroString')).toBe(0);
@@ -81,7 +81,7 @@ function expectValidValues(config: ConfigReader) {
   expect(config.getConfig('nested').getNumber('one')).toBe(1);
   expect(config.get('nested')).toEqual({
     one: 1,
-    null: null,
+    null: undefined,
     string: 'string',
     strings: ['string1', 'string2'],
   });
@@ -90,6 +90,8 @@ function expectValidValues(config: ConfigReader) {
     ['string1', 'string2'],
   );
   expect(config.getOptional('missing')).toBe(undefined);
+  expect(config.getOptionalConfig('null')).toBe(undefined);
+  expect(config.getOptionalConfig('null.nested')).toBe(undefined);
   expect(config.getOptionalConfig('missing')).toBe(undefined);
   expect(config.getOptionalConfigArray('missing')).toBe(undefined);
   expect(config.getNumber('zero')).toBe(0);
@@ -120,7 +122,7 @@ function expectInvalidValues(config: ConfigReader) {
     "Invalid type in config for key 'true' in 'ctx', got boolean, wanted number",
   );
   expect(() => config.getStringArray('null')).toThrow(
-    "Invalid type in config for key 'null' in 'ctx', got null, wanted string-array",
+    "Missing required config value at 'null' in 'ctx'",
   );
   expect(() => config.getString('emptyString')).toThrow(
     "Invalid type in config for key 'emptyString' in 'ctx', got empty-string, wanted string",
@@ -137,6 +139,9 @@ function expectInvalidValues(config: ConfigReader) {
   expect(() => config.getConfig('one')).toThrow(
     "Invalid type in config for key 'one' in 'ctx', got number, wanted object",
   );
+  expect(() => config.getConfig('null')).toThrow(
+    "Missing required config value at 'null'",
+  );
   expect(() => config.getConfigArray('one')).toThrow(
     "Invalid type in config for key 'one' in 'ctx', got number, wanted object-array",
   );
@@ -742,4 +747,42 @@ describe('ConfigReader.get()', () => {
     expect(data1.foo.baz).toEqual({});
     expect(data2.x.y.z).toEqual({});
   });
+
+  it('should treat null as explicitly undefined', () => {
+    const reader = ConfigReader.fromConfigs([
+      {
+        data: { obj: { a: 2, b: 2, c: 2 }, objB: { a: 2, b: null } },
+        context: 'fallback',
+      },
+      {
+        data: { obj: { a: 1, b: null }, objA: { a: 1, b: null } },
+        context: 'primary',
+      },
+    ]);
+
+    expect(reader.getOptionalNumber('obj.a')).toBe(1);
+    expect(reader.getOptionalNumber('obj.b')).toBe(undefined);
+    expect(reader.getOptionalNumber('obj.c')).toBe(2);
+
+    expect(reader.getConfig('obj').getOptionalNumber('a')).toBe(1);
+    expect(reader.getConfig('obj').getOptionalNumber('b')).toBe(undefined);
+    expect(reader.getConfig('obj').getOptionalNumber('c')).toBe(2);
+
+    expect(reader.getConfig('obj').get('a')).toBe(1);
+    expect(() => reader.getConfig('obj').get('b')).toThrow(
+      "Missing required config value at 'obj.b' in 'primary'",
+    );
+    expect(reader.getConfig('obj').get('c')).toBe(2);
+
+    expect(reader.getConfig('obj').getOptional('a')).toBe(1);
+    expect(reader.getConfig('obj').getOptional('b')).toBe(undefined);
+    expect(reader.getConfig('obj').getOptional('c')).toBe(2);
+
+    expect(reader.get('obj')).toEqual({ a: 1, c: 2 });
+    expect(reader.getConfig('obj').get()).toEqual({ a: 1, c: 2 });
+    expect(reader.getConfig('obj').keys()).toEqual(['a', 'c']);
+
+    expect(reader.get('objA')).toEqual({ a: 1 });
+    expect(reader.get('objB')).toEqual({ a: 2 });
+  });
 });

packages/config/src/reader.ts

@@ -16,8 +16,6 @@
 
 import { JsonValue, JsonObject } from '@backstage/types';
 import { AppConfig, Config } from './types';
-import cloneDeep from 'lodash/cloneDeep';
-import mergeWith from 'lodash/mergeWith';
 
 // Update the same pattern in config-loader package if this is changed
 const CONFIG_KEY_PART_PATTERN = /^[a-z][a-z0-9]*(?:[-_][a-z][a-z0-9]*)*$/i;
@@ -26,6 +24,43 @@ function isObject(value: JsonValue | undefined): value is JsonObject {
   return typeof value === 'object' && value !== null && !Array.isArray(value);
 }
 
+function cloneDeep(value: JsonValue | null | undefined): JsonValue | undefined {
+  if (typeof value !== 'object' || value === null) {
+    return value;
+  }
+  if (Array.isArray(value)) {
+    return value.map(cloneDeep) as JsonValue;
+  }
+  return Object.fromEntries(
+    Object.entries(value).map(([k, v]) => [k, cloneDeep(v)]),
+  );
+}
+
+function merge(
+  into: JsonValue | undefined,
+  from?: JsonValue | undefined,
+): JsonValue | undefined {
+  if (into === null) {
+    return undefined;
+  }
+  if (into === undefined) {
+    return from === undefined ? undefined : merge(from);
+  }
+  if (typeof into !== 'object' || Array.isArray(into)) {
+    return into;
+  }
+  const fromObj = isObject(from) ? from : {};
+
+  const out: JsonObject = {};
+  for (const key of new Set([...Object.keys(into), ...Object.keys(fromObj)])) {
+    const val = merge(into[key], fromObj[key]);
+    if (val !== undefined) {
+      out[key] = val;
+    }
+  }
+  return out;
+}
+
 function typeOf(value: JsonValue | undefined): string {
   if (value === null) {
     return 'null';
@@ -47,8 +82,8 @@ const errors = {
   type(key: string, context: string, typeName: string, expected: string) {
     return `Invalid type in config for key '${key}' in '${context}', got ${typeName}, wanted ${expected}`;
   },
-  missing(key: string) {
-    return `Missing required config value at '${key}'`;
+  missing(key: string, context: string) {
+    return `Missing required config value at '${key}' in '${context}'`;
   },
   convert(key: string, context: string, expected: string) {
     return `Unable to convert config value for key '${key}' in '${context}' to a ${expected}`;
@@ -114,6 +149,9 @@ export class ConfigReader implements Config {
   /** {@inheritdoc Config.has} */
   has(key: string): boolean {
     const value = this.readValue(key);
+    if (value === null) {
+      return false;
+    }
     if (value !== undefined) {
       return true;
     }
@@ -124,23 +162,28 @@ export class ConfigReader implements Config {
   keys(): string[] {
     const localKeys = this.data ? Object.keys(this.data) : [];
     const fallbackKeys = this.fallback?.keys() ?? [];
-    return [...new Set([...localKeys, ...fallbackKeys])];
+    return [...new Set([...localKeys, ...fallbackKeys])].filter(
+      k => this.data?.[k] !== null,
+    );
   }
 
   /** {@inheritdoc Config.get} */
   get<T = JsonValue>(key?: string): T {
     const value = this.getOptional(key);
     if (value === undefined) {
-      throw new Error(errors.missing(this.fullKey(key ?? '')));
+      throw new Error(errors.missing(this.fullKey(key ?? ''), this.context));
     }
     return value as T;
   }
 
   /** {@inheritdoc Config.getOptional} */
   getOptional<T = JsonValue>(key?: string): T | undefined {
     const value = cloneDeep(this.readValue(key));
-    const fallbackValue = this.fallback?.getOptional<T>(key);
+    const fallbackValue = this.fallback?.getOptional(key);
 
+    if (value === null) {
+      return undefined;
+    }
     if (value === undefined) {
       if (process.env.NODE_ENV === 'development') {
         if (fallbackValue === undefined && key) {
@@ -158,23 +201,19 @@ export class ConfigReader implements Config {
           }
         }
       }
-      return fallbackValue;
+      return merge(fallbackValue) as T;
     } else if (fallbackValue === undefined) {
-      return value as T;
+      return merge(value) as T;
     }
 
-    // Avoid merging arrays and primitive values, since that's how merging works for other
-    // methods for reading config.
-    return mergeWith({}, { value: fallbackValue }, { value }, (into, from) =>
-      !isObject(from) || !isObject(into) ? from : undefined,
-    ).value as T;
+    return merge(value, fallbackValue) as T;
   }
 
   /** {@inheritdoc Config.getConfig} */
   getConfig(key: string): ConfigReader {
     const value = this.getOptionalConfig(key);
     if (value === undefined) {
-      throw new Error(errors.missing(this.fullKey(key)));
+      throw new Error(errors.missing(this.fullKey(key), this.context));
     }
     return value;
   }
@@ -187,6 +226,9 @@ export class ConfigReader implements Config {
     if (isObject(value)) {
       return this.copy(value, key, fallbackConfig);
     }
+    if (value === null) {
+      return undefined;
+    }
     if (value !== undefined) {
       throw new TypeError(
         errors.type(this.fullKey(key), this.context, typeOf(value), 'object'),
@@ -199,7 +241,7 @@ export class ConfigReader implements Config {
   getConfigArray(key: string): ConfigReader[] {
     const value = this.getOptionalConfigArray(key);
     if (value === undefined) {
-      throw new Error(errors.missing(this.fullKey(key)));
+      throw new Error(errors.missing(this.fullKey(key), this.context));
     }
     return value;
   }
@@ -244,7 +286,7 @@ export class ConfigReader implements Config {
   getNumber(key: string): number {
     const value = this.getOptionalNumber(key);
     if (value === undefined) {
-      throw new Error(errors.missing(this.fullKey(key)));
+      throw new Error(errors.missing(this.fullKey(key), this.context));
     }
     return value;
   }
@@ -273,7 +315,7 @@ export class ConfigReader implements Config {
   getBoolean(key: string): boolean {
     const value = this.getOptionalBoolean(key);
     if (value === undefined) {
-      throw new Error(errors.missing(this.fullKey(key)));
+      throw new Error(errors.missing(this.fullKey(key), this.context));
     }
     return value;
   }
@@ -305,7 +347,7 @@ export class ConfigReader implements Config {
   getString(key: string): string {
     const value = this.getOptionalString(key);
     if (value === undefined) {
-      throw new Error(errors.missing(this.fullKey(key)));
+      throw new Error(errors.missing(this.fullKey(key), this.context));
     }
     return value;
   }
@@ -323,7 +365,7 @@ export class ConfigReader implements Config {
   getStringArray(key: string): string[] {
     const value = this.getOptionalStringArray(key);
     if (value === undefined) {
-      throw new Error(errors.missing(this.fullKey(key)));
+      throw new Error(errors.missing(this.fullKey(key), this.context));
     }
     return value;
   }
@@ -384,6 +426,9 @@ export class ConfigReader implements Config {
 
       return this.fallback?.readConfigValue(key, validate);
     }
+    if (value === null) {
+      return undefined;
+    }
     const result = validate(value);
     if (result !== true) {
       const { key: keyName = key, value: theValue = value, expected } = result;
@@ -416,7 +461,7 @@ export class ConfigReader implements Config {
     for (const [index, part] of parts.entries()) {
       if (isObject(value)) {
         value = value[part];
-      } else if (value !== undefined) {
+      } else if (value !== undefined && value !== null) {
         const badKey = this.fullKey(parts.slice(0, index).join('.'));
         throw new TypeError(
           errors.type(badKey, this.context, typeOf(value), 'object'),

yarn.lock

@@ -3803,7 +3803,6 @@ __metadata:
     "@backstage/errors": "workspace:^"
     "@backstage/test-utils": "workspace:^"
     "@backstage/types": "workspace:^"
-    lodash: ^4.17.21
   languageName: unknown
   linkType: soft