webpack-cli
webpack-cli copied to clipboard
webpack
feat: built in env files support
What kind of change does this PR introduce?
feature
Adds built in support to webpack to handle .env files
Did you add tests for your changes? not yet
If relevant, did you update the documentation? not yet
Summary
Closes https://github.com/webpack/webpack-cli/issues/3747
Does this PR introduce a breaking change?
No
Other information
Creating a new folder under packages. packages/environment-variables-webpack-plugin. Does this name work?
@alexander-akait if we do want to distribute it as a standalone plugin in the future, isnt it better to add it to the webpack codebase or put it under webpack-contrib org? creating it as a plugin here and then moving it to a different repo to distribute it additional unnecessary work if we want this to be an inbuilt feature in webpack then lets add it to webpack repo or if its going to be an installable plugin then lets create a new repo under the contrib org?
@burhanuday That's right, we can do it this way, but I'd rather keep it here for better integration with CLI (i.e. allows to have special flags for this) and fast fixes and fast releases, not sure it really make sense to use it as standalone plugin (except complex cases)
The main goal is a seamless integraiton, moving it and rename is not long task, implementation is the most long task
Codecov Report
Attention: Patch coverage is 99.04762% with 1 line in your changes missing coverage. Please review.
Project coverage is 91.77%. Comparing base (
78f57d1) to head (ccf341e). Report is 298 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #3759 +/- ##
==========================================
+ Coverage 91.64% 91.77% +0.13%
==========================================
Files 22 23 +1
Lines 1628 1776 +148
Branches 466 505 +39
==========================================
+ Hits 1492 1630 +138
- Misses 136 146 +10
| Files | Coverage Δ | |
|---|---|---|
| packages/dotenv-webpack-plugin/src/index.js | 100.00% <100.00%> (ø) |
|
| packages/webpack-cli/src/webpack-cli.ts | 92.54% <95.00%> (-0.51%) |
:arrow_down: |
... and 5 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 78f57d1...ccf341e. Read the comment docs.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
since env param in the cli is already taken, what should we use for passing array of env file names in the cli?
does env-file work?
Let's implement --dot-env flag to enable it, maybe we will enable it in the next major release, but now it can break a logic, so we need to move it under the option
Regarding to --env, good questions, let's add TODO for this, because I am not sure
since env param in the cli is already taken, what should we use for passing array of env file names in the cli? does env-file work?
@burhanuday I didn't get this, do you mean in case the user wants to read variables from any other env file for example .env.my-custom-file?
since env param in the cli is already taken, what should we use for passing array of env file names in the cli? does env-file work?
@burhanuday I didn't get this, do you mean in case the user wants to read variables from any other env file for example
.env.my-custom-file?
Yes. But i think since we are publishing the plugin anyway, this can be passed as an option to the plugin and we dont need to take this via the CLI
@alexander-akait a PR with failing tests was merged into master. so tests are also failing on this PR - https://github.com/webpack/webpack-cli/pull/3783
- let's add test case for
const { DB_HOST, DB_PASS } = process.env;(we supports it in webpack, just to check it), same forimport.meta
@alexander-akait
In this plugin we are defining the variables using string like "process.env.varname":"value" and not the object syntax. Object syntax would be required for the above test case to pass.
While figuring this out, i was reading the webpack docs for define plugin and saw that this is recommended against doing in a note on this link - https://webpack.js.org/plugins/define-plugin/#usage
Other than that, i have implemented allowEmptyValues and set default value to false. also added tests for it as well as docs
@burhanuday yeah, sorry for delay, on my TODOs list on the top after https://github.com/webpack/webpack-cli/pull/3739,
https://github.com/web-infra-dev/rspack/issues/2208 this is also the requested features from rspack users, glad to see webpack-cli is willing to support it
@hardfist Yeah, it will be an official part of webpack ecosystem, I want to be align with next.js logic, it is flexible and solves almost all requests, hope we will finish it soon (this week)
@burhanuday I think also we need to check how the new node --env-file option could be used alongside this. Wil review your PR soon! :)
Х̶о̶р̶о̶ш̶о̶ с̶ н̶е̶т̶е̶р̶п̶е̶н̶и̶е̶м̶ ж̶д̶у̶
ср, 22 нояб. 2023 г., 03:04 Even Stensberg @.***>:
@burhanuday https://github.com/burhanuday I think also we need to check how the new node --env-file option could be used alongside this. Wil review your PR soon! :)
— Reply to this email directly, view it on GitHub https://github.com/webpack/webpack-cli/pull/3759#issuecomment-1821601486, or unsubscribe https://github.com/notifications/unsubscribe-auth/BBO6JJWAWFJIWT3AXTUV5QTYFUCOXAVCNFSM6AAAAAAXNRXH5KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMRRGYYDCNBYGY . You are receiving this because you are subscribed to this thread.Message ID: @.***>
@ Thanks for your update.
I labeled the Pull Request so reviewers will review it again.
@snitin315 Please review the new changes.
