Skip to content

Commit

Permalink
Ensure default values don't go through validation, fix type narrowing…
Browse files Browse the repository at this point in the history
… from defaults
  • Loading branch information
af committed Nov 3, 2021
1 parent 086609b commit 269c588
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 25 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
39 changes: 22 additions & 17 deletions src/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ function validateVar<T>({
name,
rawValue,
}: {
name: string
rawValue: string | T
spec: Spec<T> & { _parse: (input: string) => T }
name: string,
rawValue: string | T,
spec: ValidatorSpec<T>,
}) {
if (typeof spec._parse !== 'function') {
throw new EnvError(`Invalid spec for "${name}"`)
Expand Down Expand Up @@ -63,30 +63,35 @@ export function getSanitizedEnv<T>(

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)) {
throw new EnvMissingError(formatSpecDescription(spec))
}

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 })
}
Expand Down
14 changes: 12 additions & 2 deletions src/types.ts
Original file line number Diff line number Diff line change
@@ -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> =
T extends string ? string :
T extends number ? number :
T extends boolean ? boolean :
T extends object ? object :
any

export interface Spec<T> {
/**
* An Array that lists the admissable parsed values for the env var.
Expand All @@ -6,12 +16,12 @@ export interface Spec<T> {
/**
* 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<T>
/**
* 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<T>
/**
* A string that describes the env var.
*/
Expand Down
16 changes: 11 additions & 5 deletions tests/basics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
5 changes: 5 additions & 0 deletions tests/validators.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()', () => {
Expand Down

0 comments on commit 269c588

Please sign in to comment.