Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dotenv_config_path as environment variable itself [feature request] #351

Closed
damianobarbati opened this issue Nov 16, 2018 · 7 comments · Fixed by #355
Closed

dotenv_config_path as environment variable itself [feature request] #351

damianobarbati opened this issue Nov 16, 2018 · 7 comments · Fixed by #355

Comments

@damianobarbati
Copy link

Having to pass dotenv_config_path as process argument yields some downside:

  • argv is polluted with it so the application has to handle this additional logic (filtering it out from args)
  • concatenating command in a npm/yarn script may be messy in some cases when your command expects a second dynamic part (you have to deal with dotenv_config_path everywhere)

Is it possible to pass dotenv_config_path as an env variable itself?
Instead of:

$ node -r esm -r dotenv/config index.js dotenv_config_path=config/development.env

having:

$  DOTENV=config/development.env node -r esm -r dotenv/config index.js
@maxbeatty
Copy link
Contributor

What’s the real problem you’re trying to solve here?

@damianobarbati
Copy link
Author

@maxbeatty here the two scenarios I had trouble with and I couldn't come up with a solution.

  1. let's assume I want to convert a command like:
A=1 B=2 C=3 D=3 AND=so-on npx -n='-r esm' jest --no-cache --detectOpenHandles"

to dotenv.

  • the following breaks npx -n flag:
npx -n='-r esm -r dotenv/config dotenv_config_path=x.env' jest --no-cache --detectOpenHandles"
  • the following breaks jest because of unknown parameters
npx -n='-r esm -r dotenv/config' jest dotenv_config_path=x.env --no-cache --detectOpenHandles"
  • the following would work seamlessly!
DOTENV=x npx -n='-r esm -r dotenv/config' jest --no-cache --detectOpenHandles"

This apply to any executables: expecting dotenv_config_path=value to be the first parameter of whatever the developer is working with/on is very obtrusive, while expecting a given ENV to be set is not and developer can opt-in dotenv without friction.

  1. let assume I have a package script used as "base" for other scripts which have to load different environments. With an ENV a developer could easily do the following:
"run": "node -r dotenv/config my-script.js"
"run:dev": "DOTENV=dev.env yarn run myarg=1"
"run:stg": "DOTENV=stg.env yarn run myarg=2"
"run:prod": "DOTENV=prod.env yarn run myarg=3"

These are just my 2 cents on this, great package guys :)

@abcd-ca
Copy link

abcd-ca commented Nov 26, 2018

@maxbeatty regarding my pull request which I believe will also solve this issue, what more would you like to discuss?

@maxbeatty
Copy link
Contributor

So far, the reasons to support preload configuration using environment variables:

  1. Jest does not allow unknown CLI commands
  2. Webstorm IDE is hard to configure (could use more concise detail from Set dotenv_config_* vars via env var #354)
  3. Additional flexibility when chaining commands

Tools colliding and being incompatible is no fun so let’s try to make it better. What we’ll need:

  1. Documentation
    1. Environment variables can be uppercased version of CLI arguments (e.g. DOTENV_CONFIG_PATH)
    2. examples for configuring preloaded dotenv with only arguments, only environment variables, and a mixed approach
    3. explain that CLI arguments take precedence over env vars because they are more explicit and closer to command than an exported var might be (also prevents breaking change)
  2. Implementation w/ unit and integration tests. Set dotenv_config_* vars via env var #354 is a good start.
  3. Changelog and minor release (I can help with this)

@abcd-ca
Copy link

abcd-ca commented Dec 3, 2018

Hi @maxbeatty, thanks for considering this fix.
For #2 I wouldn't emphasize that WebStorm is hard to configure, rather that when you do configure it, it doesn't parse CLI args the way that bash does. Instead of node ignoring the unknown dotenv_config_path option and keeping it in argv where dotenv can later read it from argv, the IDE passes dotenv_config_path to node, forcing node to interpret it as a node module resulting in a node crash that complains that it can't find the module. In the first commit I tried to add this explanation to the readme. I'm happy to reword it if it's not clear enough, let me know.

Note, I considered requiring that the env var versions of the config vars be uppercase but when I dumped my env vars I noticed that a lot of node default env vars are actually lower case despite what I thought was the convention of using uppercase, so I thought I'd leave it lowercase. I also considered allowing the user to use upper OR lower case and having the parser interpret it case insensitive to make it a bit friendlier. Currently it expects lowercase. I can change the PR if you prefer uppercase or case insensitive.

@maxbeatty
Copy link
Contributor

Will you both test #355 by installing dotenv@next? If it works as expected, I'll release v6.2.0 officially

@maxbeatty
Copy link
Contributor

dotenv@6.2.0 has been released with this addition

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants