Skip to content
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

fix: make define-process-env-node-env alias node-env #3514

Merged
merged 4 commits into from
Dec 4, 2022
Merged

fix: make define-process-env-node-env alias node-env #3514

merged 4 commits into from
Dec 4, 2022

Conversation

CreativeTechGuy
Copy link
Contributor

@CreativeTechGuy CreativeTechGuy commented Dec 2, 2022

What kind of change does this PR introduce?

Restores v4's --node-env CLI flag while preserving the new --define-process-env-node-env flag.

Did you add tests for your changes? Yup

If relevant, did you update the documentation? Yup (although depending on what the desired future is for these flags, possibly the web documentation needs to be updated too).

Summary

Fix #3503. Since the full fix discussed in that thread would be breaking, the interim solution is to restore the --node-env flag and have both --define-process-env-node-env and --node-env do the same thing. In the future these may diverge as discussed here: #3503 (comment)

Does this PR introduce a breaking change?

No. This was very intentionally done in this way to not create a breaking change. It seems unclear as to what the desired long-term behavior is so for now there are two alias commands.

Other information

N/A

@CreativeTechGuy CreativeTechGuy requested a review from a team as a code owner December 2, 2022 05:47
@codecov
Copy link

codecov bot commented Dec 2, 2022

Codecov Report

Merging #3514 (30dba1f) into master (3ec7b16) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3514      +/-   ##
==========================================
+ Coverage   91.36%   91.37%   +0.01%     
==========================================
  Files          22       22              
  Lines        1586     1588       +2     
  Branches      446      447       +1     
==========================================
+ Hits         1449     1451       +2     
  Misses        137      137              
Impacted Files Coverage Δ
...s/generators/init-template/default/package.json.js 100.00% <ø> (ø)
packages/webpack-cli/src/webpack-cli.ts 92.71% <100.00%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ec7b16...30dba1f. Read the comment docs.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/cc @webpack/cli-team Please review too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--define-process-env-node-env doesn't do as advertised
5 participants