From c9ee24b8bb296a6df075a3f56d7de03a739d1aca Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 17 Jun 2020 17:28:26 +0200 Subject: [PATCH 1/3] packages/config: allow config readers to be backed by undefined data --- packages/config/src/reader.test.ts | 6 ++++++ packages/config/src/reader.ts | 19 ++++++++++++------- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/packages/config/src/reader.test.ts b/packages/config/src/reader.test.ts index f7804d5d49..2e61a34512 100644 --- a/packages/config/src/reader.test.ts +++ b/packages/config/src/reader.test.ts @@ -118,6 +118,8 @@ describe('ConfigReader', () => { expect(config.getOptionalString('X-x2')).toBeUndefined(); expect(config.getOptionalString('x0_x0')).toBeUndefined(); expect(config.getOptionalString('x_x-x_x')).toBeUndefined(); + + expect(new ConfigReader(undefined).getOptionalString('x')).toBeUndefined(); }); it('should throw on invalid keys', () => { @@ -138,6 +140,10 @@ describe('ConfigReader', () => { expect(() => config.getString('a.a.a.a.')).toThrow(/^Invalid config key/); expect(() => config.getString('a._')).toThrow(/^Invalid config key/); expect(() => config.getString('a.-.a')).toThrow(/^Invalid config key/); + + expect(() => new ConfigReader(undefined).getString('.')).toThrow( + /^Invalid config key/, + ); }); it('should read valid values', () => { diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts index ff7c8be6e5..f1ed8a4f3d 100644 --- a/packages/config/src/reader.ts +++ b/packages/config/src/reader.ts @@ -40,11 +40,9 @@ function typeOf(value: JsonValue | undefined): string { } export class ConfigReader implements Config { - private static readonly nullReader = new ConfigReader({}); - static fromConfigs(configs: AppConfig[]): ConfigReader { if (configs.length === 0) { - return new ConfigReader({}); + return new ConfigReader(undefined); } // Merge together all configs info a single config with recursive fallback @@ -55,13 +53,14 @@ export class ConfigReader implements Config { } constructor( - private readonly data: JsonObject, + private readonly data: JsonObject | undefined, private readonly fallback?: ConfigReader, ) {} getConfig(key: string): ConfigReader { const value = this.readValue(key); const fallbackConfig = this.fallback?.getConfig(key); + if (isObject(value)) { return new ConfigReader(value, fallbackConfig); } @@ -72,7 +71,7 @@ export class ConfigReader implements Config { )}, wanted object`, ); } - return fallbackConfig ?? ConfigReader.nullReader; + return fallbackConfig ?? new ConfigReader(undefined, undefined); } getConfigArray(key: string): ConfigReader[] { @@ -191,12 +190,18 @@ export class ConfigReader implements Config { private readValue(key: string): JsonValue | undefined { const parts = key.split('.'); - - let value: JsonValue | undefined = this.data; for (const part of parts) { if (!CONFIG_KEY_PART_PATTERN.test(part)) { throw new TypeError(`Invalid config key '${key}'`); } + } + + if (this.data === undefined) { + return undefined; + } + + let value: JsonValue | undefined = this.data; + for (const part of parts) { if (isObject(value)) { value = value[part]; } else { From 5294d71fe914d5f7f70dd9ab9281c5a44bb3ec21 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 17 Jun 2020 17:35:16 +0200 Subject: [PATCH 2/3] packages/config: optimize some error message handling --- packages/config/src/reader.ts | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts index f1ed8a4f3d..2c2b560536 100644 --- a/packages/config/src/reader.ts +++ b/packages/config/src/reader.ts @@ -39,6 +39,16 @@ function typeOf(value: JsonValue | undefined): string { return type; } +// Separate out a couple of common error messages to reduce bundle size. +const errors = { + type(key: string, typeName: string, expected: string) { + return `Invalid type in config for key ${key}, got ${typeName}, wanted ${expected}`; + }, + missing(key: string) { + return `Missing required config value at '${key}'`; + }, +}; + export class ConfigReader implements Config { static fromConfigs(configs: AppConfig[]): ConfigReader { if (configs.length === 0) { @@ -65,11 +75,7 @@ export class ConfigReader implements Config { return new ConfigReader(value, fallbackConfig); } if (value !== undefined) { - throw new TypeError( - `Invalid type in config for key ${key}, got ${typeOf( - value, - )}, wanted object`, - ); + throw new TypeError(errors.type(key, typeOf(value), 'object')); } return fallbackConfig ?? new ConfigReader(undefined, undefined); } @@ -94,7 +100,7 @@ export class ConfigReader implements Config { getNumber(key: string): number { const value = this.getOptionalNumber(key); if (value === undefined) { - throw new Error(`Missing required config value at '${key}'`); + throw new Error(errors.missing(key)); } return value; } @@ -109,7 +115,7 @@ export class ConfigReader implements Config { getBoolean(key: string): boolean { const value = this.getOptionalBoolean(key); if (value === undefined) { - throw new Error(`Missing required config value at '${key}'`); + throw new Error(errors.missing(key)); } return value; } @@ -124,7 +130,7 @@ export class ConfigReader implements Config { getString(key: string): string { const value = this.getOptionalString(key); if (value === undefined) { - throw new Error(`Missing required config value at '${key}'`); + throw new Error(errors.missing(key)); } return value; } @@ -140,7 +146,7 @@ export class ConfigReader implements Config { getStringArray(key: string): string[] { const value = this.getOptionalStringArray(key); if (value === undefined) { - throw new Error(`Missing required config value at '${key}'`); + throw new Error(errors.missing(key)); } return value; } @@ -178,10 +184,7 @@ export class ConfigReader implements Config { value: theValue = value, expected, } = result; - const typeName = typeOf(theValue); - throw new TypeError( - `Invalid type in config for key ${keyName}, got ${typeName}, wanted ${expected}`, - ); + throw new TypeError(errors.type(keyName, typeOf(theValue), expected)); } } From 625a50989d253eeb9417efac6082d59482281560 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 17 Jun 2020 17:54:11 +0200 Subject: [PATCH 3/3] packages/config: keep track of key prefix to display better error messages --- packages/config/src/reader.test.ts | 5 ++++- packages/config/src/reader.ts | 31 +++++++++++++++++++++--------- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/packages/config/src/reader.test.ts b/packages/config/src/reader.test.ts index 2e61a34512..f23fad8d3b 100644 --- a/packages/config/src/reader.test.ts +++ b/packages/config/src/reader.test.ts @@ -231,6 +231,9 @@ describe('ConfigReader with fallback', () => { 'a', 'b', ]); + expect(() => config.getConfig('merged').getStringArray('x')).toThrow( + 'Invalid type in config for key merged.x, got string, wanted string-array', + ); // Config arrays aren't merged either expect(config.getConfigArray('merged.configs').length).toBe(1); @@ -238,7 +241,7 @@ describe('ConfigReader with fallback', () => { expect(config.getConfigArray('merged.configs')[0].getString('a')).toBe('a'); expect(() => config.getConfigArray('merged.configs')[0].getString('missing'), - ).toThrow("Missing required config value at 'missing'"); + ).toThrow("Missing required config value at 'merged.configs[0].missing'"); expect( config.getConfigArray('merged.configs')[0].getOptionalString('b'), ).toBeUndefined(); diff --git a/packages/config/src/reader.ts b/packages/config/src/reader.ts index 2c2b560536..e3c349768b 100644 --- a/packages/config/src/reader.ts +++ b/packages/config/src/reader.ts @@ -65,19 +65,23 @@ export class ConfigReader implements Config { constructor( private readonly data: JsonObject | undefined, private readonly fallback?: ConfigReader, + private readonly prefix: string = '', ) {} getConfig(key: string): ConfigReader { const value = this.readValue(key); const fallbackConfig = this.fallback?.getConfig(key); + const prefix = this.fullKey(key); if (isObject(value)) { - return new ConfigReader(value, fallbackConfig); + return new ConfigReader(value, fallbackConfig, prefix); } if (value !== undefined) { - throw new TypeError(errors.type(key, typeOf(value), 'object')); + throw new TypeError( + errors.type(this.fullKey(key), typeOf(value), 'object'), + ); } - return fallbackConfig ?? new ConfigReader(undefined, undefined); + return fallbackConfig ?? new ConfigReader(undefined, undefined, prefix); } getConfigArray(key: string): ConfigReader[] { @@ -94,13 +98,16 @@ export class ConfigReader implements Config { return true; }); - return (configs ?? []).map(obj => new ConfigReader(obj)); + return (configs ?? []).map( + (obj, index) => + new ConfigReader(obj, undefined, this.fullKey(`${key}[${index}]`)), + ); } getNumber(key: string): number { const value = this.getOptionalNumber(key); if (value === undefined) { - throw new Error(errors.missing(key)); + throw new Error(errors.missing(this.fullKey(key))); } return value; } @@ -115,7 +122,7 @@ export class ConfigReader implements Config { getBoolean(key: string): boolean { const value = this.getOptionalBoolean(key); if (value === undefined) { - throw new Error(errors.missing(key)); + throw new Error(errors.missing(this.fullKey(key))); } return value; } @@ -130,7 +137,7 @@ export class ConfigReader implements Config { getString(key: string): string { const value = this.getOptionalString(key); if (value === undefined) { - throw new Error(errors.missing(key)); + throw new Error(errors.missing(this.fullKey(key))); } return value; } @@ -146,7 +153,7 @@ export class ConfigReader implements Config { getStringArray(key: string): string[] { const value = this.getOptionalStringArray(key); if (value === undefined) { - throw new Error(errors.missing(key)); + throw new Error(errors.missing(this.fullKey(key))); } return value; } @@ -165,6 +172,10 @@ export class ConfigReader implements Config { }); } + private fullKey(key: string): string { + return `${this.prefix}${this.prefix ? '.' : ''}${key}`; + } + private readConfigValue( key: string, validate: ( @@ -184,7 +195,9 @@ export class ConfigReader implements Config { value: theValue = value, expected, } = result; - throw new TypeError(errors.type(keyName, typeOf(theValue), expected)); + throw new TypeError( + errors.type(this.fullKey(keyName), typeOf(theValue), expected), + ); } }