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(gatsby): add better splitchunks config #17093

Merged
merged 1 commit into from Aug 26, 2019

Conversation

@wardpeet
Copy link
Member

commented Aug 26, 2019

Description

Creates a basic splitChunks config that works better than our current one. It's not the final config we would like as that one needs more testing. This one works pretty well so far.

Credits go to Nextjs as I copied the code from their codebase.
https://github.com/zeit/next.js/blob/canary/packages/next/build/webpack-config.ts#L200-L216

We want to go to a granular way of splitting chunks like nextjs does https://github.com/zeit/next.js/blob/canary/packages/next/build/webpack-config.ts#L217-L275 but create one that works for Gatsby.

This PR saves around 400kb on .org and lowers visual-complete and TTI (fist-cpu-idle) by 400ms which will lower even more when guess-js is fixed.

Comparison can be found here:
https://www.webpagetest.org/video/compare.php?tests=190826_2H_5a44521034c35d43ff27a3af95b7c53c%2C190826_D7_ed0d076acfadfd3575943dfcf6cf34b8&thumbSize=200&ival=100&end=visual

Test current:
https://www.webpagetest.org/result/190826_D7_ed0d076acfadfd3575943dfcf6cf34b8/

Test this pr:
https://www.webpagetest.org/result/190826_2H_5a44521034c35d43ff27a3af95b7c53c/

bundle analyzer:
current master: https://5d63face1c5e2fc7e4802b5d--gatsbyjs-dev.netlify.com/old-chunks.html
PR: https://5d63face1c5e2fc7e4802b5d--gatsbyjs-dev.netlify.com/splitchunks.html

Related Issues

Fixes #15302

@wardpeet wardpeet requested a review from gatsbyjs/core as a code owner Aug 26, 2019
chunks: `all`,
// if a chunk is used more than half the components count,
// we can assume it's pretty global
minChunks: componentsCount > 2 ? componentsCount * 0.5 : 2,

This comment has been minimized.

Copy link
@sidharthachatterjee

sidharthachatterjee Aug 26, 2019

Member

More than half components count is a nice metric 😅

This comment has been minimized.

Copy link
@sidharthachatterjee

sidharthachatterjee Aug 26, 2019

Member

Does store.getState().components.size include pages?

This comment has been minimized.

Copy link
@wardpeet

wardpeet Aug 26, 2019

Author Member

I need components as they are the ones that create chunks. We can have 1000 of pages that only use 1 component so we need to check components and not pages.

This comment has been minimized.

Copy link
@sidharthachatterjee

sidharthachatterjee Aug 26, 2019

Member

Hmm, fair enough

This comment has been minimized.

Copy link
@KyleAMathews

KyleAMathews Aug 26, 2019

Contributor

we could do more analysis on this in the future but it'd make sense to account for the number of pages / component — the key question about adding code to a chunk is "how likely is the user to need this code?" — if a page component is used for 1000s of pages — then users are much more likely to visit that page than a page component for some rarely visited page.

It'd be nice also to integrate analytics into this stuff.

This comment has been minimized.

Copy link
@slorber

slorber Aug 26, 2019

Contributor

Crazy idea: GuessJS for webpack config :D

This comment has been minimized.

Copy link
@slorber

slorber Aug 26, 2019

Contributor

Note I've opened a related issue where you'll find links to what has been done recently in NextJS which has a new "granularChunks" config: #16661

This comment has been minimized.

Copy link
@wardpeet

wardpeet Aug 26, 2019

Author Member

Yeah so the idea is to go on an even more granular level, like nextjs but I wanted something I could get in fairly easy without having to check x amount of sites.

Copy link
Member

left a comment

This is solid! 🥇

@sidharthachatterjee sidharthachatterjee merged commit fc93b7b into gatsbyjs:master Aug 26, 2019
20 checks passed
20 checks passed
Danger All good
Details
Peril All green. Good on 'ya.
Details
ci/circleci: bootstrap Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_development_runtime Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_gatsby-image Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_path-prefix Your tests passed on CircleCI!
Details
ci/circleci: e2e_tests_production_runtime Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_gatsby_pipeline Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_long_term_caching Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: starters_validate Your tests passed on CircleCI!
Details
ci/circleci: themes_e2e_tests_development_runtime Your tests passed on CircleCI!
Details
ci/circleci: themes_e2e_tests_production_runtime Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_node10 Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_node12 Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_node8 Your tests passed on CircleCI!
Details
ci/circleci: unit_tests_www Your tests passed on CircleCI!
Details
ci/circleci: windows_unit_tests Your tests passed on CircleCI!
Details
cypress: default-group 67 tests passed in 00:27
Details
unit_tests_windows Build #20190826.27 succeeded
Details
@wardpeet wardpeet deleted the wardpeet:fix/large-bundles branch Aug 26, 2019
waltercruz added a commit to waltercruz/gatsby that referenced this pull request Sep 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.