Modifies webpack config in ReactReduxSpa template to enable treeshaking #690
Conversation
|
Hi @markthiessen, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! TTYL, DNFBOT; |
|
I am wondering whether we couldn't keep the .babelrc and put the 'modules: false' into that as all it does is stop the transformation of ES6 modules into CommonJS modules. Not sure why that would cause problems for the serverside bundle? As it stands its a bit inconsistent that the serverside build would use the .babelrc, but the client doesn't in this PR. |
markthiessen
force-pushed
the
enable-webpack-tree-shaking-reactredux
branch
from
4f869eb to
1e85b0e
Compare
|
@DanHarman I think you're probably right.. I've modified the PR to just include the .babelrc change |
|
Awesome. Hopefully this will be merged in soon. |
|
So I just tested this config, and I've got a feeling it doesn't offer much (any?) benefit with the build methodology in this template. I think the fundamental problem is that 'webpack --config webpack.config.vendor.js', uses a config which explicitly sets entrys to the top levels of all the heavy packages. This means none of this stuff gets shaken at all, and in fact can't be as its not working out the true dependency chain based on what the app is actually using. I'm now left wondering if this separation of vendor and app packages is actually a good trade off, as we miss out on potentially significant savings on package size due to tree shaking. At least for a 'release' build. @SteveSandersonMS - thoughts? |
|
Thanks @markthiessen for suggesting this! Like @DanHarman, I'm also not seeing any significant changes in the resulting bundle size though, whether or not we use the vendor bundle. First, with the vendor bundle enabled (as it is by default in the template), I'm seeing:
Second, with the vendor bundle disabled (i.e., by commenting out the
These are all production builds (i.e., with So although it does very slightly reduce the size in the non-vendor-split case, it's not the sort of dramatic effect that people usually hype up about tree shaking. Is it possible that further build config changes are needed for it to really strip out large parts of the third-party libraries that never get called? |
|
Hi, I just tested by removing the vendor bundle (thanks for explaining how to do that). I am seeing benefit now. So I have a very large vendor bundle as currently swapping from bootstrap to semantic-ui so both being pulled in: λ webpack --config webpack.config.vendor.js
Hash: 18469d22acbb001775eaf66083ccd1a7528a5586
Version: webpack 2.2.1
Child
Hash: 18469d22acbb001775ea
Time: 37617ms
Asset Size Chunks Chunk Names
674f50d287a8c48dc19ba404d20fe713.eot 166 kB [emitted]
912ec66d7572ff821749319396470bde.svg 444 kB [emitted] [big]
b06871f281fee6b241d60582ae9369b9.ttf 166 kB [emitted]
89889688147bd7575d6327160d64e760.svg 109 kB [emitted]
vendor.js 3.39 MB 0 [emitted] [big] vendor
vendor.css 1.15 MB 0 [emitted] [big] vendor
Child
Hash: f66083ccd1a7528a5586
Time: 30403ms
Asset Size Chunks Chunk Names
674f50d287a8c48dc19ba404d20fe713.eot 166 kB [emitted]
89889688147bd7575d6327160d64e760.svg 109 kB [emitted]
912ec66d7572ff821749319396470bde.svg 444 kB [emitted] [big]
b06871f281fee6b241d60582ae9369b9.ttf 166 kB [emitted]
vendor.js 3.84 MB 0 [emitted] [big] vendorNow using this vendor bundle my project is as follows: without vendor bundle: So without tree shaking:
With:
Now this is just a test project which is mostly just the templates code, so not sure how much would be saved with a 'real' app. i.e. right now 100% of semantic-ui-react is being dumped I would think. On the plus side, the build time, given I'm using the cacheDirectory setting is actually still very good. approx 10 seconds with the client config, which is faster than without the cacheDirectory setting using the vendor config split out approach. |
|
Did a further test with pretty much the bare template (i.e. no semantic-*) and vendor.js is about 2MB, if we go with a tree shaken no vendor approach the output is 1.2MB. So does seem like a good saving. I'm not sure whether the split is better or not in light of this. Obviously if framework is fairly static and the app iterates fast, then its a win to only be downloading a few k for the app. However we are looking at a pretty big overhead in total of not tree shaking. I wonder if there is a way to tree shake the vendor.js based on what's in the client, whilst keeping it split out? Obviously this would be subject to change as new components were pulled in. Forgot to say I did all my tests in Development... |
|
@SteveSandersonMS I think the results you included for with vendor dll enabled are without the These are the results I get: Without prod switch Before PR: running With --env.prod It is a fairly small savings, true, but it's also a template with hardly any code :) It's slightly more pronounced in my project. |
|
@markthiessen perhaps I've misunderstood, but to get a major benefit from tree shaking we need to consider vendor.js as that's typically where the pruning would happen. i.e. we compare the client + vendor without shaking (because we can't shake the vendor modules when they are packaged separately) vs a much fatter client (which can shake out the vendor modules). From my tests above, it does make a big difference, but I only ran them for dev builds. |
Ah yes, thanks.
I'd like to be able to reproduce your results before we consider the effects. If you have a 2MB bundle, that suggests you're not minifying (not using On the original template, these are the production build sizes I see:
So in all cases the differences are negligible, and certainly the advantages of vendor-splitting outweigh that sort of difference. In what production-build cases does it makes a significant difference? Is it possible that we still don't have it configured correctly and tree-shaking should be making a bigger difference even on the default template?
You should expect the effect to work the opposite way. The fact that the template is small should (theoretically) mean that tree-shaking produces much greater savings, because we're invoking less of the third-party code. As your application gets bigger, you'd expect tree-shaking to produce smaller savings, since you'll be referencing a larger proportion of third-party code paths. |
|
Ok so I've rerun with --env.prod. Thanks for the correction there, still feeling my way out around nodeland! Firstly I think I can confirm that tree shaking is enabled. With semantic-ui and semantic-ui-react pulled in, but not using any components I get:
i.e. it has shaken out semantic-ui. However this is a completely artificial test as I need to use some of that module to see how it would fare under more realistic conditions. n,b. my So perhaps the main take away here is that it really depends on what frameworks you are pulling in and how much of them you are using. The existing template is clearly factored pretty tightly without a great deal of anything to trim. I don't see any downside to having the Also another option is to just use `babel-preset-es2015-native-modules’ btw, and this may impact my results, are you guys having success running |
Agreed. I'll merge this change, even if the benefits are very modest as far as we can tell. There doesn't seem to be any drawback and it might make a significant difference in some case.
So far we don't have any examples of realistic cases where it would be a good idea not to do the vendor-splitting. If anyone finds themselves in a real-world case where it's best not to vendor-split, please let us know! Failing that, I think we're good as-is.
Yes, I was removing |
|
FYI, I would also recommend trying out https://github.com/babel/babel-preset-env instead of es2015 😄. Let me know if you have questions (I made preset-env + maintain Babel) You can also join our slack: http://slack.babeljs.io/ |
One of the big features in Webpack 2 is tree shaking. Modified config to enable this in client bundle output.
Debated adding logic to common config as server bundle still needs module transform, but in the end decided it was probably clearer for developers to just pull out module rules from common and have client and server config define separately.