keithmorris / node-dotenv-extended

A module for loading .env files and optionally loading defaults and a schema for validating all values are present.
MIT License
111 stars 24 forks source link

Missing values should default to empty strings for errorOnRegex #45

Closed FokkeZB closed 4 years ago

FokkeZB commented 4 years ago

IMO the RegExp().test() for errorOnRegex should default missing - or actually any non-string - values to "".

Currently, ^(true|undefined)$ is required to require a variable to either be "true" or not set. This feels hacky as it relies on RegExp().test() casting non-string values to a string, in which case undefined becomes "undefined".

As environment variables should always be strings, I think it's better to default non-strings to "" so that you can use ^(true)?$ to require a value to either be "true" or not set.

Without the change in src/index.js the newly added test fails with:

      AssertionError: expected [Function: runTest] to throw error including 'REGEX MISMATCH: TEST_MISSING_REQUIRED' but got 'REGEX MISMATCH: TEST_MISSING_OPTIONAL, TEST_MISSING_REQUIRED'
      + expected - actual

      -REGEX MISMATCH: TEST_MISSING_OPTIONAL, TEST_MISSING_REQUIRED
      +REGEX MISMATCH: TEST_MISSING_REQUIRED

Also see #41

coveralls commented 4 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 61b99ed8b4ff597663680364bc7fa6c462a27971 on FokkeZB:RegExp-String into 43e550d0dce3b6345398915958fa6e31eea24672 on keithmorris:develop.

coveralls commented 4 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 61b99ed8b4ff597663680364bc7fa6c462a27971 on FokkeZB:RegExp-String into 43e550d0dce3b6345398915958fa6e31eea24672 on keithmorris:develop.

coveralls commented 4 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 61b99ed8b4ff597663680364bc7fa6c462a27971 on FokkeZB:RegExp-String into 43e550d0dce3b6345398915958fa6e31eea24672 on keithmorris:develop.

keithmorris commented 4 years ago

@FokkeZB This makes complete sense. Thank you for the PR. I will merge and release shortly.