Added overwrite option

[jordaaash]

I added an option to overwrite the variables. This has been useful to me for a number of reasons, like overriding NODE_ENV for staging environments on the fly to test, loading multiple env files in succession, etc. In the process of adding a new test for it, I discovered that the stubs in beforeEach weren't setting the TEST env properly (they were looking at test instead), so the test for does not write over keys already in process.env was guaranteed to succeed, but not because the code was running correctly -- it was just assigning process.env.test rather than process.env.TEST. This patch also fixes that.

[motdotla]

We'll take a look soon @jordansexton.

In the meantime, can you fix line 41 of lib/main.js to fit the @feross standard style. Currently this is failing in travis as a result of that.

Error: Use JavaScript Standard Style (https://github.com/feross/standard)
  lib/main.js:41:11: Closing curly brace does not appear on the same line as the subsequent block.
npm ERR! Test failed.  See above for more details.

[maxbeatty]

I'm not in favor of this. Environment variables should vary by environment. If you are overwriting variables in an environment, you're misusing dotenv.

[jordaaash]

Misusing dotenv? I don't see why the library should be opinionated about how you get variables into your environment. It's job is just to do that. If you happen to want to overwrite ones that are already there, then why should it stop you from doing so?

[jordaaash]

For context, how I use it is with a .env file and a .env.[NODE_ENV] file. It's valuable to be able to use the former to provide environment variables that may be the same for several environments, and the latter to provide environment-specific variables, which may or may not be the same as the ones in .env. As such, I think it's reasonable to provide an option to overwrite them, and it's a non-breaking change. And FWIW, the original Ruby dotenv mentions this usage style and uses overwriting to do it.

[maxbeatty]

All libraries are opinionated. dotenv chooses to have as simple of an interface as possible, only introducing additional configuration when necessary.

Using multiple .env files is an anti-pattern because you are not distinguishing your environments. If you have values that are the same for several environments, you should reevaluate if they are indeed environmental or if you're sharing sensitive data which could lead to bigger problems (e.g. reusing your production database password in staging, having your staging environment compromised, and now having production at risk). By not having an overwrite option or reading multiple files by default, dotenv encourages a single .env file so developers don't fall into these traps.

How would you use this override option? What would the implementation look like?

var dotenv = require("dotenv");
dotenv.config(); // .env
dotenv.config({overwrite: true, path: ".env." + process.env.NODE_ENV});

You could just as easily switch the two lines to accomplish the same thing without the additional option:

var dotenv = require("dotenv");
dotenv.config({path: ".env." + process.env.NODE_ENV});
dotenv.config(); // .env

You can load environment variables however you like. We can maintain less code and documentation.

[jordaaash]

Except in your example, you can't override variables that are already part of the environment merely by reversing the order of the config calls. For example, let's imagine I want to debug an error that's occurring in production on my local machine with automated tests. With this change, I can add NODE_ENV=production to .env.test, load the config at the beginning of the suite as usual, and change the environment on the fly. This is not possible without overwriting, but this is far from the only case for it. At any rate, why should we expect developers to know the internals of the library and reverse the logical calls when you can offer them an easy out?

Putting your environment variables in files at all is an antipattern according to the 12 factor guidelines. If we're going to rely on dogma, this library shouldn't exist to begin with. I submit that it exists because it's convenient and not because it's correct, and thus we should care about developer convenience and not make the end user jump through hoops or unnecessarily block usages that may not be standard. And since the readme claims this is based on Ruby's dotenv, this is standard usage there regardless.

[maxbeatty]

Thanks for offering a solution to your problem. It's not something I feel would improve this module based on the usage you've outlined.

[jordaaash]

All libraries may be opinionated, but good tools can accommodate more than one opinion on how they should be used. While I appreciate the feedback and don't expect you to agree with me, it does seem like you'd rather not actually engage with this because it's not your preferred usage. The existing behavior is a sane default and keeps people from shooting themselves in the foot, but have you considered that with a few PRs now (#35, #55) for a feature like this, all less generally applicable than this one, it might be more than just "a solution to [my] problem"?

[rdsubhas]

@motdotla @maxbeatty I respect your opinion, and went through multiple bug reports. We're using Docker from development to production. Can you please suggest us an approach.

• The base image does not contain any .env file, since the same image is being used from Development to Production. Docker injects environment variables depending on the running environment.
• docker-compose up api runs the development container with a .env file. Environment variables are already set.
• For tests, we do docker-compose run api sh to start a shell. We use this dotenv library only for tests to load an additional .env.test file.
• Problem is, since Docker already sets environment variables from .env container-wide, none of the .env.test variables are overridden.
• This is a case where I felt having an override option would have helped.

Hope you get the scenario. Changing the order won't help, since there is no default .env file anywhere inside the image. As you would know, using Docker to create separate images for each environment just for the sake of a .env file would be very wrong. Any suggestions please?

[maxbeatty]

@rdsubhas I just have some brief experience with docker-compose and docker. I like that they have thought about and support environment variables. Because of their support, I do not think you should use dotenv. Have you tried docker-compose.yml's env-file config? Docker has a pretty active IRC channel that may be more helpful in figuring out a solution.

[rdsubhas]

@maxbeatty Yes exactly, I am using docker-compose.yml's env_file. But just when I'm running tests, I have to override.

To be a bit more clear:

• I set env_file=.env to use default environment
• I start the container, it gives me a development console
• In the console, I develop and run tests
• When running tests, I want to override using .env.local
• BUT since I have already specified env_file (in step 1), and the container is already booted with the default environment variables loaded system-wide, I am unable to override using .env.local using dotenv.

Right now, before starting all the tests, I'm manually loading the .env.test override file using dotenv.parse, and manually merging it with process.env.

[maxbeatty]

Perhaps you could have a docker-compose.test.yml file that specifies a different env_filew and start up the container using that file when you want to run tests? docker-compose -f docker-compose.test.yml -p my-app-testing start

Probably best to keep brainstorming this docker solution in their GH Issues, StackOverflow, or discuss live on IRC. Good luck!

[rdsubhas]

@maxbeatty nice suggestion, just that having a separate yml and env_file means I have to copy and paste all environment variables from dev to test, and just change the ones that differ. I was kinda more looking at test-only overrides. I have a temporary alternative in loading the env variables manually after parsing, will keep brainstorming in other place as well. Thanks for the help!