apollographql / apollo-client-nextjs

Apollo Client support for the Next.js App Router
https://www.npmjs.com/package/@apollo/experimental-nextjs-app-support
MIT License
358 stars 25 forks source link

Large bundle size compared to the standard @apollo/client #126

Closed tombryden closed 4 months ago

tombryden commented 6 months ago

Hi,

I have noticed when running npm run build the first load JS is incredibly large when using any of the hooks from this library when compared with the standard @apollo/client

Import size using 'Import Cost' extension of both the libraries:

image vs image

npm run build results

image vs image

Thanks

phryneas commented 6 months ago

I've seen something similar in #95 where using NextSSRApolloClient instead of ApolloClient in registerApolloClient (so for RSC, not SSR) drove bundle size a lot - is that maybe happening here?

tombryden commented 6 months ago

I am using the NextSSRApolloClient, however I just tested with a fresh project and even without any wrapper the package size is 75+kb for any import from @apollo/experimental-nextjs-app-support/ssr.

Fresh project, only containing page.tsx and layout.tsx causes the same issue:

"use client";

import { useQuery } from "@apollo/experimental-nextjs-app-support/ssr";

console.log(useQuery);

export default function Home() {
  return (
    <main>
    </main>
  );
}

image

tombryden commented 6 months ago

https://github.com/apollographql/apollo-client-nextjs/issues/95#issuecomment-1721965647 - I have also noticed strange behaviour in other instances with bundle sizes related to "use client".

Just raised a bug with MUI earlier today showing similar behaviour - https://github.com/mui/material-ui/issues/39814

tombryden commented 4 months ago

@phryneas I have just re-tested this on 14.0.4 but still the same issue. The MUI bug I mentioned above appears to be fixed on 14.0.4 so I had some hope

phryneas commented 4 months ago

Looking into this, this looks like some bug in the next.js build report.

If I build the pages (working with our integration test here), I get these entries in my manifest (.next/app-build-manifest.json):

{
  "/cc/dynamic/useSuspenseQuery/page": [
    "static/chunks/webpack-570b061e254f9b2e.js",
    "static/chunks/fd9d1056-2db6654aae4e30bf.js",
    "static/chunks/472-ae7a0071c9c24f93.js",
    "static/chunks/main-app-8059d80e590c3de5.js",
    "static/chunks/521-f6286d24f3481bd1.js",
    "static/chunks/679-3e2ead1f48a6bd5f.js",
    "static/chunks/100-a5bf1a60e71cf4e8.js",
    "static/chunks/app/cc/dynamic/useSuspenseQuery/page-112ed6fcb4431919.js"
  ],
  "/cc/dynamic/useSuspenseQueryAC/page": [
    "static/chunks/webpack-570b061e254f9b2e.js",
    "static/chunks/fd9d1056-2db6654aae4e30bf.js",
    "static/chunks/472-ae7a0071c9c24f93.js",
    "static/chunks/main-app-8059d80e590c3de5.js",
    "static/chunks/521-f6286d24f3481bd1.js",
    "static/chunks/app/cc/dynamic/useSuspenseQueryAC/page-bd72f692458105ca.js"
  ]
}

Where "/cc/dynamic/useSuspenseQuery/page" uses import { useSuspenseQuery } from "@apollo/experimental-nextjs-app-support/ssr" and "/cc/dynamic/useSuspenseQueryAC/page" uses import { useSuspenseQuery } from "@apollo/client";.

Now you will notice that the import from @apollo/client indeed does not contain the two dependencies

    "static/chunks/679-3e2ead1f48a6bd5f.js",
    "static/chunks/100-a5bf1a60e71cf4e8.js",

If you look at these, 679-3e2ead1f48a6bd5f contains the @apollo/client package and 100-a5bf1a60e71cf4e8 contains @apollo/experimental-nextjs-app-support.

So... The one where we import @apollo/client directly does not contain @apollo/client. That seems weird. And it is. Until you look further, at the layout:

{
    "/cc/layout": [
      "static/chunks/webpack-570b061e254f9b2e.js",
      "static/chunks/fd9d1056-2db6654aae4e30bf.js",
      "static/chunks/472-ae7a0071c9c24f93.js",
      "static/chunks/main-app-8059d80e590c3de5.js",
      "static/chunks/db4cc85f-52d9a110b29d1dde.js",
      "static/chunks/ab91f1f5-c5e022c50fe46f19.js",
      "static/chunks/aaea2bcf-9807dd94c29a0ac5.js",
      "static/chunks/521-f6286d24f3481bd1.js",
      "static/chunks/679-3e2ead1f48a6bd5f.js", // @apollo/client
      "static/chunks/988-f8ecbd3dba53a260.js",
      "static/chunks/100-a5bf1a60e71cf4e8.js", // @apollo/experimental-nextjs-app-support
      "static/chunks/app/cc/layout-a809e480e8f29bda.js"
    ],
}

