Skip to content
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

feat: add documentation for SplitChunksPlugin and deprecation warning… #1955

Merged
merged 9 commits into from
Apr 5, 2018

Conversation

jeremenichelli
Copy link
Member

@jeremenichelli jeremenichelli commented Mar 26, 2018

Sorry for the noise, the content for this branch was already approved here: #1863 but because of some issues on rebasing I needed to create a new PR to make sure I'm not making regression on content after the next branch merge.

Solves #1812

@jeremenichelli
Copy link
Member Author

Hey @skipjack, Travis is throwing an error: API rate limit exceeded for 35.224.112.202, did we exceeded some quota around the service? 🤔

@montogeek
Copy link
Member

@jeremenichelli It is exceeded every a few hours, something to fix in Infrastructure

@jeremenichelli
Copy link
Member Author

@montogeek should we just trigger a new build for the branch?

@montogeek
Copy link
Member

@jeremenichelli I am fixing it for once and all #1963

Copy link
Collaborator

@skipjack skipjack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, I would say aside from some grammar issues this was pretty intuitive (both the docs and the new API).

We don't really have a standard for the plugin docs yet but I tried to provide some general grammar and content related feedback. I also think this might be mixing the new configuration options in with the plugin docs too much.

Correct me if I'm wrong but I think some config can be done via optimization.splitChunks and optimization.runtimeChunk without needing to include the plugin manually but more advanced functionality can be achieved by using the plugin directly.

If this is the case, I'd recommend moving the config parts to the /configuration section somewhere and only keeping the plugin-specific bits in SplitChunksPlugin.md.

url: https://medium.com/webpack/webpack-4-code-splitting-chunk-graph-and-the-splitchunks-optimization-be739a861366
---

Originally in webpack, chunks (and modules imported inside them) were connected by a parent-child relationship in the internal webpack graph.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rephrase as (merged with the next lines as well):

By default, chunks (and the modules inside them) are connected by direct parent-child relationships in the dependency graph. However, there are instances where a dependency may be duplicated across chunks. Previously, the CommonsChunkPlugin was use to dedupe these dependencies. In v4, the optimization.splitChunks and optimization.runtimeChunk configuration options provide a more intuitive interface.

Here is how the new flow works...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that stating that they are (present tense) linked by parent-child relationship is confusing/wrong since this has completely changed on the core of the package.

This connection didn't allow to further optimizations and resulted into more downloaded code.

webpack v4 removes the `CommonsChunkPlugin` in favor of `optimization.splitChunks` and `optimize.runtimeChunk` options. Here is how the new flow works.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We typically use two line breaks between h2 sections. We should really automate this via remark and/or prettier along with various other markdown guidelines.

* Condition 4: Doesn't affect request at initial page load

Putting the content of `helpers` into each chunk will result into its code being downloaded twice. By using a separate chunk this will only happen once. We pay the cost of an additional request, which could be considered a tradeoff. That's why there is a minimum size of 30kb.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same two line breaks nit (just helps with readability when editing).


## Defaults

Out of the box `SplitChunksPlugin` should work great for most users.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it included out of the box or only when certain flags/config are present? Or do you just mean when included with no options, like so:

plugins: [
  new SplitChunksPlugin()
]


Out of the box `SplitChunksPlugin` should work great for most users.

By default it only affects on-demand chunks because changing initial chunks would affect the script tags the HTML file should include to run the project.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe explain why changing those initial script tags is a problem?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I think that the sentence is pretty clear by itself.

}
```

W> This downloads more code than neccessary.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/neccessary/necessary

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also maybe elaborate this warning a bit...

This will probably yield an unnecessarily large bundle as many applications use a bunch of node_modules. It's typically better to extract and load your core frameworks on initial load, and let other dependencies be chunked out and loaded dynamically later on.


### Split Chunks: Example 1

Create a `commons` chunk, which includes all code shared between entrypoints.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/entrypoints/entry points

@phun-ky
Copy link

phun-ky commented Mar 28, 2018

Nice work, this is very much needed for all webpack4 users!

But, I can't see that you have included all the options for splitChunks, for example:

automaticNameDelimiter (defaults to: ~)

All webpack options is in: schemas\WebpackOptions.json, and the default options are set here: lib\WebpackOptionsDefaulter.js

Specially automaticNameDelimiter was important to us, because we have a WAF (NAXSI) that had a ruled triggered for the usage of the tilde(~) character, resulting in a 406 Not Acceptable on the server when the chunk is requested: vendor~moduleA~moduleB.js.

The `CommonsChunkPlugin` is an opt-in feature that creates a separate file (known as a chunk), consisting of common modules shared between multiple entry points. By separating common modules from bundles, the resulting chunked file can be loaded once initially, and stored in cache for later use. This results in pagespeed optimizations as the browser can quickly serve the shared code from cache, rather than being forced to load a larger bundle whenever a new page is visited.
The `CommonsChunkPlugin` is an opt-in feature that creates a separate file (known as a chunk), consisting of common modules shared between multiple entry points.

W> The CommonsChunkPlugin has been deprecated in webpack v4 legato. To learn how chunks are treated in the latest version, check out the [SplitChunksPlugin](/plugins/split-chunks-plugin/).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not deprecated: it has been removed.


## `optimization.runtimeChunk`

Setting `optimization.runtimeChunk` to `true` adds an additonal chunk to each entrypoint containing only the runtime.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you could also talk about optimization.runtimeChunk="single" which creates a single runtime chunk shared by all entries.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know what's the default value and behavior when this is not set?

splitChunks: {
cacheGroups: {
commons: {
test: /[\\/]node_modules[\\/],
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test: /[\\/]node_modules[\\/]/,

@jeremenichelli
Copy link
Member Author

Everyone here, I've updated the branch with your feedback. Feel free to take a look at it, if it is I think we should merge it already, information about this plugin should have been there from day one in my humble opinion.

@jeremenichelli
Copy link
Member Author

Some things might not be covered on the first version, I'm tracking the missing stuff here: #1956

minChunks: 1,
maxAsyncRequests: 5,
maxInitialRequests: 3,
automaticNameDelimiter: '~',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the indentation is wrong here. It looks like everything else is using tabs, but this line uses spaces.
screen shot 2018-03-30 at 5 44 50 pm

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the catch!

Copy link
Collaborator

@skipjack skipjack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for making the updates. The only thing I'm still a little unclear on is what bits are webpack config options and what's supposed to be specified as options of the SplitChunksPlugin. Totally understand the need to get this merged though so maybe that can be clarified later on.

@jeremenichelli
Copy link
Member Author

jeremenichelli commented Apr 3, 2018 via email

@jeremenichelli jeremenichelli merged commit 7c8c30f into master Apr 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants