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

useQuery runs twice with skip=true and ApolloNextAppProvider #437

Open
corbin-mosher opened this issue Feb 14, 2025 · 8 comments
Open

useQuery runs twice with skip=true and ApolloNextAppProvider #437

corbin-mosher opened this issue Feb 14, 2025 · 8 comments

Comments

@corbin-mosher
Copy link

Version info

{
  "react": "18.3.1",
  "react-dom": "18.3.1",
  "next": "15.1.5",
  "@apollo/client": "3.10.4",
  "@apollo/experimental-nextjs-app-support": "0.11.9"
}

Behavior

useQuery(QUERY, { skip: true }) causes a re-render after mount of any component leveraging it inside of the ApolloNextAppProvider

Expected behavior

useQuery(QUERY, { skip: true }) should not cause any additional renders after mount. This matches the behavior it has when running within the standard ApolloProvider.

Consequence

Unable to leverage ApolloClients in the NextJS app directory due to the increased cost of running large chunks of React components twice the expected amount.

Minimal reproduction

https://codesandbox.io/p/devbox/elegant-snow-x9h97v

/example = pages directory route using ApolloProvider. Renders once to mount.

/ = app directory route using ApolloNextAppProvider. Renders a second time after mount.

Screen.Recording.2025-02-14.at.1.24.50.PM.mov
@corbin-mosher corbin-mosher changed the title useQuery runs twice with skip=true useQuery runs twice with skip=true and ApolloNextAppProvider Feb 14, 2025
@corbin-mosher corbin-mosher changed the title useQuery runs twice with skip=true and ApolloNextAppProvider useQuery runs twice with skip=true and ApolloNextAppProvider Feb 14, 2025
@phryneas
Copy link
Member

This is necessary to prevent hydration mismatches and only happens on the first render of the application in the browser after it loaded - we have to first render with the exact data the component was SSRed with on the server, and then rerender with the actual data the client component would have.

Unable to leverage ApolloClients in the NextJS app directory due to the increased cost of running large chunks of React components twice the expected amount.

Generally, in React, renders are considered cheap and a single additional rerender should not make any difference to the performance of an application.
If it does make a big difference, there might be something wrong with one of your components, maybe you are making heavy computations that are not memoized somewhere?

@phryneas
Copy link
Member

That said, there might be a small optimization we can make to prevent or at least reduce reexecutions of your render functions. I'll take a look.

@phryneas
Copy link
Member

Could you give the PR build in #440 (comment) a try and report back?

@corbin-mosher
Copy link
Author

This is necessary to prevent hydration mismatches and only happens on the first render of the application in the browser after it loaded - we have to first render with the exact data the component was SSRed with on the server, and then rerender with the actual data the client component would have.

Interesting. I think technically it is possible to only re-render if the information had changed, though depending on what we are talking about here, it would come with overhead of comparing unknown data. So probably not ideal. However, I think from useQuery's perspective, this should be avoidable when we know the information couldn't have changed and a comparison wouldn't even be necessary. Like in the scenarios where:

  1. The query was skipped (because no consumer based unknown data would be getting pulled in anyways, and called/loading/data would be stable)
  2. The query was preloaded and read from in memory cache (because called/loading/data would be stable)

That was how it works in the pages directory with the prior provider in SSR scenarios. Those two methods were useful to avoid unnecessary client processing.

Generally, in React, renders are considered cheap and a single additional re-render should not make any difference to the performance of an application.

Technically this is true... and technically we can ignore it, you are correct. But it is a slippery slope. React as a whole is expensive and RSCs have their overhead as well, yet they are often reached for as a way to lower the JS quantity and execution for performance critical sites. So its just a shame if moving to the lower JS execution solution ends up causing more JS execution. Plus this is a "single" render of a majority of the components on the page.

For more flavor to the scenario, I am working on an e-commerce site that will be transitioning from the pages to the app directory. To enable compatibility of more components to be used across those two routers, we preload some historical global contexts. If those were provided initial data from the server, they are set to skip and not query new data. In the page's directory, they will be missing the initial data and need to load it. For things like authentication status, user traits, and shopping cart metadata, they are often accessed from many locations to decide what to render. Combine that with a styled-components application and the breadth of these contexts running twice on hydration is not ideal, even if technically acceptable.

Styled Components are the major culprit as to why it would take 90ms to hydrate a simple page on a high-end MacBook, but just trying to keep that cost to one initial payment rather than two.

@corbin-mosher
Copy link
Author

All that being said, I tried this out and it works as expected. Thank you, and I apologize for not attempting to contribute first. I appreciate your help.

@phryneas
Copy link
Member

phryneas commented Feb 17, 2025

I think technically it is possible to only re-render if the information had changed, though depending on what we are talking about here, it would come with overhead of comparing unknown data.

Yeah, that's what I'm doing now in #440 - it's likely not that expensive and it can save an additional reexecution of the render function (it's not really a rerender, React afaik doesn't go through a full rerender cycle here).

Like in the scenarios where:

1. The query was skipped (because no consumer based unknown data would be getting pulled in anyways, and called/loading/data would be stable)

Even if you know that the query is currently being called with skip during hydration, you don't have a 100% guarantee that it was called with the same argument during SSR.

2. The query was preloaded and read from in memory cache (because called/loading/data would be stable)

We are in streaming SSR here, so the preloaded value could have been transported over, this component is still suspended on the server, and before it finishes on the server, a user interaction already modifies the client's cache contents.

Streaming SSR is really complex here because the app is active both on the server and in the browser at the same time and data is rehydrated "chunk by chunk"


Generally, also for your use case, this is an important thing to keep in mind: you can't just transport data over once - the app will already start running in the browser before it finishes running on the server, so it's not possible to wait for all data to be transported before starting it in the browser.
It's essentially the main purpose of this library to get around those quirks, at least for Apollo Client data.

@corbin-mosher
Copy link
Author

corbin-mosher commented Feb 17, 2025

We are in streaming SSR here, so the preloaded value could have been transported over, this component is still suspended on the server, and before it finishes on the server, a user interaction already modifies the client's cache contents.

I had not considered that.

Generally, also for your use case, this is an important thing to keep in mind: you can't just transport data over once - the app will already start running in the browser before it finishes running on the server, so it's not possible to wait for all data to be transported before starting it in the browser.

In this case I am pretty sure we can know that. The way things are being preloaded in this case is related to middleware and the root layout's RSCs. The preloaded data is a prop passed into a client context provider in the root layout. So it is just serialized with the layout as the initial data for that client component. Things are definitely complex, but the Layout's server components shouldn't normally be executing again, even on route transitions.

@corbin-mosher
Copy link
Author

It's not the end of the world if at some point they did re-execute. But in practice and based on server traces, I haven't seen it occurring. They are higher level than can be affected by client-side user interactions. And the first thing to run on a page request.

This conversation helped expand how I think of these situations though... thanks for taking the time to share and fill in some gaps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants