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

[Next 9] Importing react-dom would bloat the bundle size #7949

Closed
zen0wu opened this issue Jul 14, 2019 · 12 comments · Fixed by #8609
Closed

[Next 9] Importing react-dom would bloat the bundle size #7949

zen0wu opened this issue Jul 14, 2019 · 12 comments · Fixed by #8609
Assignees
Milestone

Comments

@zen0wu
Copy link

zen0wu commented Jul 14, 2019

Bug report

Describe the bug

Importing react-dom in a page or in one of its dependent component will cause react-dom.production.min.js to be included in the page bundle, adding about 32K gzipped size to a page.

This is pretty common, examples:

  • ReactDOM.createPortal
  • react-transition-group also imports react-dom

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

  1. Go to 'pages/any-page.js'
  2. Add import ReactDOM from 'react-dom';
  3. In the render function, do console.log(ReactDOM). Just to use that dependency.
  4. Production build the project and inspect the bundle size

Expected behavior

react-dom should just be in the common bundle, not included again here.
This does not happen with Next 8.

Screenshots

System information

  • OS: macOS
  • Version of Next.js: 9.0.1

Additional context

Add any other context about the problem here.

@timneutkens
Copy link
Member

@atcastle is this fixed by the new chunking logic?

@Timer Timer modified the milestones: 9.0.x, 9.0.4 Aug 10, 2019
@atcastle
Copy link
Collaborator

atcastle commented Aug 12, 2019

@timneutkens This definitely shouldn't happen with granular chunks--but then, it shouldn't happen without it, either. The old SplitChunksConfig has
test: /[\\/]node_modules[\\/](react|react-dom|scheduler)[\\/]/, which should always split react-dom out of the endpoint bundles and into commons.

I just tried to repro this by following the directions in the bug report on one of the integration test apps and I didn't see the behavior (even without granular chunks). Do we have an example repo that exhibits this behavior?

@timneutkens
Copy link
Member

Yeah it definitely shouldn't happen / duplicate

@wyattanderson
Copy link

I'm definitely seeing this in an app of mine; I'll see about putting together a reproducer.

@zen0wu
Copy link
Author

zen0wu commented Aug 12, 2019

Indeed it cannot be reproduced with a simple example.

Did some experiment. It seems that it only appears after your project reaches a certain level of complexity. I pinned down to something like

pageA (which imports react-dom) -> componentA -> componentA2`
pageB (does not import react-dom) -> componentB
index (does not import react-dom) -> componentX

And it's like, removing any of these dependency would make pageA shrinks to normal. Even in componentA2 there's an export function foo(), removing that would makes pageA shrink. So my guess is definitely related to complexity of the dependency graph.

@wyattanderson What do you see in your case? react-dom as well? It would be great if we can have a repro.

@wyattanderson
Copy link

wyattanderson commented Aug 12, 2019

Yeah, I'm away from the machine that has this project on it at the moment, but it's for react-dom and by using webpack-bundle-analyzer, I can see that:

  • the commons chunk has react-dom
  • any page or component chunk that imports react-modal (a third-party NPM module which itself imports react-dom, as a peer dependency) also has a separate copy of react-dom; these pages don't import react-dom directly.

Removing the react-modal import removes react-dom from the page/component chunk.

@zen0wu
Copy link
Author

zen0wu commented Aug 26, 2019

Just tried out 9.0.5, it still isn't fixed yet. Is that expected?

@timneutkens
Copy link
Member

@ZenoZen it's fixed in the new chunking behavior:

module.exports = {
  experimental: {
    granularChunks: true
  }
}

@zen0wu
Copy link
Author

zen0wu commented Aug 26, 2019

@timneutkens Thanks. Confirmed it's fixed with this config. What's the plan going forward? Is it going to be stablized? Or I should wait a little more while to upgrade

@timneutkens
Copy link
Member

It's going to be out of experimental soon once we have more metrics 👍

@zen0wu
Copy link
Author

zen0wu commented Aug 27, 2019

@timneutkens I just tried it out.

One issue is the shared bundle generated, since its encoded using base64, it causes some problem with CDN. (Cloudfront doesn’t seem to work with characters like + and =)

I believe the code is this line: https://github.com/zeit/next.js/blob/f94e75d8aa9ed6869c106530271f216361ce4425/packages/next/build/webpack-config.ts#L276

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants