-
-
Notifications
You must be signed in to change notification settings - Fork 490
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: Allow flags to support the webpack-cli #628
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.
And now let's use them here - https://github.com/webpack-contrib/webpack-bundle-analyzer/blob/master/src/bin/analyzer.js#L27
there is an API - https://github.com/webpack/webpack/blob/main/lib/cli.js#L612
So:
- you
require
options from this file - then use
processArguments
(don't forget to handleProblem
) - then use
.option(...)
to create all options
on it |
What are flags? |
src/bin/analyzer.js
Outdated
const {resolve, dirname} = require('path'); | ||
const { resolve, dirname } = require("path"); |
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 do we need to have so much formatting changes here?
package-lock.json
Outdated
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 need to change this big amount of packages that GitHub even has trouble visualizing the changes in package-lock.json
?
@valscion We need these flags to support webpack CLI - webpack/webpack-cli#1838, so you can use |
Thanks for the background information! It would indeed be valuable to allow this tool to be used easily through webpack CLI. In order for me to eventually be able to review this PR, I'd need to see the least amount of changes possible. That includes:
And then we'd need these:
Also, I'd like to know whether this change will indeed still be backwards-compatible (that is, releasing this would be a minor version release and not a major release). If there is no other way than doing a major release then I'm also OK with that. All in all, I do think that this feature would make sense — but I also have some wishes as the maintainer of this project before I can accept the changes. |
@valscion Yeah, I will ping you when it will be ready to review |
Sounds nice! Thanks! |
@alexander-akait shall i proceed to write the tests ? |
@@ -12,6 +12,11 @@ _Note: Gaps between patch versions are faulty, broken or test releases._ | |||
|
|||
## UNRELEASED | |||
|
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.
uncommit
@@ -1,6 +1,6 @@ | |||
{ | |||
"name": "webpack-bundle-analyzer", | |||
"version": "4.10.2", | |||
"version": "4.10.3", |
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 new version
Context
The webpack-bundle-analyzer has various options such as host which are accessible via flags with reference to webpack-cli#1838. In this PR we would be trying to support these flags in the webpack-cli, like below :
webpack build --analyze --analyze-no-open