[WIP] New vendor bundle block #188
Conversation
|
I think both are (or could be) the same. I'm all for having an official block and deprecating mine. I would just check if the problem I wrote there still persists: https://github.com/diegohaz/webpack-blocks-split-vendor#how-it-does (the reason I used |
packages/vendor-bundle/README.md
Outdated
|
|
||
| #### minChunks *(default: 5)* | ||
|
|
||
| Minimal number of chunks to include dependency into the bundle. |
There was a problem hiding this comment.
Why wouldn't you want to include a dependency with few chunks into the vendor bundle?
There was a problem hiding this comment.
Because you may have some big dependencies that you’re using only on several pages, like some fancy chart library.
There was a problem hiding this comment.
So my idea was to include dependencies that are used on most pages.
There was a problem hiding this comment.
Ahhh, it's not about the size of the dependency, but how many chunks are using this dependency. I think the documentation should highlight that 😉
There was a problem hiding this comment.
But that’s exactly what is says! ;-)
There was a problem hiding this comment.
At least I didn't get that it means "number of chunks using the dependency" and I am maintainer 😜
There was a problem hiding this comment.
OK, I’ll update it ;-)
|
I think the md5-hashing is just an alternative solution to what @sapegin implemented as Thanks for pointing out your package, @diegohaz. I almost forgot about it. I think the way this package (in this PR) works seems cleaner, but from a high-level view they do pretty much exactly the same, right? |
|
What's |
Yup, it’s solving the same problem. As I understand the benefit of md5-hashing is that you don’t have another bundle just for IDs.
Here it is: /**
* Extract webpack runtime with module IDs so changes in app code won’t invalidate the vendor bundle.
*
* @returns {function}
*/
module.exports = function() {
return (context, { addPlugin }) => addPlugin(
new context.webpack.optimize.CommonsChunkPlugin({
name: 'runtime',
minChunks: Infinity,
})
);
}; |
|
I see now (didn't find it in the code though). That's pretty neat. |
|
We still need a decision on that, right? |
|
I want to make it more generic and support both use cases: my initial and from @diegohaz’s block so we could have one block “to rule them all” ;-) |
|
Looks like this needs more research. Reading: https://medium.com/webpack/predictable-long-term-caching-with-webpack-d3eee1d3fa31 |
|
Whoa, good idea with this PR, vendor splitting is the most common and most reasonable code splitting. I did this once to a big project to minimize dev build times, I also tried to implement the DllPlugin on the vendor chunk to basically freeze it and almost remove the compile times of that chunk, but I wasn't able to do it because of the too much overhead of that plugin, but maybe you guys can do it! |
|
An one more link: |
|
I'd like all options to be passed down to a CommonsChunkPlugin rather than a small whitelist. |
|
@vlad-zhukov Yeah, I’m going to make it more generic. |
|
@sapegin The functionality of this block should be included in the
|
| ## Usage | ||
|
|
||
| ```js | ||
| const { createConfig } = require('@webpack-blocks/webpack') |
There was a problem hiding this comment.
@webpack-blocks/webpack re-exports createConfig from @webpack-blocks/core.
|
|
||
| ```js | ||
| const { createConfig } = require('@webpack-blocks/webpack') | ||
| const vendorBundle = require('@webpack-blocks/vendorBundle') |
There was a problem hiding this comment.
Change @webpack-blocks/vendorBundle to @webpack-blocks/vendor-bundle. NPM doesn't support capital letters in the package names.
| name: options.main, | ||
| filename: options.filename, | ||
| children: true, | ||
| minChunks ({ userRequest }, count) { |
There was a problem hiding this comment.
resource is better than userRequest because it's more accurate.
| filename: options.filename, | ||
| children: true, | ||
| minChunks ({ userRequest }, count) { | ||
| return userRequest && userRequest.includes('node_modules') && |
There was a problem hiding this comment.
String#indexOf is faster than String#includes (see this benchmark).
| "@webpack-blocks/core": "^1.0.0-beta", | ||
| "ava": "^0.18.2", | ||
| "standard": "^8.6.0", | ||
| "webpack": "^3.5.1" |
There was a problem hiding this comment.
webpack must be in peerDependencies because index.js imports it.
|
Just a reminder, webpack 4 splits vendors by defult, so this functionality will be already there when webpack-blocks is upgraded wo webpack 4 |
|
This block doesn't make any sense in webpack 4, I'm going to close the PR. |
No description provided.