Warn if `ssrMode` or `ssrForceFetchDelay` are used. #358

phryneas · 2024-09-04T12:01:35Z

I've seen this a few times too often now.

netlify · 2024-09-04T12:02:25Z

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

Name	Link
🔨 Latest commit	`197ba3e`
🔍 Latest deploy log	https://app.netlify.com/sites/apollo-client-nextjs-docmodel/deploys/6716073220d61c000845567e
😎 Deploy Preview	https://deploy-preview-358--apollo-client-nextjs-docmodel.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

phryneas · 2024-09-04T12:03:10Z

packages/client-react-streaming/src/DataTransportAbstraction/WrappedApolloClient.tsx

+    for (const warning of warnings) {
+      console.warn(warning, info.pkg, "ApolloClient");
+    }


This is down here because we need access to info, which relies on this, which is only available after the super call - but we want to remove these options before that call.

github-actions · 2024-09-04T12:04:02Z

size-limit report 📦

Path	Size
{ ApolloNextAppProvider, ApolloClient, InMemoryCache } from '@apollo/experimental-nextjs-app-support' (Browser ESM)	4.93 KB (+2.19% 🔺)
{ WrapApolloProvider, ApolloClient, InMemoryCache } from '@apollo/client-react-streaming' (Browser ESM)	1.8 KB (+6.46% 🔺)
{ buildManualDataTransport } from '@apollo/client-react-streaming/manual-transport' (Browser ESM)	3 KB (+3.55% 🔺)
@apollo/client-react-streaming (Browser ESM)	2.51 KB (+4.43% 🔺)
@apollo/client-react-streaming (SSR ESM)	7.34 KB (+1.35% 🔺)
@apollo/client-react-streaming (RSC ESM)	7.18 KB (+1.52% 🔺)
@apollo/client-react-streaming/manual-transport (Browser ESM)	3.21 KB (+3.4% 🔺)
@apollo/client-react-streaming/manual-transport (SSR ESM)	8.3 KB (+1.18% 🔺)
@apollo/experimental-nextjs-app-support (Browser ESM)	5.52 KB (+1.99% 🔺)
@apollo/experimental-nextjs-app-support (SSR ESM)	10.68 KB (+0.97% 🔺)
@apollo/experimental-nextjs-app-support (RSC ESM)	7.23 KB (+1.42% 🔺)
@apollo/experimental-nextjs-app-support/rsc (RSC ESM)	6.38 KB (0%)

relativeci · 2024-09-04T12:05:27Z

#264 Bundle Size — 1.1MiB (+0.03%).

197ba3e(current) vs e6bfd3a main#263(baseline)

Warning

Bundle contains 1 duplicate package – View duplicate packages

Bundle metrics

1 change

1 regression

	Current #264	Baseline #263
Initial JS	`924.27KiB`(`+0.04%`)	`923.9KiB`
Initial CSS	`70B`	`70B`
Cache Invalidation	`22.66%`	`22.66%`
Chunks	`33`	`33`
Assets	`57`	`57`
Modules	`588`	`588`
Duplicate Modules	`99`	`99`
Duplicate Code	`6.23%`	`6.23%`
Packages	`26`	`26`
Duplicate Packages	`1`	`1`

Bundle size by type

1 change

1 regression

	Current #264	Baseline #263
JS	`1.09MiB` (`+0.03%`)	`1.09MiB`
Other	`8.78KiB`	`8.78KiB`
CSS	`70B`	`70B`

Bundle analysis report Branch pr/warn-on-useless-options Project dashboard

^{Generated by RelativeCI Documentation Report issue}

jerelmiller

Good call on this one!

pkg-pr-new · 2024-10-21T07:49:40Z

yarn@berry undefined https://pkg.pr.new/apollographql/apollo-client-nextjs/@apollo/client-react-streaming@358

yarn@berry undefined https://pkg.pr.new/apollographql/apollo-client-nextjs/@apollo/experimental-nextjs-app-support@358

commit: 197ba3e

t-tiger · 2024-12-25T02:38:14Z

@phryneas
May I ask a question about this implementation?

Actually, this change is causing an issue with the application we are using. Specifically, with PreloadQuery, a query is first executed on the RSC, and then the same query is executed again on the client side. This is causing duplicate requests in all places where we are using useSuspenseQuery.

Until the PR is shipped, I had been able to prevent the client-side query execution after the RSC query by setting ssrMode and ssrForceFetchDelay. However, since these options have been removed, duplicate requests are now occurring.

I think there may be others facing the same issue. Do you know of any way to prevent duplicate requests between RSC and the client side? Or is there any point I might be overlooking?

phryneas · 2025-01-06T09:22:55Z

@phryneas May I ask a question about this implementation?

Actually, this change is causing an issue with the application we are using. Specifically, with PreloadQuery, a query is first executed on the RSC, and then the same query is executed again on the client side. This is causing duplicate requests in all places where we are using useSuspenseQuery.

Until the PR is shipped, I had been able to prevent the client-side query execution after the RSC query by setting ssrMode and ssrForceFetchDelay. However, since these options have been removed, duplicate requests are now occurring.

I think there may be others facing the same issue. Do you know of any way to prevent duplicate requests between RSC and the client side? Or is there any point I might be overlooking?

Hi @t-tiger,
most of the purpose of the @apollo/experimental-nextjs-app-support package is to transport that data over and prevent that duplicate network request from happening - so if you see a double network request, you have run into a bug we have to fix and ssrForceFetchDelay would only add a band-aid, hiding the real culprit here.
Could you maybe create a small reproduction of the problem and open a new issue?

t-tiger · 2025-01-22T15:35:51Z

@phryneas

I'm sorry for the delayed response. Allow me to ask the follow-up question 🙇

I think I could understand the design intent of the library. After reading this comment, I see that @apollo/experimental-nextjs-app-support is designed to make the client transparent to server-side and use cache-first for fetchPolicy.

Duplicate requests can be reproduced by setting the fetchPolicy to network-and-cache, where PreloadQuery triggers once when the page loads, and then useSuspenseQuery triggers it again.

However, my use case differs. Here are the underlying conditions:

We use both useSuspenseQuery and useQuery. Since fetchPolicy for useQuery needs to be network-and-cache, default fetchPolicy is also set to network-and-cache.
When browser back/forward occurs, server components are not re-rendered. But we need to ensure the data remains up-to-date, client-side has to refetch.

For point 1, it's difficult for developers to keep specifing cache-first for useSuspenseQuery and network-and-cache for useQuery in a large-scale development project. Therefore, I would like to unify the default fetchPolicy.

For point 2, when browser back/forward occurs, Next.js does not re-render page.tsx and stale data will be displayed if fetchPolicy is cache-first. However, due to the nature of our application, we need to refetch the data.

When these two cases combine, using ssrMode and ssrForceFetchDelay is desired. Without them, it's not possible to achieve both.

I think this kind of situation can occur, but is the design intent of the library is to use cache-first and remain transparent to server-side?

phryneas · 2025-01-22T15:52:15Z

@t-tiger

Since the fetch policy for useQuery needs to be network-and-cache

That fetch policy is the right choice only in very rare cases, I would not recommend to use it as a default, especially not without combining it with a nextFetchPolicy. It might lead to a lot more network requests than you might expect.

When browser back/forward occurs, server components are not re-rendered. But we need to ensure the data remains up-to-date, client-side has to refetch.

As a user, honestly I would be quite taken aback if I were to press the back button and get different information than I saw previously - but this is your call to make in the end.

When these two cases combine, using ssrMode and ssrForceFetchDelay is desired.

You might have gotten your desired result on accident, but you are really relying on undefined behaviour here that might change on a whim. This library is not designed to be used with these options and they are not supported.

What I read here is that you are running into problems using the hooks in certain situations with certain network policies - this seems like a XY-Problem: We are discussing ssrMode where we should actually discuss fetch policies.
We might need to adjust the hooks to enforce certain fetch policies during SSR (useQuery already is pinned to "cache-only" during SSR as it would otherwise make network requests that never wait for a result) - could you please open an issue on that topic so we can discuss that instead? (And please, let's do this in an issue and not in a PR that was closed months ago).

t-tiger · 2025-01-22T16:00:56Z

Thank you for your response. Understood. It would have been better to open an issue rather than asking in the PR. My apologies for it.

Warn if ssrMode or ssrForceFetchDelay are used.

5ca35fa

phryneas requested a review from a team as a code owner September 4, 2024 12:01

vercel bot had a problem deploying to Preview September 4, 2024 12:02 Failure

phryneas commented Sep 4, 2024

View reviewed changes

lockfile

730a35b

vercel bot deployed to Preview September 4, 2024 12:07 View deployment

fixup test

f63daf0

vercel bot deployed to Preview September 4, 2024 12:13 View deployment

jerelmiller approved these changes Sep 4, 2024

View reviewed changes

Merge branch 'main' into pr/warn-on-useless-options

197ba3e

phryneas merged commit 4834f55 into main Oct 21, 2024
23 of 24 checks passed

phryneas deleted the pr/warn-on-useless-options branch October 21, 2024 07:53

vercel bot deployed to Preview October 21, 2024 07:53 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Warn if `ssrMode` or `ssrForceFetchDelay` are used. #358

Warn if `ssrMode` or `ssrForceFetchDelay` are used. #358

phryneas commented Sep 4, 2024

netlify bot commented Sep 4, 2024 •

edited

Loading

phryneas Sep 4, 2024

github-actions bot commented Sep 4, 2024 •

edited

Loading

relativeci bot commented Sep 4, 2024 •

edited

Loading

jerelmiller left a comment

pkg-pr-new bot commented Oct 21, 2024 •

edited

Loading

t-tiger commented Dec 25, 2024

phryneas commented Jan 6, 2025

t-tiger commented Jan 22, 2025 •

edited

Loading

phryneas commented Jan 22, 2025 •

edited

Loading

t-tiger commented Jan 22, 2025

Warn if ssrMode or ssrForceFetchDelay are used. #358

Warn if ssrMode or ssrForceFetchDelay are used. #358

Conversation

phryneas commented Sep 4, 2024

netlify bot commented Sep 4, 2024 • edited Loading

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

phryneas Sep 4, 2024

Choose a reason for hiding this comment

github-actions bot commented Sep 4, 2024 • edited Loading

size-limit report 📦

relativeci bot commented Sep 4, 2024 • edited Loading

#264 Bundle Size — 1.1MiB (+0.03%).

jerelmiller left a comment

Choose a reason for hiding this comment

pkg-pr-new bot commented Oct 21, 2024 • edited Loading

t-tiger commented Dec 25, 2024

phryneas commented Jan 6, 2025

t-tiger commented Jan 22, 2025 • edited Loading

phryneas commented Jan 22, 2025 • edited Loading

t-tiger commented Jan 22, 2025

Warn if `ssrMode` or `ssrForceFetchDelay` are used. #358

Warn if `ssrMode` or `ssrForceFetchDelay` are used. #358

netlify bot commented Sep 4, 2024 •

edited

Loading

github-actions bot commented Sep 4, 2024 •

edited

Loading

relativeci bot commented Sep 4, 2024 •

edited

Loading

pkg-pr-new bot commented Oct 21, 2024 •

edited

Loading

t-tiger commented Jan 22, 2025 •

edited

Loading

phryneas commented Jan 22, 2025 •

edited

Loading