Ensure default values don't go through validation, fix type narrowing…

… from defaults (#176)

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.

src/core.ts

@@ -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}"`)
@@ -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 })
       }

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> =
+  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.
@@ -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.
    */

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', () => {

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()', () => {