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.
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
feat: built in env files support #3759
feat: built in env files support #3759
Changes from 3 commits
abe80f0
f5bcec5
011627e
bb735ec
5a1b4db
95ca34e
08d16ff
e3a23ce
e2cf250
ca5a782
fd6d2ba
d544597
b83ccaf
a4388d7
ab00786
4e039ba
3790945
c3d97d7
1aef466
bd6c4a6
74ad4c8
afcbf8c
f203e8b
9286cb5
8689a2e
ca6fc8d
75ebe62
4388c69
7ab13cf
bf8cb58
ff7246f
9d7e1db
8a45c8b
7401474
f4b8835
9ee072d
f7f79b8
88d3660
0158dca
f8a6142
a165f38
06399c5
a0534f5
7f8c4c7
e00469c
d89d73c
0c477b0
8911ba7
00fb0b8
cdeb09f
07dbcd5
99a7163
03d285c
ac666c4
7de5892
07ac04e
a3e81fa
8e8b78c
c191cb2
67d8aed
a29fc1b
ca7a264
8a98a05
03211fe
1cb9309
26d136f
cdd9bde
8acb20c
1388d5e
81edc4c
55b64a1
c4ec39b
f0e32aa
6681233
8d92b67
ccf341e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really
Object.assign
? If we will not modifythis.config
we don't need to do extra clone, it spends CPU and memory.Also let's rename to
this.options
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since this plugin needs to work standalone as well, we take input from user which we merge with inbuilt config using
Object.assign
renamed variable to
options
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each file should be added to buildDependecies, otherwise cache will be broken
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done. all paths added to
buildDependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why only
WEBPACK_
, we need to load all ENV variablesThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if any variable with sensitive information gets added to any env file, it will be shipped with the client bundle if the env var is used. having
WEBPACK_
at the start of the variable makes sure that consumers are aware that this variable will be added to the build.I've seen this pattern implemented in other bundlers and Create React App, and hence, ported it here. Can remove if not required
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It means it should be configurable, some application can be written with env without
WEBPACK_
prefix and we can't force them to useWEBPACK_
prefixThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also we need this logic https://github.com/mrsteele/dotenv-webpack/blob/master/src/index.js#L62
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from what i understand, that plugin implements a "blueprint" feature. We have not implemented it. Although i dont find it entirely necessary, I am open to suggestions on why this is required
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it allows to catch problems on startup