manual loading and parsing of dotenv in watch mode #558

[tandibar]

I think this could be the fix for the issue #558 that I opened.

The problem with dotenv.config().parsed is that dotenv.config() modifies the process.env. So the fix uses dotenv.parse() directly so that `process.env is untouched.

All tests are green. I will provide an additional test in the next days.

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[mcollina]

lgtm

[mcollina]

Thanks for opening a PR! Can you please add a unit test?

[tandibar]

So I added a unit test and implemented the feature. The problem is, that it breaks another feature, see test test/start.test.js should reload the env on restart when watching. In the current implementation I think it is very hard to have both features. For me it would be more consistent if dotenv does not change its behavior in watch mode. But that would mean, that you have to restart your server if you change your .env even in watch mode.
What do you think?

(I intentionally commited with --no-verify so that you can see and test the conflicting tests for yourself.)

[mcollina]

that would mean, that you have to restart your server if you change your .env even in watch mode.

Why? I think it might be possible to have the env variables defined in process.env to have priority over the ones in .env.
Or viceversa. I think the surprising behavior is lack of consistency.

[tandibar]

So the problem ist, that dotenv is already loaded before, so we cannot just check which env vars are already present, because all are present. So I decided to check if there is a difference between the process.env and the parsed dotenv. If yes, we should not overwrite the env var. If it is the same, it seems to be managed by dotenv file, so we should overwrite.
Tests are now all passing, so I think that is the solution.