Make the vendor bundle hold vendor code #307
Conversation
|
Review status: 0 of 3 files reviewed at latest revision, 2 unresolved discussions. client/package.json, line 83 [r1] (raw file):
This wasn't used. client/webpack.client.base.config.js, line 37 [r1] (raw file):
@robwise @alexfedoseev Does this look right? Worth committing this? without this change, the vendor-bundle.js is much smaller than the app-bundle.js, because libraries like react are in the app-bundle.js. Comments from Reviewable |
justin808
force-pushed
the
move-vendor-libs-to-vendor-bundle
branch
from
a7690e4 to
fada45b
Compare
|
Reviewed 3 of 3 files at r1. client/webpack.client.base.config.js, line 37 [r1] (raw file):
|
|
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed. client/webpack.client.base.config.js, line 37 [r1] (raw file):
|
|
Reviewed 3 of 3 files at r1. Comments from Reviewable |
|
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed. client/webpack.client.base.config.js, line 37 [r1] (raw file):
|
|
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed. client/webpack.client.base.config.js, line 37 [r1] (raw file):
|
|
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed. client/webpack.client.base.config.js, line 37 [r1] (raw file):
config.entry.vendor.unshift(
'es5-shim/es5-shim',
'es5-shim/es5-sham',
'jquery-ujs',
// Configures extractStyles to be true if NODE_ENV is production
'bootstrap-loader/extractStyles'
);
Comments from Reviewable |
By setting the vendor libs as entry points, we force the vendor code into the vendor-bundle.js. This has the advantage in that app code should be changing less often, so client browsers will have the vendor bundle cached longer, and the smaller app bundle can be refreshed. Warning on what gets included: If in your vendor array you have react-router and somewhere in the codebase you import react-router/es6 — this import won't get to vendor bundle.
justin808
force-pushed
the
move-vendor-libs-to-vendor-bundle
branch
from
fada45b to
e058da3
Compare
|
@justin808 @robwise It's not the end of the story tho. This change does bust the cache, but there is another issue: it busts it on every rebuild, even if vendor modules bundle wasn't changed at all. This happens b/c of the way webpack identifies dependencies — it uses integer ids, so when any new import is added to the app bundle, ids are changed and hashes of all bundles are invalidated. It's an old ongoing issue, there are a several workarounds, but non of them is perfect. Here is the long read on topic: webpack/webpack#1315 |
|
Wow @alexfedoseev! great reference! So basically, this is sort of pointless, except, if you want to open up the small app-bundle file and see what Webpack is doing. |
|
@justin808 Sometimes yes, sometimes no. It's still useful when you use code splitting w/ dynamic chunks loading. Also it can be solved for some cases. Previously I used |
Absolutely not. Even with the cache busting issue, that's only an issue when re-deploying. This is still useful because it's not requiring you to keep re-downloading the same JS over and over again as you visit pages with different bundles on them. If you don't do this, and you have 8 bundles, then you're going to have 8 instances of jquery, one in each bundle, that you must keep re-downloading over and over. Same for all the other ones. I did this on our F&G in the first place because it was taught here: https://egghead.io/lessons/tools-grouping-vendor-files-with-the-webpack-commonschunkplugin. Without this, our bundles were averaging 5MB each, with this change, we got the vendor bundle to 2MB and the other bundles to ~120KB |
By setting the vendor libs as entry points, we force the vendor code
into the vendor-bundle.js. This has the advantage in that app code
should be changing less often, so client browsers will have the vendor
bundle cached longer, and the smaller app bundle can be refreshed.
This change is