You see: loading the layout already loads those two anyways - no matter which of the two imports we actually use.

So, both of these should actually be equally-sized, until you look at the build report:

├ λ /cc/dynamic/useSuspenseQuery                       435 B           208 kB
├ λ /cc/dynamic/useSuspenseQueryAC                     5.26 kB         109 kB

So, the page /cc/dynamic/useSuspenseQuery reports to use 99kb more. But only because it counts one file twice? If you look further, though, /cc/dynamic/useSuspenseQueryAC, by importing from @apollo/client, the page size suddenly increased from 435B to 4.26kB. That's because for some reason, here the bundler starts inlining a few functions from @apollo/client - from a library that at that point in time has already loaded anyways.

So not only is the reporter broken here, but using @apollo/client instead of @apollo/experimental-nextjs-app-support actually leads into actual double-shipping of an additional 5kB of code. 🤦

phryneas commented 4 months ago

That said, it is concerning that there is no tree-shaking going on at all in the first place. I'll dig a bit more.

Strike that, it tree-shakes just fine. So I believe in the end this is just a weird choice by Next.js how they report the built page sizes, not taking into account what already shipped in the layout.

phryneas commented 4 months ago

At this point I'm actually not that sure about the tree-shaking here...

graphql should tree-shake down to 12kB (4kB gzipped) for Kind, visit, print (which is all that AC uses) or 40kB (10kB gzipped) if you also take into account parse, which is used by graphql-tag.

But if you look at this, it's 500kB (44kB gzipped) instead, which is crazy! image

phryneas commented 4 months ago

@tombryden could you give #161 a try? It should shake off a big part of the graphql package.

npm i @apollo/experimental-nextjs-app-support@0.0.0-commit-12fa821
tombryden commented 4 months ago

Many thanks for looking into this.

I have given this a quick test, this change definitely helps - still not near the size of @apollo/client though unfortunately.

Fresh project, npm run build with Next.js 14.0.4, no AC

image

Import Cost Extension (originally 76.6k gzipped so nice improvement here)

image

npm run build including useQuery from @apollo/experimental-nextjs-app-support/ssr (originally 211kB)

image image

and to compare - regular @apollo/client import of useQuery

image image

phryneas commented 4 months ago

@tombryden I believe your "import cost" calculator makes the same mistake as the Next.js build report itself here - it "forgets" that the layout imports ApolloClient etc. anyways - and while those now turn up in your page, they already have been sent to the user with the Layout - this is likely not actually additional bundle size.

Can you measure this another way, e.g. by calling the page in the browser and looking at the actually transmitted data in both cases?

tombryden commented 4 months ago

That would make sense, tbh the bundler from next.js 13 onwards seems flaky atm, there are other bugs I am following relating to it.

Here are the results of loading the page:

useQuery from @apollo/experimental-nextjs-app-support/ssr

image

useQuery from @apollo/client

image

Then I tested using dynamic importing to fully isolate the component with useQuery

Dynamic import with @apollo/experimental-nextjs-app-support/ssr

image Note _app-pages-browser_src_comps_test_tsx.js size

Dynamic import with @apollo/client

image

Very strange!

phryneas commented 4 months ago

Could it be that you just tried this in dev, not with the next build/next start artefact? The page.js in the next build variant is <1kB for me.

tombryden commented 4 months ago

Whoops... apologies

Here are the same results using build

useQuery from @apollo/experimental-nextjs-app-support/ssr

image

useQuery from @apollo/client

image

Dynamic import with @apollo/experimental-nextjs-app-support/ssr

image

Dynamic import with @apollo/client

image

Think you hit the nail on the head - looks like an issue with the bundle reporter

phryneas commented 4 months ago

Yeah :/ Unfortunate that Next reports it like that.

But it wasn't for naught - we got that optimization in #161 out of it :)

Are you okay if I close this issue though? I don't think I can really address this from our side.

tombryden commented 4 months ago

Will close it now. Many thanks for https://github.com/apollographql/apollo-client-nextjs/pull/161 and looking into this!