Set dotenv_config_* vars via env var #354

abcd-ca · 2018-11-23T19:54:57Z

Now dotenv_config_* can be supplied in a system env var instead of in CLI command.

Helps some IDEs that get confused with the CLI option like WebStorm/IntelliJ. WebStorm’s application of those command line config options was resulting in node trying to load them as additional npm modules rather than simple argv options that dotenv would consume.

Now `dotenv_config_*` can be supplied in a system env var instead of in CLI command. Helps some IDEs that get confused with the CLI option like WebStorm/IntelliJ. WebStorm’s application of those command line config options was resulting in node trying to load them as additional npm modules rather than simple argv options that dotenv would consume.

abcd-ca · 2018-11-23T20:14:31Z

@motdotla My tests pass locally, I'm not sure what the problem is with the failed build here, I'm not understanding the reported problem.

maxbeatty · 2018-11-24T15:26:25Z

As I mentioned in #351, what problem is really being solved?

abcd-ca · 2018-11-26T20:01:43Z

@maxbeatty Thanks for taking a look! I'm not too sure if #351 is solving the same problem or not, it's hard to tell. What I know is that while passing the dotenv_config_path on the command line (bash) works because the variable is treated as an argv as it's supposed to.

My IDE has some nice features that enhance mocha testing. As such, you can configure how to run the mocha tests from within the IDE. You have to tell it which node arguments to send, which environment variables, the working directory etc. The problem, is that when I put

-r dotenv/config your_script.js dotenv_config_path=/custom/path/to/your/env/vars

