-
-
Notifications
You must be signed in to change notification settings - Fork 603
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
refactor!: the minimum supported webpack version is v5.0.0 #3342
Conversation
Codecov Report
@@ Coverage Diff @@
## next #3342 +/- ##
==========================================
- Coverage 93.06% 92.92% -0.14%
==========================================
Files 22 22
Lines 1686 1669 -17
Branches 491 479 -12
==========================================
- Hits 1569 1551 -18
- Misses 117 118 +1
Continue to review full report at Codecov.
|
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.
Let's separete this PR on parts to avoid regressions:
- Remove old options
- Remove workaround for webpack v4
Or verca vice
@alexander-akait I've created #3346 to remove workarounds, I'll update this PR to remove old options. |
47f5a03
to
52f2913
Compare
BREAKING CHANGE: webpack-cli no longer supports webpck v4, the minimum supported version is webpack v5.0.0
52f2913
to
63a9f59
Compare
63a9f59
to
9d1f8c1
Compare
@alexander-akait Done ✅ |
@@ -239,15 +239,17 @@ describe("core flags", () => { | |||
expect(stdout).toContain(`devtool: 'source-map'`); | |||
}); | |||
|
|||
it("should allow string value devtool option using alias", async () => { | |||
// TODO: Enable alias with webpack 5 | |||
it.skip("should allow string value devtool option using alias", async () => { |
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 skip?
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 have alias property coming from webpack.
We had defined alias for only few config options which we had for webpack 4 which we removed in this PR.
--no-target Negative 'target' option. | ||
-w, --watch Watch for files changes. |
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.
I think keeping short -w
/-d
and etc make sense, they are very popular, let's keep them, what do you think?
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, we can add alias
support for some popular flags devtool (-d)
, target (-t)
, --output-path (-o)
, --watch (-w)
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.
Ideally, we should add this in the webpack schema itself.
Or we can write logic to generate an alias property for each option in webpack-cli.
Or add some workaround logic to support alias for some popular options only (coming from webpack core).
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.
Or we can write logic to generate an alias property for each option in webpack-cli.
I think so, it was my original idea, so we will support all options using short names, we can do it separatly
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. Let's add them separately. In the meantime should I update the PR to keep the popular ones?
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, let's keep popular options here, some of them we can kept for better compatibility
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.
@alexander-akait Added in 00fae2f
9d2973d
to
dc5dc9b
Compare
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.
Looks good
BREAKING CHANGE: webpack-cli no longer supports webpack v4, the minimum supported version is webpack v5.0.0
BREAKING CHANGE: webpack-cli no longer supports webpack v4, the minimum supported version is webpack v5.0.0
BREAKING CHANGE: webpack-cli no longer supports webpack v4, the minimum supported version is webpack v5.0.0
BREAKING CHANGE: webpack-cli no longer supports webpack v4, the minimum supported version is webpack v5.0.0
BREAKING CHANGE: webpack-cli no longer supports webpack v4, the minimum supported version is webpack v5.0.0
BREAKING CHANGE: webpack-cli no longer supports webpack v4, the minimum supported version is webpack v5.0.0
BREAKING CHANGE: webpack-cli no longer supports webpack v4, the minimum supported version is webpack v5.0.0
BREAKING CHANGE: webpack-cli no longer supports webpack v4, the minimum supported version is webpack v5.0.0
BREAKING CHANGE: webpack-cli no longer supports webpack v4, the minimum supported version is webpack v5.0.0
What kind of change does this PR introduce?
Breaking change.
Did you add tests for your changes?
Already present.
If relevant, did you update the documentation?
No
Summary
The minimum supported webpack version is
v5.0.0
.Does this PR introduce a breaking change?
BREAKING CHANGE:
--target-reset
,--entry-reset
flags are required to be used before using original flags like--target
,--entry
.Other information
No