[perf] Set all:false in stats.toJson call #708
Conversation
|
🤯 , let me double-check the results and merge. Unfortunately, there are no real automated tests around stats json generation. |
packages/webpack-plugin/src/index.js
Outdated
| assets: true, | ||
| chunks: true, | ||
| chunkGroups: true, | ||
| errorDetails: false, |
There was a problem hiding this comment.
Is there a reason to keep these false options provided? Or does all: false set that as a default for anything not explicitly provided?
There was a problem hiding this comment.
looking at https://github.com/webpack/webpack/blob/afc9b2fcf9bef0831640b3ebb02b73068ba18e17/lib/stats/DefaultStatsPresetPlugin.js#L132 and https://webpack.js.org/configuration/stats/#statsall, it doesn't seem so since all: false will basically default anything not set to false.
i opted for leaving these as-is since i was trying to make a minimal change here but happy to remove the unnecessary false options here
There was a problem hiding this comment.
IMO it'd be clearer to just totally omit anything that isn't true, since the all: false covers all that. But I'll defer to the repo maintainers, I'm just taking a look since this PR will solve my issue too (#709) 😄
| source: false, | ||
| errorDetails: false, | ||
| timings: false, | ||
| chunkGroups: true, |
There was a problem hiding this comment.
Updated to:
- remove the falsey options (not needed now that we're specifying all: false)
- added
chunkGroupChildren: trueandids: true. These are necessary for webpack 5 otherwise and will fail
|
Looking good to me, but I'm not a maintainer - would love to see this merged to close #709 :) |
|
@theKashey hey just wanted to check in and see if there was anything else I can/should do to help get this merged. Thanks! |
|
Just waiting for the Easter break to go through all changes |
Summary
I noticed while upgrading a codebase from webpack v4 -> v5 that there seemed to be a significant increase in incremental build times.
From a local profile, the time seemed to be spent on a
stats.toJsoncall originating from the loadable webpack pluginUpdating the
toJsoncall so that it specifies all: false improves the time spent on this call from 40seconds -> 55ms (for the specific codebase i tested on)I checked this change against webpack 4 on our codebase and it also reduced the time spent from ~9s -> 500ms
Test plan
I tested this change locally against both a webpack 4 build and webpack 5 build and loadable worked as expected in both cases.