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

Feature: Migrate should move CommonsChunkPlugin to SplitChunksPlugin #393

Closed
ematipico opened this issue Apr 10, 2018 · 8 comments
Closed

Comments

@ematipico
Copy link
Contributor

Do you want to request a feature or report a bug?
Feature.

What is the current behavior?
At the moment migrate does nothing when it encounters CommonsChunkPlugin

What is the expected behavior?
It should migrate to SplitChunksPlugin

If this is a feature request, what is motivation or use case for changing the behavior?
CommonsChunkPlugin is now deprecated in webpack v4 and migrate, as its objective, should opt in in favor of SplitChunksPlugin

@ematipico ematipico added Semver: minor ⚙️ When delivering new features that don't break Help Wanted Feature Migrate labels Apr 10, 2018
@dhruvdutt
Copy link
Member

dhruvdutt commented Apr 10, 2018

I can pick this up. ✋

I think apart from changing name, there are some defaults as well that CommonsChunkPlugin was doing.

WDYT? @ematipico @montogeek

@ematipico
Copy link
Contributor Author

Great! That was quick

@evenstensberg evenstensberg added this to the Vatican (v3) milestone Apr 10, 2018
@ematipico ematipico removed the Semver: minor ⚙️ When delivering new features that don't break label Apr 10, 2018
@ematipico
Copy link
Contributor Author

No, just changing the name is not enough. It's a complete different section of the configuration

https://webpack.js.org/plugins/split-chunks-plugin/#optimization-splitchunks

@montogeek
Copy link
Member

@dhruvdutt First change is its name, second step is play a lot with all its options until it fits CCP behaviour, if you discover something new share it on webpack.js.org docs!

@matheus1lva
Copy link
Member

@ematipico this was already merged #399 , we can close

@ematipico
Copy link
Contributor Author

Are we sure? I am looking better at the PR, the changes were around different plugins.
Here the work is actually different because we have to migrate and existing CommonsChunkPlugin (I don't see any test around it).

@dhruvdutt was going to do that.

@ematipico ematipico reopened this Apr 23, 2018
@evenstensberg
Copy link
Member

Closed this by accident, a migration for commonsChunk would need to be added, as it has changed in newer versions of webpack. Nice catch, thanks!

@dhruvdutt

This comment has been minimized.

@dhruvdutt dhruvdutt removed their assignment May 8, 2018
@evenstensberg evenstensberg changed the title [Feature] Migrate should move CommonsChunkPlugin to SplitChunksPlugin Feature: Migrate should move CommonsChunkPlugin to SplitChunksPlugin May 18, 2018
@dhruvdutt dhruvdutt self-assigned this Jul 19, 2018
dhruvdutt added a commit to dhruvdutt/webpack-cli that referenced this issue Jul 31, 2018
dhruvdutt added a commit to dhruvdutt/webpack-cli that referenced this issue Jul 31, 2018
dhruvdutt added a commit to dhruvdutt/webpack-cli that referenced this issue Jul 31, 2018
dhruvdutt added a commit to dhruvdutt/webpack-cli that referenced this issue Aug 1, 2018
dhruvdutt added a commit to dhruvdutt/webpack-cli that referenced this issue Aug 1, 2018
dhruvdutt added a commit to dhruvdutt/webpack-cli that referenced this issue Aug 1, 2018
dhruvdutt added a commit to dhruvdutt/webpack-cli that referenced this issue Aug 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants