apollo-client-nextjs icon indicating copy to clipboard operation
apollo-client-nextjs copied to clipboard

Experiment: add `disableHookValueTransportationoption` to `ManualDataTransport`

Open phryneas opened this issue 7 months ago โ€ข 5 comments

We would need feedback from someone who runs this for a while with real traffic before we can consider adding this to the library.

phryneas avatar Apr 01 '25 11:04 phryneas

๐Ÿฆ‹ Changeset detected

Latest commit: d70da047981dcfcac43d16ccc6b8487d190e83ea

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar Apr 01 '25 11:04 changeset-bot[bot]

The latest updates on your projects. Learn more about Vercel for Git โ†—๏ธŽ

Name Status Preview Updated (UTC)
apollo__client-integration-react-router โœ… Ready (Inspect) Visit Preview Jul 29, 2025 3:01pm
apollo__client-integration-tanstack-start โœ… Ready (Inspect) Visit Preview Jul 29, 2025 3:01pm
apollo__experimental-nextjs-app-support ๐Ÿ”„ Building (Inspect) Visit Preview Jul 29, 2025 3:01pm

vercel[bot] avatar Apr 01 '25 11:04 vercel[bot]

Deploy Preview for apollo-client-nextjs-docmodel ready!

Name Link
Latest commit d70da047981dcfcac43d16ccc6b8487d190e83ea
Latest deploy log https://app.netlify.com/projects/apollo-client-nextjs-docmodel/deploys/6888e10d8ab3ad0008d9e707
Deploy Preview https://deploy-preview-463--apollo-client-nextjs-docmodel.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

netlify[bot] avatar Apr 01 '25 11:04 netlify[bot]

@apollo/client-react-streaming

npm i https://pkg.pr.new/apollographql/apollo-client-integrations/@apollo/client-react-streaming@463
@apollo/experimental-nextjs-app-support

npm i https://pkg.pr.new/apollographql/apollo-client-integrations/@apollo/experimental-nextjs-app-support@463
@apollo/client-integration-nextjs

npm i https://pkg.pr.new/apollographql/apollo-client-integrations/@apollo/client-integration-nextjs@463
@apollo/client-integration-react-router

npm i https://pkg.pr.new/apollographql/apollo-client-integrations/@apollo/client-integration-react-router@463
@apollo/client-integration-tanstack-start

npm i https://pkg.pr.new/apollographql/apollo-client-integrations/@apollo/client-integration-tanstack-start@463

commit: d70da04

pkg-pr-new[bot] avatar Apr 01 '25 11:04 pkg-pr-new[bot]

size-limit report ๐Ÿ“ฆ

Path Size
{ ApolloNextAppProvider, ApolloClient, InMemoryCache } from '@apollo/client-integration-nextjs' (Browser ESM) 8.66 KB (0%)
{ WrapApolloProvider, ApolloClient, InMemoryCache } from '@apollo/client-react-streaming' (Browser ESM) 2.11 KB (0%)
{ buildManualDataTransport } from '@apollo/client-react-streaming/manual-transport' (Browser ESM) 7.83 KB (0%)
@apollo/client-react-streaming (Browser ESM) 7.4 KB (0%)
@apollo/client-react-streaming (SSR ESM) 7.66 KB (0%)
@apollo/client-react-streaming (RSC ESM) 8 KB (0%)
@apollo/client-react-streaming/manual-transport (Browser ESM) 8.07 KB (0%)
@apollo/client-react-streaming/manual-transport (SSR ESM) 8.67 KB (+0.17% ๐Ÿ”บ)
@apollo/client-integration-nextjs (Browser ESM) 9.23 KB (0%)
@apollo/client-integration-nextjs (SSR ESM) 14.2 KB (+0.31% ๐Ÿ”บ)
@apollo/client-integration-nextjs (RSC ESM) 7.78 KB (0%)

github-actions[bot] avatar Apr 01 '25 11:04 github-actions[bot]

Hello @phryneas! First off, thank you so much for all of the incredible work across the Apollo ecosystem. I was responsible for migrating a large code base from pages router to app router and you and the other maintainers' work was an absolute life-saver!

I've recently come to notice how much bandwidth the hook serialization is consuming in our applications and I'm very interested in exploring ways to cut back on that, especially considering our applications aren't massively interactive and most of the state for that interactivity is outside the Apollo Cache. WIth that said, I'm also still somewhat wary of turning off the hook synchronization entirely.

Would you be open to a more granular approach? Possibly opting types/fields in/out of hook serialization or opting specific calls to hooks out of the serialization? If one of these sounds like a good approach to you I'd be happy to put together a more concrete API proposal and potentially a PR.

TomMahle avatar Jul 08 '25 17:07 TomMahle

On a lighter-weight front, could we get this merged into the library with a name that makes it clear that it's experimental? It seems like the hardest part about getting some testing behind this would be applying these changes. We're specifically using the NextJS integration so having it surface there as an experimental feature as well would be ideal.

TomMahle avatar Jul 08 '25 22:07 TomMahle

@TomMahle you can always install the published version of the package with this PR in it from this comment:

npm i https://pkg.pr.new/apollographql/apollo-client-integrations/@apollo/client-integration-nextjs@463

As for making this even more configurable - generally I'd be open to that, but first I'd like some real-life data that this

  1. actually helps
  2. doesn't cause a lot of hydration mismatches

phryneas avatar Jul 09 '25 08:07 phryneas

apollo-client-nextjs

#565 Bundle Size โ€” 1.29MiB (~+0.01%).

d70da04(current) vs c465aff main#562(baseline)

[!WARNING] Bundle contains 1 duplicate package โ€“ View duplicate packages

Bundle metricsย ย Change 2 changes Regression 1 regression
โ€ƒโ€ƒโ€ƒโ€ƒโ€ƒโ€ƒโ€ƒโ€ƒโ€ƒโ€ƒ โ€ƒโ€ƒโ€ƒโ€ƒโ€ƒโ€ƒCurrent
#565
โ€ƒโ€ƒโ€ƒโ€ƒโ€ƒBaseline
#562
Regressionย ย Initial JS 1.03MiB(~+0.01%) 1.03MiB
No changeย ย Initial CSS 0B 0B
Changeย ย Cache Invalidation 6.97% 0.08%
No changeย ย Chunks 35 35
No changeย ย Assets 59 59
No changeย ย Modules 641 641
No changeย ย Duplicate Modules 88 88
No changeย ย Duplicate Code 4.08% 4.08%
No changeย ย Packages 25 25
No changeย ย Duplicate Packages 1 1
Bundle size by typeย ย Change 1 change Regression 1 regression
โ€ƒโ€ƒโ€ƒโ€ƒโ€ƒโ€ƒโ€ƒโ€ƒโ€ƒโ€ƒ โ€ƒโ€ƒโ€ƒโ€ƒโ€ƒโ€ƒCurrent
#565
โ€ƒโ€ƒโ€ƒโ€ƒโ€ƒBaseline
#562
Regressionย ย JS 1.28MiB (~+0.01%) 1.28MiB
No changeย ย Other 9.59KiB 9.59KiB

Bundle analysis reportโ€ƒBranch pr/disableHookValueTransportatio...โ€ƒProject dashboard


Generated by RelativeCIโ€ƒDocumentationโ€ƒReport issue

relativeci[bot] avatar Jul 09 '25 08:07 relativeci[bot]

I did some local testing of this today with a few use cases on a large and heavily streamed web app and saw HTML resource size reduce by 36%, 61%, and 44% with transferred size reducing by 24%, 61%, and 17%. These would represent substantial performance improvements for this application!

Here are those HTML document sizes expressed as [kB over the wire] / [kB resource size] for a few pages in one of our apps.

Use case Transferred HTML size (kB) HTML Resource size (kB)
Page 1 (hook transport enabled) 117 598
Page 1 (hook transport disabled) 88.7 382
Page 1 (hook transport enabled, using fragments to compose larger queries) 265 1114
Page 1 (hook transport disabled, using fragments to compose larger queries) 102 431
Page 2 (hook transport enabled) 101 614
Page 2 (hook transport disabled) 83.9 344

In terms of hydration errors I could not find any difference between hook transport enabled vs. disabled though I'll note that I'm working off of next 14 for all of these results and hitting hydration mismatches even outside Apollo state were pretty easy to reproduce with interaction while streaming. ~Tomorrow~ I'll update and perform more thorough testing of hydration, but I wanted to share the promising results with respect to HTML size!

[Edit]: I should have known better than to promise anything about timing ๐Ÿ˜… My day was eaten up by other concerns, as we all know happens often. โณ

TomMahle avatar Jul 09 '25 22:07 TomMahle

Apologies for the delay, it turns out upgrading across major versions of core dependencies in an enterprise app containing several legacy components took a while ๐Ÿ˜….

The good(?) news is that in local development and low-speed network environments the pages I work on already have easy-to-reproduce hydration errors ๐Ÿซค that are, for our purposes, identical to what would be risked by turning on this option and finishing up porting our state management into Apollo. Furthermore, these experiences have had those errors in production on a high-traffic site (akarank top 10000) for over a year ๐Ÿ˜ฎโ€๐Ÿ’จ. These are holdovers from a state-management paradigm that worked fine in pages router but has fundamental flaws in App router that we didn't know about until I was testing this option because no one has reported issues! Undoubtedly, our logging and/or monitoring needs some work, but I want to focus on that whole 'no one has reported issues even with hydration errors' side of things ๐Ÿ‘.

This is because even on React 18 and Next 14 that we have in production these do not have catastrophic consequences for UX ๐Ÿ˜Œ. You can see that here or here by throttling network and interacting with pickup/shipping on the right side.

So, while I don't have hard data for you on how many hydration errors this option is likely to cause I can still say confidently that this option is worth shipping, perhaps still with an _experimental prefix on the option name. In our use case this option enables pretty huge performance gains ๐Ÿ“ˆ and my local testing shows identical hydration errors to what we have in production today โœ….

I still think there's value in providing opt-in / opt-out capabilities on a field/type/hook-invocation basis and am happy to continue on with that conversation, but that doesn't seem to interfere with this option of specifying a default behavior. Can we get this shipped? ๐Ÿ˜

TomMahle avatar Jul 18 '25 14:07 TomMahle

For posterity I'm drawing a connection from here back to https://github.com/apollographql/apollo-client-integrations/issues/344 which is what originally got me looking down this path.

TomMahle avatar Jul 18 '25 14:07 TomMahle

For context: I'm running into a bit of a chicken-and-egg scenario where there is hesitation around utilizing this functionality in production unless it is merged into main and thus guaranteed to have an up-to-date version containing it. If we can get this merged in to main, even with something identifying it as experimental, I'll have a much easier time getting it used in prod and reporting back whatever specific metrics are desired ๐Ÿ˜„. Hopefully my findings above are enough to get past that hurdle.

TomMahle avatar Jul 18 '25 15:07 TomMahle

@phryneas Hello again! Where would you like to go from here with this? If you've just been busy or relaxing, no worries. If there's something else specific we need in order to get this merged in let's talk about what that looks like. I'd really like to get this change out to our users, I just need a way to make sure we can get them any upstream changes to this library easily and punctually as well.

TomMahle avatar Jul 26 '25 16:07 TomMahle

Hi @TomMahle, sorry for the delay in responding. Currently I'm focused on getting Apollo Client 4.0 out of the door (we're in the final documentation writing phase).

Once that's done - ETA is in 2 weeks - I'll get around to all the surrounding problems and will pick this up, too - I hope that's okay!

phryneas avatar Jul 28 '25 07:07 phryneas

Hi @TomMahle, sorry for the delay in responding. Currently I'm focused on getting Apollo Client 4.0 out of the door (we're in the final documentation writing phase).

Once that's done - ETA is in 2 weeks - I'll get around to all the surrounding problems and will pick this up, too - I hope that's okay!

@phryneas Thanks for getting back to me! That should work fine. If I could get access to be able to merge main into this PR or assurance that this will be kept up to date in case of any unexpected high priority main merges I may be able to get this out in front of some users.

TomMahle avatar Jul 28 '25 15:07 TomMahle

@TomMahle you know what? Let's just get this in in-between. I'll await for a review from @jerelmiller, but from my side tbh. this is as ready as it can be.

phryneas avatar Jul 29 '25 08:07 phryneas

@TomMahle see https://github.com/apollographql/apollo-client-integrations/releases/tag/%40apollo%2Fclient-integration-nextjs%400.12.3

phryneas avatar Jul 29 '25 15:07 phryneas

@phryneas Thank you very much!

TomMahle avatar Jul 29 '25 16:07 TomMahle