Treat "" As Missing Value

[beeequeue]

This PR treats "" values the same as undefined, with an option to use the old behavior.

We use dotenv files to show what options are available in our projects, with empty values in them to show that they are optional or not possible to commit to repositories:

PORT=8080
APP_ENV=development

SYSTEM_TOKEN=
CDN_URL=
GRAPHQL_URL=

// Current: Errors due to "" not being a valid port
url({
  input: process.env.CDN_URL, // ""
  devDefault: "https://...",
  default: "http://localhost:3000/cdn",
})
// New: Empty string is treated as null, falls back to defaults

This causes problems with envsafe when we have default values for these env vars (e.g. defaulting the URLs to localhost) since it results in process.env.CDN_URL = "".

• Test manually in project
• Add functionality
• Add tests
• Ensure other validators other than str() don't have the allowEmpty-property
• Update README.md

[KATT]

Hey Adam, thanks so much for the PR!

I really like the idea, I actually think it should be the default behaviour and an opt-out rather than an opt-in (breaking behaviour so I'll do a new major).

Would you mind updating your PR so the API is something like this? 👇

Other ideas on approaching the problem are welcome as well.. Also, tick the "Allow edits"-thingy, please :)

Default behaviour

import { str, envsafe, port, url } from 'envsafe';

export const env = envsafe({
  CDN_URL: str({
    devDefault: 'http://localhost:3000',
  }),
 // ...
})

❌ Fail if CDN_URL is empty string

New option allowEmpty: boolean

• Only on str() as the other ones are never gonna be valid as empty str

import { str, envsafe, port, url } from 'envsafe';

export const env = envsafe({
  CDN_URL: str({
    devDefault: 'http://localhost:3000',
    allowEmpty: true,
  }),
 // ...
})

✅ Succeed if CDN_URL is empty string

Checklist

• Add functionality
• Add tests
• Ensure other validators other than str() don't have the allowEmpty-property
• Update README.md

[beeequeue]

Sure I'll update it!

If you mean allow maintainers to edit it is already checked for me...

[KATT]

Great, looking forward to reviewing it. I've merged main so the CI will now run on this branch now.

Fine if you want to make a fresh PR as well!

[KATT]

See previous comment #31 (comment)

[beeequeue]

Looking into it more I realised that the issue exists on the validators that parse the string input - e.g. port and url. str is fine as it thinks "" is a valid string, and is g2g already.

After considering the different ways to do this I still think the best way to handle it is an option on envsafe rather than the validators as we would have to conditionally handle empty string inputs in every single one that it might apply to, which will be a mess to maintain. It'll be a lot easier to change an empty string into null

[KATT]

Empty string will never be valid on url nor number/port though, will it?

[beeequeue]

No, but it does allow defaulting over the missing value instead of it just erroring

e.g.

// Current: Errors due to "" not being a valid port
port({
  input: process.env.PORT, // ""
  devDefault: 3000,
  default: 80,
})

// New: Empty string is treated as null, falls back to defaults

[KATT]

Oh, true! I didn't consider defaults, now I'm with you.

Maybe an option to allowEmpty: boolean on the env options / all the validators then and throw MissingEnvError if it's not true and otherwise pass it to the parsers.

I'm still contemplating if it's better to do it granularly on the validators; I reckon it's rare that one would want to do this and it might be useful to do it granularly.

[KATT]

So -

• Either in Spec

  envsafe/src/types.ts

  Line 1 in f5e632a

  export interface Spec<TValue> {

• Or in EnvsafeOpts

  envsafe/src/types.ts

  Line 50 in f5e632a

  export type EnvsafeOpts<TCleanEnv> = {

I'm leaning towards Spec being nicer...

[KATT]

It'll probably leave the codebase cleaner to do it with the options object though, so do that if you want! Sorry for the back-and-forth! :)

[beeequeue]

I agree that having the option on the validators for more granular control is better, and I did finally realize that I just had to change where the option came from in the implementation for it to work...
So that's done as well as the documentation.

Should I also add a changelog? Just the simple template from https://keepachangelog.com/en/1.0.0/

[KATT]

Simplified a bit in f24df97

Released as 2.0.0 🚀

Thanks!