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

Expand only after all env configs are loaded #101

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bhunjadi
Copy link

@bhunjadi bhunjadi commented Jan 18, 2024

Consider this example with two env files, .local.env and .env:

node cli.js -e .local.env -e .env -- printenv

where the files are like this:

# .local.env
TEST_PASS=some\$pass

# .env
TEST_PASS=

What happens with the things as they're implemented now:

  1. First pass: .local.env loaded and expanded, we have process.env.TEST_PASS == 'some$pass' now
  2. Second pass: .env loaded and expanded, but now dotenv-expand loads from process.env, so TEST_PASS='some$pass' is passed to expand(). Consequently, after expansion TEST_PASS == 'some'. $pass variable doesn't exist, so it is replaced with an empty string.

Proposed solution in this PR

Just do expand after all the config files are loaded.

@entropitor
Copy link
Owner

I think some users might have some env variables were they do want intermediate expansion (so they can e.g. set a PORT in one file and construct an URL from that port in another file).

Maybe to avoid breaking changes, we should have a different flag? --expand-at-the-end-only or something?

@bhunjadi
Copy link
Author

bhunjadi commented Mar 8, 2024

Would the use case be something like this?

# .local.env
URL=http://localhost:${PORT}/api/v1

# .env
PORT=3344

Then you do

node cli.js -e .local.env -e .env -- printenv

And I'm still getting expected results (I think at least):

URL=http://localhost:3344/api/v1
PORT=3344

@entropitor
Copy link
Owner

Yes, but reversed, values from the first file, can be used in values from later files. I'm not sure if anybody relies on this, but this is something that would break so I don't think it's something we can change

# .local.env
PORT=3344

# .env
URL=http://localhost:${PORT}/api/v1

and then running

node cli.js -e .local.env -e .env -- printenv

@Swivelgames
Copy link
Contributor

@entropitor I would have to agree. I would expect the first file to contribute to the second, and for the second to override the first.

@bhunjadi
Copy link
Author

I did one test.

The command I'm executing is:

node cli.js -e .local.env -e .env  -- printenv | grep TEST_

File contents

.env.local

# This overrides TEST_PASS from .env
TEST_PASS=some\$pass

# This will use TEST_PORT_REVERSE from .env
TEST_URL_REVERSE=http://localhost:${TEST_PORT_REVERSE}/api/v1

# I want this to contribute to TEST_URL in .env
TEST_PORT=3000

.env

TEST_PASS=
TEST_URL=http://localhost:${TEST_PORT}/api/v1

TEST_PORT_REVERSE=4000
# Default value for port if no other specifies it
TEST_PORT=5000

Results

node cli.js -e .local.env -e .env  -- printenv | grep TEST_
TEST_PASS=some$pass
TEST_URL_REVERSE=http://localhost:5000/api/v1
TEST_PORT=3000
TEST_PORT_REVERSE=5000
TEST_URL=http://localhost:3000/api/v1
# w/o expand
node cli.js -e .local.env -e .env --no-expand -- printenv | grep TEST_
TEST_PASS=some\$pass
TEST_URL_REVERSE=http://localhost:${TEST_PORT_REVERSE}/api/v1
TEST_PORT=3000
TEST_URL=http://localhost:${TEST_PORT}/api/v1
TEST_PORT_REVERSE=4000

Results before

node cli.js -e .local.env -e .env  -- printenv | grep TEST_
TEST_PASS=some        # this is incorrect
TEST_URL_REVERSE=http://localhost:/api/v1   # not working, but why not support it?
TEST_PORT=3000
TEST_URL=http://localhost:3000/api/v1
TEST_PORT_REVERSE=4000

To comment on @Swivelgames

I would expect the first file to contribute to the second

This is satisfied.

for the second to override the first

This is not how it works and wasn't even before this change, at least how I understand it.
If I have TEST_PASS in first and TEST_PASS in second, TEST_PASS from first will override TEST_PASS in second.

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 this pull request may close these issues.

None yet

3 participants