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

Webpack 5: Add "ids" to the retrieved JSON stats #200

Merged
merged 5 commits into from
Oct 24, 2019

Conversation

Lyrkan
Copy link

@Lyrkan Lyrkan commented Oct 10, 2019

This PR adds the ids objects to the stats retrieved by stats.toJson().

It is needed by the latest alpha of Webpack 5 (v5.0.0-alpha.31) in order to retrieve asset.chunks later on (see webpack/webpack#9793 (review)).

Without it, the following error is thrown by the plugin:

TypeError: Cannot read property 'length' of undefined
    at (...)/node_modules/webpack-manifest-plugin/lib/plugin.js:128:39

Related: #186

ian-craig and others added 2 commits October 2, 2019 17:56
)

* Store custom hook in WeakMap for Webpack 5 compatability

* Make unit tests run on Windows too

* Expose custom hooks in Webpack 4+ through static method, update tests for Webpack 5

* Revert webpack and extract-text-webpack-plugin versions

* Exclude node 6 + webpack 5 from travis matrix as they aren't compatible

* Fix travis exclude rule to be exact match

* Re-standardize file paths after opts.map is run

* Exclude manifest in compilation assets UT from webpack 5 as feature isn't available
@codecov-io
Copy link

codecov-io commented Oct 10, 2019

Codecov Report

Merging #200 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #200   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           4      4           
  Lines         129    129           
  Branches       26     26           
=====================================
  Hits          129    129
Impacted Files Coverage Δ
lib/plugin.js 100% <ø> (ø) ⬆️

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 5715c07...6adbe39. Read the comment docs.

@Lyrkan
Copy link
Author

Lyrkan commented Oct 17, 2019

@mastilver Should I add webpack@next to the peer dependencies and tests? I noticed that you also changed them in #197, so maybe I should target that PR's branch instead?

@mastilver
Copy link
Contributor

@Lyrkan Yes, I believe your PR should target next
Unfortunately I'm having some issue with the tests on that branch and until this is resolved I'm blocked

@Lyrkan Lyrkan changed the base branch from master to next October 18, 2019 19:29
@Lyrkan
Copy link
Author

Lyrkan commented Oct 18, 2019

@mastilver Changed the target branch to next and tried to fix the tests after merging master to get the Webpack 5 fix that was commited on it (hence the additional commits and comment on this PR).

I also added back webpack@next to the CI matrix.

Hope I didn't forget anything :)

@mastilver mastilver merged commit def5778 into shellscape:next Oct 24, 2019
@mastilver
Copy link
Contributor

That's great, thank you

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

Successfully merging this pull request may close these issues.

5 participants