fix: don't override dot envs with site envs #1490
Conversation
|
|
||
| execa(this.argv[0], this.argv.slice(1), { | ||
| stdio: 'inherit', | ||
| env: fromEntries(envSettings.vars), |
There was a problem hiding this comment.
These are applied globally now (which is not nice, but would require more refactoring as we log each env that is applied in a different way)
There was a problem hiding this comment.
Would it be possible to keep those envSettings as a variable, instead of modifying process.env?
There was a problem hiding this comment.
addEnvVariables modifies process.env but also prints log messages while doing so:
https://github.com/netlify/cli/pull/1490/files#diff-fc861c54030ca23b6d43638aa92f3f446bca1b187ce3b3fea5b8e6d7d1c0a19cR98
We can clone process.env in addEnvVariables, use the clone instead of process.env and return it to dev:exec.
That would work for dev:exec but we would still need to modify process.env in other places due to the way netlify dev and netlify functions:create commands work at the moment.
I would rather be consistent in how we're applying env variables and handle mutating process.env (in all locations we're doing it) in another PR. WDYT?
There was a problem hiding this comment.
This makes sense 👍
Should we create an issue for it?
erezrokah
force-pushed
the
refactor/cleanup_add_envs
branch
from
c93d29b to
1c41dc4
Compare
- Summary
Fixes #1386
Related to #1331
Also fixed some stuff related to addons since during my manual testing I noticed it was broken.
- Test plan
Fixed some tests.
I still need to think about adding new tests, as testing #1386 will require creating a live site and then running
netlify devornetlify dev:exec.- Description for the changelog
Refactor environment variables handling.
- A picture of a cute animal (not mandatory but encouraged)
