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

Allow passing in a client object to the useFragment hook #407

Closed
adamfortuna opened this issue Oct 24, 2023 · 2 comments
Closed

Allow passing in a client object to the useFragment hook #407

adamfortuna opened this issue Oct 24, 2023 · 2 comments
Labels
🪝 react hooks Feature requests related to React hooks

Comments

@adamfortuna
Copy link

adamfortuna commented Oct 24, 2023

I've loved using Apollo client's new useFragment hook these last few months! This feature request is about improving the flexibility (and possible performance) when using that feature.

I'm currently in the process of trying to optimize a site for SEO where most pages don't have GraphQL queries that are being run unless the user is logged in. I'm using the new Apollo Provider from @apollo/experimental-nextjs-app-support for this:

"use client";

import { ApolloNextAppProvider } from "@apollo/experimental-nextjs-app-support/ssr";
import { getClient } from "lib/apollo/client";

export default function Providers({
  children,
}: {
  children: React.ReactNode;
}): JSX.Element {
  return <ApolloNextAppProvider makeClient={getClient}>{children}</ApolloNextAppProvider>;
}

My getClient function matches the example in the docs.

Everything works, however this ends up negatively impacting the performance of the page. Loading just this part alone ends up being a long running task according to Google DevTools performance analysis.

Screenshot 2023-10-24 at 4 40 29 PM

This results in a lower Google PageSpeed Index, and lower SEO. Every little bit helps, but I'm not sure on other ways to optimize this aside from dropping the provider approach altogether. If anyone has alternative solutions, I'd love to hear them too.

Before useFragment, the solution would've been to remove the provider and just pass in the client to useQuery everywhere while maintaining my own singleton for the request. That's more work than the provider, but it's doable.

Right now useFragment uses the useApolloClient() hook to get the current client (and cache) from the Provider same as useQuery, useLazyQuery and useMutation. However, all of those hooks take in an optional client parameter, or use the client from the Provider if not provided.

I'd like to request the useFragment hook have an optional client argument that could be passed in similar to useQuery, useLazyQuery and useMutation? That would allow for dropping the global provider when using useFragment.

@phryneas
Copy link
Member

phryneas commented Oct 25, 2023

That won't solve your initial problem - in Next.js you have to create your Apollo Client inside of a component.

Your "use client" components will also render on the server - and if your client is some kind of global, the same client instance would be shared by all requests rendering at the same time, potentially leaking private user information between multiple concurrent users.

So, the ApolloClient instance has to be created inside of a component and passed down using context.
Also, the ApolloNextAppProvider does a lot more than just being a context provider. It coordinates transporting values downloaded during your SSR render to the browser, so you don't do every network request twice.

That said, I don't believe the creation of ApolloClient should take 40ms. Could you share that profiler recording?

@jerelmiller jerelmiller added the 🪝 react hooks Feature requests related to React hooks label Oct 25, 2023
@adamfortuna
Copy link
Author

Hmm, the ApolloNextAppProvider is doing a lot more than I thought! The handoff between server and client isn't one I have a full grasp of yet. I thought that this would improve my worst pages performance, but in production this ended up being a lot faster (10ms!). I'm going to close this one out actually, because I don't think this is going to be much help for the way I thought it was. Thanks for the response @phryneas !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪝 react hooks Feature requests related to React hooks
Projects
None yet
Development

No branches or pull requests

3 participants