shellscape / webpack-manifest-plugin

webpack plugin for generating asset manifests
MIT License
1.44k stars 185 forks source link

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

Closed Lyrkan closed 4 years ago

Lyrkan commented 4 years ago

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 https://github.com/webpack/webpack/pull/9793#pullrequestreview-300371456).

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

codecov-io commented 4 years ago

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% <ø> (ø) :arrow_up:

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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

That's great, thank you