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

Treeshaking and Codesplitting Default Behaviour #24866

Closed
1 of 3 tasks
Undistraction opened this issue Jun 8, 2020 · 9 comments
Closed
1 of 3 tasks

Treeshaking and Codesplitting Default Behaviour #24866

Undistraction opened this issue Jun 8, 2020 · 9 comments
Assignees
Labels
status: needs reproduction This issue needs a simplified reproduction of the bug for further troubleshooting. topic: webpack/babel Webpack or babel type: question or discussion Issue discussing or asking a question about Gatsby

Comments

@Undistraction
Copy link
Contributor

Undistraction commented Jun 8, 2020

Summary

I'm opening this as a question as I'm unsure if Gatsby's behaviour in this instance is correct / expected. It certainly doesn't seem to be optimal, but it could be something I've overlooked. If this is unexpected I'll open this as a bug.

Our Gatsby site consumes a number of packages from our monorepo. Two of these packages are:

  • @example/utils
  • @example/site-forms

@example/utils contains a lot of utility functions, many of which wrap other npm libraries, so has a lot of third-party dependencies, however it often only uses a small subset of those dependencies' apis.

@example/site-forms contains a large number of React components representing forms we use on the site. The forms are fairly complex and non-trivial in size. Almost every form is used on a single page of the site only.

Both packages are transpiled, but leave ES6 imports/exports intact to facilitate tree-shaking. Both packages' package.json have sideEffects: false. Both packages use re-exports from an index.js file which is their main file:

Either:

export * from './utils/api'

Or

export { default as ContactForm } from './forms/ContactForm'

Expected behaviour

From reading the docs on Gatsby's usage of code-splitting and treeshaking, I would expect the following:

  1. Any exports that are not used in the site are not included in any bundles.
  2. Any exports that are used on every page are included in the commons bundle.
  3. Any exports that are used on a single page only are included in the bundle for that individual page.

Observed behaviour

  1. Any exports that are not used in the site are not included in any bundles.
  2. Any exports that are used in three or more files are included in the commons bundle.
  3. Any exports that are used on a single page only are included in the bundle for that individual page.

For 2. I'm seeing every export from either of these packages that is used anywhere in the site being included in the commons bundle.

For 3. I'm seeing bundles being generated for individual pages, but these only include page-specific code that is defined within my gatsby project (not third-party).

So the end result is that I have a huge commons bundle and lots of very small page bundles.

As far as my understanding goes, this is definitely not optimal behaviour and appears to be unexpected in relation to the documentation. Am I expecting too much from Gatsby/Webpack? The packages themselves are used by more than one page, but their exports are not.

Note that I'm seeing similar behaviour with third-party libs. For example, netlify-cms-app is used only on our /admin/ page, however, at least some of its dependencies are included in thecommons chunk.

Relevant information

  • I have no custom Webpack configuration.

  • Imports of form components are happening directly from the packages within templates. There is no hidden interdependency going on.

  • gatsby@2.23.1

  • gatsby-cli@2.12.44

@Undistraction Undistraction added the type: question or discussion Issue discussing or asking a question about Gatsby label Jun 8, 2020
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jun 8, 2020
@herecydev
Copy link
Contributor

Discussion around bundling is always quite tricky, it might be worth reading:

#22253
https://web.dev/granular-chunking-nextjs/
#24799

There's a lot of challenge balancing the needs of all websites. Does the behavior outlined in the granular chunking PR (which is now merged) match what you are seeing?

@Undistraction
Copy link
Contributor Author

Undistraction commented Jun 8, 2020

@herecydev Thanks. Yes I believe it does, but the upshot for our project is we now have an enormous commons chunk which is killing our Lighthouse scores. Is there any way to configure things so the old behaviour is returned? This new approach also means distributing components via a package becomes problematic, as they will all be gathered into the commons chunk, despite the fact the intention is the exact opposite.

So it seems with this new behaviour, our only options are to:

  1. Make every component to a separate package.
  2. Move all our components our of our monorepo and into our Gatsby site so they are not treated as a single package.

Without doing this, as soon as exports from a package are used on more than two pages, the entire package will be baked into the commons chunk.

@herecydev
Copy link
Contributor

I think you'll need to roll your sleeves up and potentially modify the splitChunks config:
https://www.gatsbyjs.org/docs/add-custom-webpack-config/

Without doing this, as soon as exports from a package are used on more than two pages, the entire package will be baked into the commons chunk.

I suppose it's always a tradeoff, If an import is used 10 times, do you still want to "burn it into the page chunk" for each page? Where do you draw the line?

@Undistraction
Copy link
Contributor Author

Undistraction commented Jun 8, 2020

I suppose it's always a tradeoff, If an import is used 10 times, do you still want to "burn it into the page chunk" for each page? Where do you draw the line?

Yeah. It's a tricky trade-off. It's just a pretty big change for a patch release, but I guess it won't have such a negative impact for most people. Like you said, looks like I'll have to to do this by hand. Not sure what to do with this issue. I think the point stands so I'm a little reluctant to close it. And I imagine others will have the same issues.

@KyleAMathews
Copy link
Contributor

cc @wardpeet

@danabrit danabrit added topic: webpack/babel Webpack or babel and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Jun 8, 2020
@wardpeet
Copy link
Contributor

wardpeet commented Jun 9, 2020

@Undistraction is it possible to share your sourcemaps or source code somehow? If you're on the latest gatsby, you should have pretty good bundling. We can't move everything out of commons/apps as plugins using gatsby-browser, ... are sadly tied to all pages.

@Undistraction
Copy link
Contributor Author

@wardpeet I can't I'm afraid, but I'll see if I can get permission. Definitely something strange going on. This combined with the disaster of Lighthouse 6 scores is tanking my week ;)

@LekoArts
Copy link
Contributor

Please provide a minimal reproduction for this issue. Thanks!

@LekoArts LekoArts added the status: needs reproduction This issue needs a simplified reproduction of the bug for further troubleshooting. label Jun 26, 2020
@freiksenet
Copy link
Contributor

Hi!

I'm going to close this now, as we can't do much to help without a reproduction. If you are able to create a minimal reproduction for this then please do reopen the issue.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs reproduction This issue needs a simplified reproduction of the bug for further troubleshooting. topic: webpack/babel Webpack or babel type: question or discussion Issue discussing or asking a question about Gatsby
Projects
None yet
Development

No branches or pull requests

7 participants