Iterate on .env file loading
#10093
Merged
Iterate on .env file loading
#10093
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jtoar
added
release:fix
jtoar
added a commit
that referenced
this pull request
Mar 2, 2024
Follow up to #9961. Got some feedback from @cannikin and @Josh-Walker-GM. There are still some things I need to clarify, but the changes here are improvements, and we'll keep iterating before this goes out next week (currently in the v7.1 RC): - the name was shortened from `--include-env-files` to `--add-env-files` - `.env` files that are loaded based on `NODE_ENV` override instead of add - the logic was moved up from middleware because env loading should be done as soon as possible, and the earliest is right after we set the cwd I'll bring up the override vs at the next meeting. Just to document what currently happens to completeness, env files are loaded in two steps... #### Layer 1 - `.env.defaults` - `.env` - `.env.${NODE_ENV}` These are loaded in an overriding manner, so that if there are conflicts, `.env.${NODE_ENV}` has the final say. This can't be disabled. #### Layer 2 If users want, they can add more via `--add-env-file`, like `--add-env-file stripe`. The env vars loaded from files specified this way are additive—they won't override anything already set in the first layer. I'll see about moving this functionality out of the CLI package and into CLI helpers so its shareable in another PR.
|
Is it --add-env-file or --add-env-files? The PR description har both versions EDIT: At my computer now and checked the actual code. It's with an "s" at the end, |
jtoar
added a commit
that referenced
this pull request
Mar 2, 2024
Follow up to #10093. After discussing with @orta, the `--add-env-files` flag being additive instead of overriding wasn't a sticking point in the original implementation, and the more feedback I get, it seems like most people expect later files to override earlier ones, so let's switch the behavior. In tandem, at first I tried to rename the flag back to just `--env-file` or `--env-files`, but I'm afraid both give this cryptic error: ``` redwood-app % yarn rw --env-file prod node: prod: not found # Or `yarn rw --env-files prod`, which also gives this error, # which I didn't know node would also process? Their docs only say `--env-file`... ``` The above was with my framework changes. I tried it again without my framework changes and the error persists. As far as I can tell, `rw` never executes and it seems like the Node.js binary itself is evaluating the `--env-file` flag. It supports `--env-file` now, and expects it to be a full path (so `.env.prod`). But this isn't how I thought node processes `process.argv` at all... I thought that all flags would've been passed to the script (here, `rw`) for processing. But maybe not, and if so that means we can just pass options to node via `yarn rw --my-node-option`. Which I still don't quite believe and if it is true, then I'm not sure what to make of yet cause people have been wanting to pass node options for a while and I always thought `NODE_OPTIONS="--my-node-option" yarn rw ...` was the only way. So TL;DR, that's why I've left the name here and I'd like to keep that friction point out of the scope of this PR being considered mergeable.
jtoar
added a commit
that referenced
this pull request
Mar 2, 2024
Follow up to #10093. After discussing with @orta, the `--add-env-files` flag being additive instead of overriding wasn't a sticking point in the original implementation, and the more feedback I get, it seems like most people expect later files to override earlier ones, so let's switch the behavior. In tandem, at first I tried to rename the flag back to just `--env-file` or `--env-files`, but I'm afraid both give this cryptic error: ``` redwood-app % yarn rw --env-file prod node: prod: not found # Or `yarn rw --env-files prod`, which also gives this error, # which I didn't know node would also process? Their docs only say `--env-file`... ``` The above was with my framework changes. I tried it again without my framework changes and the error persists. As far as I can tell, `rw` never executes and it seems like the Node.js binary itself is evaluating the `--env-file` flag. It supports `--env-file` now, and expects it to be a full path (so `.env.prod`). But this isn't how I thought node processes `process.argv` at all... I thought that all flags would've been passed to the script (here, `rw`) for processing. But maybe not, and if so that means we can just pass options to node via `yarn rw --my-node-option`. Which I still don't quite believe and if it is true, then I'm not sure what to make of yet cause people have been wanting to pass node options for a while and I always thought `NODE_OPTIONS="--my-node-option" yarn rw ...` was the only way. So TL;DR, that's why I've left the name here and I'd like to keep that friction point out of the scope of this PR being considered mergeable.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow up to #9961. Got some feedback from @cannikin and @Josh-Walker-GM. There are still some things I need to clarify, but the changes here are improvements, and we'll keep iterating before this goes out next week (currently in the v7.1 RC):
--include-env-filesto--add-env-files.envfiles that are loaded based onNODE_ENVoverride instead of addI'll bring up the override vs at the next meeting.
Just to document what currently happens to completeness, env files are loaded in two steps...
Layer 1
.env.defaults.env.env.${NODE_ENV}These are loaded in an overriding manner, so that if there are conflicts,
.env.${NODE_ENV}has the final say. This can't be disabled.Layer 2
If users want, they can add more via
--add-env-files, like--add-env-files stripe. The env vars loaded from files specified this way are additive—they won't override anything already set in the first layer.I'll see about moving this functionality out of the CLI package and into CLI helpers so its shareable in another PR.