Support options: explicit opts.env and opts.allowUnknown

[devinivy]

While working on #7 and testing things out against the hapipal boilerplate, I had some thought which I figured I'd express through code, which is what you find here. There are a few different ideas in here, which are fully tested but not documented. If you want to accept any of these ideas, I'd be happy to complete the documentation and split-out any features you do want from those you do not.

• Drop node v8 support Node v8 has hit its end-of-life. I know you're planning a new major release for #7, so thought it might be a good opportunity to end support for old versions of node.

• Test against windows It's clear this library is intended to work with windows, so I figured we might as well give it "the treatment" and ensure the test suite also runs properly everywhere. This may require a bit more work, and I'm willing to spend some more time with it.

• Fix all lint warnings I spent some time cleaning-up lint warnings and // eslint-disable-lines while I was in there simply because it wasn't too hard to do so. Admittedly the 30 statement limit within functions made me write some things differently than I would have liked, but it still seems like a good result.

• Support env option to override implicit process.env input Supporting an option to allow the user to pass the env input allows a few nice things: a. the user can pre-process the environment with some of their own logic if they like without mutating process.env, b. by making envy a little more pure it makes some cases simpler to test, which I was able to leverage immediately (including writing the first test of some behavior that predates this PR: "ignores unknown global env vars by default").

• Support allowUnknown option to allow specific vars not listed in .env files While working with the boilerplate I ran into some more use-cases that seemed to call for this. For example, I would like to always support NODE_ENV without forcing the user to manually provide it in the example file and .env file. Silently ignoring NODE_ENV=production (e.g. if the users passes it via CLI or a process runner) could cause issues for users when that environment variable turns on/off important checks, routines, and configurations in production. And at the same time I would like this var to remain optional, as production is the only special value that the boilerplate cares about (in the 12 factor spirit, we minimize use of "named groups"). It's worth noting that the .env file is also not used in some variants of the boilerplate, e.g. the docker flavor. I think this is a reasonable escape hatch for users with special needs for their application, but I also see how it may not be a welcome feature. I would propose envy maintain the same default functionality, because I do like envy's opinionation on this point even though it causes issues for the boilerplate.

[sholladay]

This is looking good so far, thanks Devin!

Drop node v8 support

👍

Test against windows

Seems like this PR closes #3, then. Nice. One thing I'm not sure about is whether Travis CI's Windows environment provides a useful representation of file permissions, as I believe it uses git bash, which of course has to translate *nix style permissions to Windows style permissions when our setup-tests.sh script runs, and this conversion is "lossy" as the permission models are fairly different. The end result is that, while setup-tests.sh may run without errors, I'm uncertain as to whether it's a good approximation of real (non-bash) Windows usage. On a related note, according to https://github.com/sholladay/envy/issues/4, our Windows support may be broken as the user reports that chmod 555 doesn't really work (presumably due to the lossy conversion), though I haven't gotten around to reproducing that bug report yet. Ultimately, we just need to figure out what the strictest set of permissions are (from Node's fs.statSync() perspective) that can be set on Windows that are as close as possible to 600, then update the code accordingly. (Note: I'm okay with requiring permissions that cannot be set from git bash via chmod, as long as there's some way to do it, whether it be through PowerShell or otherwise).

Fix all lint warnings ... Admittedly the 30 statement limit within functions made me write some things differently than I would have liked, but it still seems like a good result.

Ah, yes, the max statements rule, I've run into that before. Something about this codebase makes me want to write one long function with more statements than I usually would. I think because the internals are fairly domain specific and there's not a whole lot of isolated or reusable logic.

I'd actually prefer for you to write the code as clearly / elegantly as possible and then disable that rule if necessary. I'll either refactor it myself or loosen up the linter configuration in package.json to clean up the noisy comments after merging.

Support env option to override implicit process.env input

I like it, generally. How do you see the interplay between this and https://github.com/sholladay/envy/issues/6? Should envy.assign() assign the result to process.env directly or assign to the env option object, which merely defaults to process.env? The latter seems reasonable, but what if a user wants to use both a custom env object as input and assign the output to process.env? I suppose we could have an assign option that takes either a boolean or an object to assign to, but envy({ assign : someObject }) seems confusingly named for that behavior. Thoughts?

Support allowUnknown option to allow specific vars not listed in .env files

Did you see the CLI tip? It sort of covers this use case and that same pattern could be extended to mix in environment variables, but I'll grant that it may not be as simple as desired. I'm open to allowUnknown (should probably be named stripUnknown, though).

[devinivy]

Great, thanks for the speedy feedback.

Test against windows

I hear all of that 👍 . I actually really underestimated the complexity of this task. I spent some time today trying to understand node's view of Windows file modes (without direct access to a Windows machine), and I will say I did not find clarity! I didn't even come across userland node libraries for working with Windows permissions. I will post a little blurb on #4 when I get a chance, but TLDR: anything can happen particularly under WSL (here's some useful documentation). #4 is a bit of an issue for my purposes with the pal boilerplate because I would not like to block Windows users with permissions errors that they can't resolve, so I hope to help push this forward, but might have to separate it from this PR and follow-up on #3 and #4.

I'd actually prefer for you to write the code as clearly / elegantly as possible and then disable that rule if necessary. I'll either refactor it myself or loosen up the linter configuration in package.json to clean up the noisy comments after merging.

Got it! In that case I might make some small tweaks to how I have left it.

How do you see the interplay between this [env option] and #6?

I think to cater to the case you're talking about you might consider two separate methods. I imagine reading from my.env and overwriting their.env would might look like this:

const result = envy({ env: my.env });
envy.assign(result, { env: their.env });

Both would default to use process.env, so usage with process.env could be as simple as this:

envy.assign(envy());

Did you see the CLI tip? ... could be extended to mix in environment variables

I did read it but hadn't considered using it that way, interesting. I will investigate! I suppose camelizing the keys identically to envy and filtering the object are the only two "obstacles" from pursuing that.

I'm open to allowUnknown (should probably be named stripUnknown, though).

Not sure I fully understand. I imagined a granular allow-list of env vars typically being used with this option e.g. allowUnknown: ['NODE_ENV'] with a default of [] (upholding the current behavior). Is that also what you were thinking? Or perhaps you were imagining a simple boolean, in which case I think I see what you mean.