From ef5a895f70a369352c4954f836a0bf08b0411d89 Mon Sep 17 00:00:00 2001 From: Aaron Franks <25241+af@users.noreply.github.com> Date: Sun, 6 Feb 2022 19:50:18 -0800 Subject: [PATCH] Ensure default values don't go through validation, fix type narrowing from defaults (#176) --- README.md | 2 +- src/core.ts | 39 ++++++++++++++++++++++----------------- src/types.ts | 14 ++++++++++++-- tests/basics.test.ts | 16 +++++++++++----- tests/validators.test.ts | 5 +++++ 5 files changed, 51 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 664026d..2739978 100644 --- a/README.md +++ b/README.md @@ -107,7 +107,7 @@ Each validation function accepts an (optional) object with the following attribu * `choices` - An Array that lists the admissable parsed values for the env var. * `default` - A fallback value, which will be present in the output if the env var wasn't specified. Providing a default effectively makes the env var optional. Note that `default` - values are not passed through validation logic. + values are not passed through validation logic, they are default *output* values. * `devDefault` - A fallback value to use *only* when `NODE_ENV` is _not_ `'production'`. This is handy for env vars that are required for production environments, but optional for development and testing. diff --git a/src/core.ts b/src/core.ts index 6e4dadd..4b4c75a 100644 --- a/src/core.ts +++ b/src/core.ts @@ -15,9 +15,9 @@ function validateVar({ name, rawValue, }: { - name: string - rawValue: string | T - spec: Spec & { _parse: (input: string) => T } + name: string, + rawValue: string | T, + spec: ValidatorSpec, }) { if (typeof spec._parse !== 'function') { throw new EnvError(`Invalid spec for "${name}"`) @@ -63,20 +63,25 @@ export function getSanitizedEnv( for (const k of varKeys) { const spec = specs[k] + const rawValue = readRawEnvValue(environment, k) - // Use devDefault values only if NODE_ENV was explicitly set, and isn't 'production' - const usingDevDefault = - rawNodeEnv && rawNodeEnv !== 'production' && spec.hasOwnProperty('devDefault') - const devDefaultValue = usingDevDefault ? spec.devDefault : undefined - const rawValue = - readRawEnvValue(environment, k) ?? - (devDefaultValue === undefined ? spec.default : devDefaultValue) - - // Default values can be anything falsy (including an explicitly set undefined), without - // triggering validation errors: - const usingFalsyDefault = - (spec.hasOwnProperty('default') && spec.default === rawValue) || - (usingDevDefault && devDefaultValue === rawValue) + // If no value was given and default/devDefault were provided, return the appropriate default + // value without passing it through validation + if (rawValue === undefined) { + // Use devDefault values only if NODE_ENV was explicitly set, and isn't 'production' + const usingDevDefault = + rawNodeEnv && rawNodeEnv !== 'production' && spec.hasOwnProperty('devDefault') + if (usingDevDefault) { + // @ts-expect-error default values can break the rules slightly by being explicitly set to undefined + cleanedEnv[k] = spec.devDefault; + break; + } + if (spec.hasOwnProperty('default')) { + // @ts-expect-error default values can break the rules slightly by being explicitly set to undefined + cleanedEnv[k] = spec.default; + break; + } + } try { if (isTestOnlySymbol(rawValue)) { @@ -84,9 +89,9 @@ export function getSanitizedEnv( } if (rawValue === undefined) { - if (!usingFalsyDefault) throw new EnvMissingError(formatSpecDescription(spec)) // @ts-ignore (fixes #138) Need to figure out why explicitly undefined default/devDefault breaks inference cleanedEnv[k] = undefined + throw new EnvMissingError(formatSpecDescription(spec)) } else { cleanedEnv[k] = validateVar({ name: k as string, spec, rawValue }) } diff --git a/src/types.ts b/src/types.ts index dfc59ef..6ce7cfa 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,3 +1,13 @@ +// Hacky conditional type to prevent default/devDefault from narrowing type T to a single value. +// Ideally this could be replaced by something that would enforce the default value being a subset +// of T, without affecting the definition of T itself +type DefaultType = + T extends string ? string : + T extends number ? number : + T extends boolean ? boolean : + T extends object ? object : + any + export interface Spec { /** * An Array that lists the admissable parsed values for the env var. @@ -6,12 +16,12 @@ export interface Spec { /** * A fallback value, which will be used if the env var wasn't specified. Providing a default effectively makes the env var optional. */ - default?: T + default?: DefaultType /** * A fallback value to use only when NODE_ENV is not 'production'. * This is handy for env vars that are required for production environments, but optional for development and testing. */ - devDefault?: T + devDefault?: DefaultType /** * A string that describes the env var. */ diff --git a/tests/basics.test.ts b/tests/basics.test.ts index f02555c..04ee5b4 100644 --- a/tests/basics.test.ts +++ b/tests/basics.test.ts @@ -120,17 +120,23 @@ test('choices field', () => { test('choices should refine the type of the field to a union', () => { type NodeEnvType = 'production' | 'test' | 'development' - const spec = cleanEnv({ NODE_ENV: 'test' }, { - NODE_ENV: str({ choices: ['production', 'test', 'development'] }), - }) + const env = cleanEnv( + { NODE_ENV: 'test' }, + { + NODE_ENV: str({ choices: ['production', 'test', 'development'] }), + WITH_DEFAULT: str({ choices: ['production', 'test', 'development'], default: 'production' }), + }, + ) // type of the output should be the union type, not the more generic `string` - const nodeEnv: NodeEnvType = spec.NODE_ENV + const nodeEnv: NodeEnvType = env.NODE_ENV + const withDefault: NodeEnvType = env.WITH_DEFAULT // @ts-expect-error specifying a type that doesn't match the choices union type should cause an error - const shouldFail: 'test' | 'wrong' = spec.NODE_ENV + const shouldFail: 'test' | 'wrong' = env.NODE_ENV expect(nodeEnv).toEqual('test') + expect(withDefault).toEqual('production') }) test('misconfigured spec', () => { diff --git a/tests/validators.test.ts b/tests/validators.test.ts index f7f9a6a..4fa1b42 100644 --- a/tests/validators.test.ts +++ b/tests/validators.test.ts @@ -96,6 +96,11 @@ test('json()', () => { expect(env).toEqual({ FOO: { x: 123 } }) expect(() => cleanEnv({ FOO: 'abc' }, { FOO: json() }, makeSilent)).toThrow() + + // default value should be passed through without running through JSON.parse() + expect(cleanEnv({}, { + FOO: json({ default: { x: 999 } }) + })).toEqual({ FOO: { x: 999 } }) }) test('url()', () => {