-
-
Notifications
You must be signed in to change notification settings - Fork 596
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
--define-process-env-node-env
doesn't do as advertised
#3503
Comments
Why don't use
|
Oh, our mistake https://github.com/webpack/webpack-cli/blob/master/packages/webpack-cli/src/webpack-cli.ts#L2179 @snitin315 I think we need to decide better strategy here, make sense to have ability set |
The current (and previous) behavior (of setting |
@snitin315 I think yes, it was my mistake (I forgot about we set |
I'll raise a PR. |
Thank you all! Much appreciate your work. 😃 |
@snitin315, would I be able to help out in any way? Maybe submitting a PR to get started with or something? |
@CreativeTechGuy yes, feel free to send a PR. |
Describe the bug
This is a follow up from this comment thread.
--node-env
was renamed inwebpack-cli
v5 with this comment linked as the reasoning.This is not true, not in the previous version and not in the new version. See "To Reproduce" section below for a very minimal demo proving this isn't true.
What is the current behavior?
process.env.NODE_ENV
is set to the value of--node-env
/--define-process-env-node-env
within bothwebpack.config.js
and the rest of the application.To Reproduce
These are the only two files needed to reproduce. Install the dependencies and run either command and see the value logged.
Gist: https://gist.github.com/CreativeTechGuy/ae6ec05ee9ec5cb71807f2ff74a99c41
Expected behavior
There's a conflict.
--define-process-env-node-env
is named as such to (attempt to) make it clear that it doesn't set the environment variable withinwebpack.config.js
. That precondition doesn't seem to be true. So the previous name--node-env
is much simpler and easier to understand and does do what you'd expect it to do.The text was updated successfully, but these errors were encountered: