-
-
Notifications
You must be signed in to change notification settings - Fork 601
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: added config command #4126
base: master
Are you sure you want to change the base?
Conversation
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.
Yes there are a lot of changes as I have completely deleted the built in commands --config and --config-name and introduced a whole new command in their place.
This is not the intend of the issue, what we want to achieve is a new --print-config
/--show-config
cli flag similar to other tools like jest which will output the whole webpack config on console.
Yes but in the issue itself you had asked to create a webpack config command which has 3 options: |
Hi @samrudh3125. Just a little hint from a friendly bot about the best practice when submitting pull requests:
You don't have to change it for this PR, just make sure to follow this hint the next time you submit a PR. |
No this is not the intent of the issue. Those are options for this command to print selective configurations and can be added later. |
Then can you please elaborate the issue once again so that I change it accordingly |
} | ||
|
||
const compiler = webpack(config.options as webpack.Configuration[]); | ||
compiler.run((err, stats) => { |
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.
We don't need to run this
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.
No it's running only in case of --file and --name for --inckude I will remove it
} | ||
|
||
if (options.include) { | ||
const includeOptions = webpack.config.getNormalizedWebpackOptions({}); |
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.
Also I think we need to apply defaults, so you will see how webpack sees your configuration
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.
Actually we need to comment the default values beside them which is getting very complex so shall I apply the defaults?
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.
You can get default value applyWebpackOptionsBaseDefaults
and applyWebpackOptionsDefaults
from:
const {
applyWebpackOptionsDefaults,
applyWebpackOptionsBaseDefaults
} = require("webpack/config/defaults");
How I see this logic, you load config file/files, normalize and them apply default.
Also we have two possible commands:
webpack config
just print full resolved configuration, i.e. after apply normalization and defaultwebpack config --diff
show only options which are different after normalize and default, so you can undestand which options you don't need because they are default
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.
Also please don't remove tests
My bad actually since I removed the command I deleted the tests |
@samrudh3125 Thanks for your update. I labeled the Pull Request so reviewers will review it again. @alexander-akait Please review the new changes. |
What kind of change does this PR introduce?
It introduces a new feature which is the config command which has three options i.e file, name and include.
Did you add tests for your changes?
I have added tests for name and file option but for include didn't add tests as at this stage it just prints the present configuration in the console.
Summary
The issue #3537 is solved in this PR. This was a very good issue to work on as i could understand the complete codebase of webpack as I am aiming at working in one of the projects of webpack in GSOC.
Does this PR introduce a breaking change?
Yes there are a lot of changes as I have completely deleted the built in commands --config and --config-name and introduced a whole new command in their place.
Other information