Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

refactor: Apply webpack-defaults & webpack 3.x support #540

Merged
merged 24 commits into from Jul 10, 2017

Conversation

joshwiens
Copy link
Member

@joshwiens joshwiens commented Jun 11, 2017

Working PR for the 3.x release branch

Beta & RC builds should be published from this branch to the @beta npm dist-tag

Squashed commit message body for merge and 3.0.0 release


- refactor: Pass a unique compiler name to get child compilation [483](https://github.com/webpack-contrib/extract-text-webpack-plugin/pull/483)
- refactor: Apply webpack-defaults [542](https://github.com/webpack-contrib/extract-text-webpack-plugin/pull/542)

BREAKING CHANGE: Enforces `engines` of `"node": ">=4.3.0 < 5.0.0 || >= 5.10`

- refactor: DeprecationWarning: Chunk.modules [543](https://github.com/webpack-contrib/extract-text-webpack-plugin/pull/543)

BREAKING CHANGE: Updates to `Chunk.mapModules`. This release is not backwards compatible with `Webpack 2.x` due to breaking changes in webpack/webpack#4764

- fix: css generation order issue see: webpack/webpack#5225

BREAKING CHANGE: Enforces `peerDependencies` of `"webpack": "^3.1.0"`. 

Closes #529
Closes #548

@joshwiens joshwiens changed the title refactor: Support Webpack 3.x (#483) feat: Support Webpack 3.x (#483) Jun 11, 2017
@joshwiens joshwiens changed the title feat: Support Webpack 3.x (#483) feat: Support Webpack 3.x Jun 11, 2017
@joshwiens joshwiens mentioned this pull request Jun 15, 2017
4 tasks
package.json Outdated
@@ -35,7 +35,7 @@
"should": "^11.1.2",
"standard-version": "^4.1.0",
"style-loader": "^0.18.2",
"webpack": "^2.6.1"
"webpack": "^3.0.0-rc.0"
Copy link
Member

Choose a reason for hiding this comment

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

^3.0.0

Copy link
Member Author

Choose a reason for hiding this comment

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

This is removed in #542

@michael-ciniawsky
Copy link
Member

@d3viant0ne Would this be ready as a MVP ?

@michael-ciniawsky michael-ciniawsky changed the title feat: Support Webpack 3.x breaking(package): update peerDependencies (webpack v2.0.0..3.0.0) Jun 20, 2017
@michael-ciniawsky michael-ciniawsky added this to the 3.0.0 milestone Jun 20, 2017
@joshwiens
Copy link
Member Author

joshwiens commented Jun 20, 2017

breaking is not a semver type, please leave the title of this PR alone. Breaking changes are handled via the body of the commit message which you can find in the bottom of the original post

@joshwiens joshwiens changed the title breaking(package): update peerDependencies (webpack v2.0.0..3.0.0) refactor: Apply webpack-defaults & webpack 3.x support Jun 20, 2017
@webpack-contrib webpack-contrib deleted a comment from codecov bot Jul 7, 2017
@codecov
Copy link

codecov bot commented Jul 7, 2017

Codecov Report

Merging #540 into master will decrease coverage by 2.72%.
The diff coverage is 87.41%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #540      +/-   ##
==========================================
- Coverage   90.14%   87.41%   -2.73%     
==========================================
  Files           4        7       +3     
  Lines         355      302      -53     
  Branches       75       68       -7     
==========================================
- Hits          320      264      -56     
- Misses         35       36       +1     
- Partials        0        2       +2
Impacted Files Coverage Δ
src/cjs.js 0% <0%> (ø)
src/lib/OrderUndefinedError.js 14.28% <14.28%> (ø)
src/lib/ExtractedModule.js 88.57% <88.57%> (ø)
src/loader.js 88.7% <88.7%> (ø)
src/index.js 89.7% <89.7%> (ø)
src/lib/helpers.js 89.74% <89.74%> (ø)
src/lib/ExtractTextPluginCompilation.js 90.9% <90.9%> (ø)
... and 4 more

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 dd43832...6b4e64c. Read the comment docs.

src/index.js Outdated
compilation.plugin('additional-assets', (callback) => {
extractedChunks.forEach((extractedChunk) => {
if (extractedChunk.getNumberOfModules()) {
extractedChunk.modules.sort((a, b) => {
Copy link
Member

Choose a reason for hiding this comment

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

extractedChunk.modules.sort => extractedChunk.sortModules ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Think we have the same idea, we'll know in a minute.

Copy link
Member

@michael-ciniawsky michael-ciniawsky left a comment

Choose a reason for hiding this comment

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

If it stays family friendly over the weekend g2g 😛

src/index.js Outdated
async.forEach(chunks, (chunk, callback) => { // eslint-disable-line no-shadow
const extractedChunk = extractedChunks[chunks.indexOf(chunk)];
const shouldExtract = !!(options.allChunks || isInitialOrHasNoParents(chunk));
chunk.sortModules(module);
Copy link

Choose a reason for hiding this comment

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

I'm seeing #565 in a webpack project at $WORK. I don't think you should be passing in module to sortModules (since it doesn't seem to be a comparator function) and making this change fixes the aforementioned problem for me.

Copy link
Member

Choose a reason for hiding this comment

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

Can you provide more info about your setup please? webpack.config.js etc ? Do you use karma somewhere (#554) ?

Choose a reason for hiding this comment

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

No, we don't use karma anywhere. I can try and scrub the config so I can post it, but it will be a bit of a pain.

This particular reference is to Node's current module object since there are no other references to module in this scope (from what I can tell anyway). Is the sort intended to be called during mapModules?

@joshwiens
Copy link
Member Author

For everyone watching, I'll be publishing what should be the final rc build shortly. Still planning on putting this on the dist-tag tomorrow evening.

@codecov-io
Copy link

codecov-io commented Jul 10, 2017

Codecov Report

Merging #540 into master will decrease coverage by 2.72%.
The diff coverage is 87.41%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #540      +/-   ##
==========================================
- Coverage   90.14%   87.41%   -2.73%     
==========================================
  Files           4        7       +3     
  Lines         355      302      -53     
  Branches       75       68       -7     
==========================================
- Hits          320      264      -56     
- Misses         35       36       +1     
- Partials        0        2       +2
Impacted Files Coverage Δ
src/cjs.js 0% <0%> (ø)
src/lib/OrderUndefinedError.js 14.28% <14.28%> (ø)
src/lib/ExtractedModule.js 88.57% <88.57%> (ø)
src/loader.js 88.7% <88.7%> (ø)
src/index.js 89.7% <89.7%> (ø)
src/lib/helpers.js 89.74% <89.74%> (ø)
src/lib/ExtractTextPluginCompilation.js 90.9% <90.9%> (ø)
... and 4 more

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 dd43832...6b4e64c. Read the comment docs.

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

Successfully merging this pull request may close these issues.

[webpack v3.0.0] generates CSS in wrong order feat: DeprecationWarning: Chunk.modules in Webpack 3.x
6 participants