as the node arguments, it tries to treat all of that above as part of the --require value. dotenv_confi_path does not end up in argv and I get an error that dotenv_config_path can not be found in node modules (it's trying to --require it).

My fix allows dotenv to look in argv and then in environment variables. It allows me to then put the dotenv_config_path in the environment variables section in my IDE and I can then run my tests.

abcd-ca · 2018-11-26T20:09:52Z

@maxbeatty I took a closer look at #351. He's trying to solve a different problem where variables meant for the dotenv library conflict with other apps that receive (argv) arguments that they don't expect and then complain about. I think he's saying that if variables destined for dotenv were supplied via environment variables (at least optionally), then they wouldn't conflict. I gather that in his case Jest is complaining about the unknown argument.

In my case it's my IDE that isn't treating the dotenv variable as an argv variable, rather it's treating it as a node value of the --require flag; solved by use of environment variables. Note, I still like your original argv version – I prefer it and use it in my npm scripts but I want this workaround I created for situations where it's incompatible. I also included additional documentation to support this in the readme in this PR. Let me know if you have any more questions, I do think this fix would help with #351.

maxbeatty · 2018-11-26T20:18:30Z

Both #351 and this PR are proposing the same addition: ability to configure dotenv via environment variables

Let’s continue discussing if this is necessary in #351 and then if it is, come up with a plan to add

abcd-ca · 2018-11-26T20:44:58Z

@maxbeatty further, this is what I get when I do it the argv way via my WebStorm IDE:

/usr/local/bin/node -r dotenv/config dotenv_config_path=./src_server/.env.test /Users/foo/Documents/projects/bar/node_modules/mocha/bin/_mocha --require ts-node/register --ui bdd --reporter /Applications/WebStorm.app/Contents/plugins/NodeJS/js/mocha-intellij/lib/mochaIntellijReporter.js ./src_server/tests/integration/**/.test.ts
module.js:538
throw err;
^

Error: Cannot find module '/Users/foo/Documents/projects/bar/dotenv_config_path=./src_server/.env.test'
at Function.Module._resolveFilename (module.js:536:15)
at Function.Module._load (module.js:466:25)
at Function.Module.runMain (module.js:676:10)
at startup (bootstrap_node.js:187:16)
at bootstrap_node.js:608:3

Process finished with exit code 1

abcd-ca · 2018-11-26T20:48:28Z

And here is the way that the IDE asks for the params. My fix works when I move the dotenv_config_path to the environment variables section now.

abcd-ca · 2018-11-28T20:44:48Z

@maxbeatty Can you help me see what the failed unit tests are complaining about? The tests pass for me locally

abcd-ca · 2018-11-28T20:52:39Z

FYI, here's my local success with the same version of node that appveyor CI is dying at without much explanation:

myuser$ node -v
v6.14.4
myuser$ npm test

> dotenv@6.2.0-rc1 pretest /Users/myuser/Downloads/dotenv
> npm run lint


> dotenv@6.2.0-rc1 lint /Users/myuser/Downloads/dotenv
> standard


> dotenv@6.2.0-rc1 postlint /Users/myuser/Downloads/dotenv
> standard-markdown




> dotenv@6.2.0-rc1 test /Users/myuser/Downloads/dotenv
> tap tests/*.js --100

tests/test-cli-options.js ............................. 5/5
tests/test-config-cli.js .............................. 1/1 709ms
tests/test-config.js ................................ 14/14
tests/test-parse.js ................................. 17/17
total ............................................... 37/37

  37 passing (3s)

  ok
----------------------|----------|----------|----------|----------|-------------------|
File                  |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
----------------------|----------|----------|----------|----------|-------------------|
All files             |      100 |      100 |      100 |      100 |                   |
 lib                  |      100 |      100 |      100 |      100 |                   |
  cli-options.js      |      100 |      100 |      100 |      100 |                   |
  main.js             |      100 |      100 |      100 |      100 |                   |
 tests                |      100 |      100 |      100 |      100 |                   |
  test-cli-options.js |      100 |      100 |      100 |      100 |                   |
  test-config-cli.js  |      100 |      100 |      100 |      100 |                   |
  test-config.js      |      100 |      100 |      100 |      100 |                   |
  test-parse.js       |      100 |      100 |      100 |      100 |                   |
----------------------|----------|----------|----------|----------|-------------------|

config.js

lib/env-options.js

Node 6 didn’t support spread operator on objects, only arrays. Later versions of Node do support this FYI. Replaced with older syntax

maxbeatty · 2018-12-03T18:47:57Z

lib/env-options.js

+/* @flow */
+
+module.exports = {
+  encoding: process.env.dotenv_config_encoding,


Let’s use uppercase to match our existing documentation as well as Node’s official docs. Also, a simple unit test will help with integration tests.

maxbeatty · 2018-12-03T18:50:05Z

README.md

@@ -65,6 +65,9 @@ The configuration options below are supported as command line arguments in the f
 $ node -r dotenv/config your_script.js dotenv_config_path=/custom/path/to/your/env/vars
 ```

+Note, some IDEs like Webstorm and IntellJ might have trouble properly passing `dotenv_config_path` to node. This does not impact command line use, just when configuring run options within the IDE. 


Let’s scrap this IDE-specific stuff and instead expand the existing docs to explain how to use new env vars. Examples outlined here would be a good start

I think it would be helpful for those with IDE problems to mention something like, "If your IDE is having trouble passing the CLI config arguments like dotenv_config_path you can use environment variables instead (linked to that section)". It will save someone time, I'm sure. We don't have to mention any specific IDE this way. What do you think?

It would be better to take IDE-specific questions to StackOverflow or other support forums. Write a blog post about how you fixed your issue. Those help people the most.

OK, I'll do that and make the readme more generic

maxbeatty · 2018-12-03T18:50:49Z

config.js

@@ -2,6 +2,6 @@

 (function () {
  require('./lib/main').config(
-    require('./lib/cli-options')(process.argv)
+    Object.assign({}, require('./lib/env-options'), require('./lib/cli-options')(process.argv))


Let’s add some integration tests to verify behavior that documentation outlines

maxbeatty · 2018-12-04T01:08:58Z

to avoid more back and forth comments, I opened #355 based on this with what I had in mind

abcd-ca · 2018-12-04T18:00:35Z

my pr works, your pr works

abcd-ca mentioned this pull request Nov 26, 2018

dotenv_config_path as environment variable itself [feature request] #351

Closed

maxbeatty suggested changes Dec 3, 2018

View reviewed changes

config.js Outdated Show resolved Hide resolved

lib/env-options.js Outdated Show resolved Hide resolved

abcd-ca added 2 commits December 3, 2018 10:08

fix to accommodate Node 6

e0f3a96

Node 6 didn’t support spread operator on objects, only arrays. Later versions of Node do support this FYI. Replaced with older syntax

replaced reducer with simple object

b6868f4

maxbeatty suggested changes Dec 3, 2018

View reviewed changes

options via env vars: added tests and refactored

da595e3

maxbeatty mentioned this pull request Dec 4, 2018

add support for preload configuration via env vars #355

Merged

maxbeatty closed this Dec 5, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set dotenv_config_* vars via env var #354

Set dotenv_config_* vars via env var #354

abcd-ca commented Nov 23, 2018

abcd-ca commented Nov 23, 2018

maxbeatty commented Nov 24, 2018

abcd-ca commented Nov 26, 2018 •

edited

abcd-ca commented Nov 26, 2018

maxbeatty commented Nov 26, 2018

abcd-ca commented Nov 26, 2018

abcd-ca commented Nov 26, 2018

abcd-ca commented Nov 28, 2018

abcd-ca commented Nov 28, 2018

maxbeatty Dec 3, 2018

maxbeatty Dec 3, 2018

abcd-ca Dec 3, 2018

maxbeatty Dec 3, 2018

abcd-ca Dec 3, 2018

maxbeatty Dec 3, 2018

abcd-ca Dec 3, 2018

maxbeatty commented Dec 4, 2018

abcd-ca commented Dec 4, 2018

Set dotenv_config_* vars via env var #354

Set dotenv_config_* vars via env var #354

Conversation

abcd-ca commented Nov 23, 2018

abcd-ca commented Nov 23, 2018

maxbeatty commented Nov 24, 2018

abcd-ca commented Nov 26, 2018 • edited

abcd-ca commented Nov 26, 2018

maxbeatty commented Nov 26, 2018

abcd-ca commented Nov 26, 2018

abcd-ca commented Nov 26, 2018

abcd-ca commented Nov 28, 2018

abcd-ca commented Nov 28, 2018

maxbeatty Dec 3, 2018

Choose a reason for hiding this comment

maxbeatty Dec 3, 2018

Choose a reason for hiding this comment

abcd-ca Dec 3, 2018

Choose a reason for hiding this comment

maxbeatty Dec 3, 2018

Choose a reason for hiding this comment

abcd-ca Dec 3, 2018

Choose a reason for hiding this comment

maxbeatty Dec 3, 2018

Choose a reason for hiding this comment

abcd-ca Dec 3, 2018

Choose a reason for hiding this comment

maxbeatty commented Dec 4, 2018

abcd-ca commented Dec 4, 2018

abcd-ca commented Nov 26, 2018 •

edited