Optional values

[LandonSchropp]

I would like to validate an environment variable, but allow it to optionally be undefined. I know I can do something like this:

MY_AWESOME_VARIABLE: envalid.str({
  default: '',
  choices: ['astonishing', 'awe-inspiring', 'magnificent', 'wonderous'],
  desc: 'My awesome variable'
})

However, it seems more precise to have the value of MY_AWESOME_VARIABLE be undefined (since it's not defined). Is there a way to achieve this with Envalid? Something like this would be great:

MY_AWESOME_VARIABLE: envalid.str({
  required: false,
  choices: ['astonishing', 'awe-inspiring', 'magnificent', 'wonderous'],
  desc: 'My awesome variable'
})

Thanks!

[SimenB]

SGTM. If @af wants it, I can send a PR for it

[af]

Rather than add a new option, I think it'd be better to support setting default: undefined, which would be the same as required: false. Right now setting default: undefined will trigger a validation error, but we should be able to change it so that passing undefined explicitly works this way instead. Thoughts?

[LandonSchropp]

I like that better. Supporting null would also be helpful depending on the context.

[SimenB]

We can't detect if undefined is passed explicitly or left out. So either special casing null or a new option is the way to go

[SimenB]

(we could do Object.keys and count, but that feels way too fragile)

[LandonSchropp]

default: null also seems like a fine solution to me.

[LandonSchropp]

You could use Object#hasOwnProperty to detect if the property is defined on the object, but that breaks if the user passes in config that inherits from a prototype. I'm not sure why somebody would do that, but I don't see a reason to prevent it.

[af]

Yeah I was thinking about hasOwnProperty(); I believe if ('default' in spec) should also work for these purposes. The latter is a bit more readable to me. There is the prototype gotcha, but I can't think of a case where anyone would use a prototype-based config, so I'm comfortable with going this way.

[af]

^ the commit above is a stab at how I was thinking this would work. It fails a test, plus I want to add new test(s) to cover the new use case, so it's not ready for prime-time yet. Could someone take a look and let me know if there's anything obviously wrong with this approach?

[SimenB]

It looks fine to me 🙂