-
-
Notifications
You must be signed in to change notification settings - Fork 380
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
Prune loadable stats file more aggressively #709
Comments
Hey @jdb8 👋, |
Looking at this again, I'm actually confused how things are working: loadable seems to rely on Although it looks like So I think my proposal would be to update the call to be: const stats = hookCompiler.getStats().toJson({
// by default, include nothing
all: false,
// set the stats properties loadable needs explicitly
hash: true,
publicPath: true,
outputPath: true,
assets: true,
chunks: true,
chunkGroups: true,
}); This way, if new stats properties get added in future versions of webpack, they're not included unnecessarily. It also makes it clearer which items are actually used by loadable during SSR. |
There is a fresh PR with exactly this change - #708 |
Long story short:
|
Oh great! I should have checked open PRs before making this issue.
The linked PR isn't doing that though, right? Is the plan to gate that change of behaviour behind a new flag? The fact that loadable already omits some data that would otherwise be present in stats.json means that anyone relying on loadable-stats.json to be equivalent is likely already experiencing bugs 😬
Agreed - it would also mean that you're not tied to the representation that webpack uses, which is more flexible for future changes.
EDIT: just realised this isn't your PR, sorry! I'll comment on there instead. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
I believe this is now fixed as of #708 and #735, right @theKashey? |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
🐌 @jdb8 - seems so. However now we are going to extend information a little bit, as well as there is an idea to allow controlling stats generation from the client-side, to make it a little bit reusable. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
🚀 Feature Proposal
This is a combination of a question and a feature proposal really: it's possible that this may not be possible.
Basically, while it looks like the webpack plugin already turns off a few unnecessary stats keys here
loadable-components/packages/webpack-plugin/src/index.js
Line 21 in 3676e48
assets
,chunks
, andnamedChunkGroups
since these appear to be the only properties accessed onthis.stats
insideChunkExtractor
.I'll need to look into which options webpack lets us control when outputting stats, but iirc it's basically everything. Worst case it'd still be possible to perform an additional pass and strip out anything that loadable won't be relying on.
Motivation
Even with the code setting stats options above, in my project I'm still seeing
loadable-stats.json
become very large in certain circumstances which takes up space on disk but more importantly slows down myrequire()
in ssr when passing the data intoChunkExtractor
.From looking at the code in
ChunkExtractor
(https://github.com/gregberge/loadable-components/blob/3676e488a714068baca5cefad2aa98c0de0a8fab/packages/webpack-plugin/src/index.js), it seems like most of the properties taking up space are things likewarnings
,children
, and other things Loadable doesn't seem to actually care about.Example
It would be the default way of generating stats, but if there's a usecase for errors/warnings etc. then perhaps a filter function could be added as an api option: it would be a function taking the output loadable stats json object, and returning a new object.
This might also be generally useful for anyone wishing to modify their loadable stats file before it's output, for whatever reason.
Pitch
This seems overall more efficient and seems like it has little downside unless these properties are used in a way I haven't noticed.
The text was updated successfully, but these errors were encountered: