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

Feedback for React query preloading feature #11519

Closed
jerelmiller opened this issue Jan 24, 2024 · 15 comments
Closed

Feedback for React query preloading feature #11519

jerelmiller opened this issue Jan 24, 2024 · 15 comments

Comments

@jerelmiller
Copy link
Member

Starting with Apollo Client 3.9.0, you can begin preloading queries outside React using the createQueryPreloader/preloadQuery APIs. We've marked these as experimental for this release to allow room for feedback and changes before we stabilize this API in 3.10.0.

If you would like to provide feedback or suggestions for improvement, please drop a comment below!

@jerelmiller jerelmiller pinned this issue Jan 24, 2024
@jerelmiller jerelmiller changed the title Feedback for React query preloading API Feedback for React query preloading Jan 24, 2024
@jerelmiller jerelmiller changed the title Feedback for React query preloading Feedback for React query preloading feature Jan 26, 2024
@WonderPanda
Copy link

Really excited for this change and what it means for integration with routing frameworks! Its awesome to see how much Apollo Client has been continuing to grow and improve

@jerelmiller
Copy link
Member Author

@WonderPanda thanks so much for the kind comments!

@SteveGT96
Copy link

This feature is really great. Thank you very much for the work you're doing. I can't wait for the version 3.10.0.

@jerelmiller
Copy link
Member Author

@SteveGT96 thanks so much!

@benrbrook
Copy link

benrbrook commented Mar 24, 2024

This is quite cool, thank you! Is the MockedProvider still intended to work with these features? From my limited testing it seems like the loader attempts to actually make a network request instead of using my mocks. Is there a recommended way to mock this data? Thanks!

@kobinarth-panchalingam
Copy link

Adding this feature to the React Apollo Client would simplify unnecessary conditional rendering logic in React, making development more efficient and code easier to manage.

@jerelmiller
Copy link
Member Author

jerelmiller commented Mar 26, 2024

@benrbrook great question! This won't work with MockedProvider because the request isn't kicked off inside React, so there is no access to React context. Instead, you'll want to make more use of MockLink (which MockedProvider uses) and pass that client to createQueryPreloader:

import { MockLink } from '@apollo/client/testing';

const mocks = [...] // The same structure you pass to MockedProvider

// Use `MockLink` directly
const link = new MockLink(mocks);

const client = new ApolloClient({ 
  cache: new InMemoryCache(),
  link
});

const preloadQuery = createQueryPreloader(client);
// This preloadQuery function will use the mocks in `MockLink` instead

We should have something in the docs about this though. Thanks for calling this out!

Edit: I've opened #11733 to track this.

@jerelmiller
Copy link
Member Author

Hey all 👋

We've just released v3.10 that stabilizes this API. As such, I'm going to go ahead and close out this issue. Thanks so much for all those that tried the API and provided feedback! I'm hoping you enjoy the new APIs!

Copy link
Contributor

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.

@misha-erm
Copy link

@jerelmiller sorry for posting in closed issue but this seems like a good place for asking some questions

I like the preloadQuery API but a couple of things bothers me:

  1. How do you supposed to do subscriptions with preloaded queries? Seems like it doesn't have subscribeToMore so I assume useSubscription is the right option here? (also posted this in community)
  2. Not very clear how to set up polling for such queries. Am I supposed to use useQuery somewhere down the tree or just setup it with refetch and timeouts in useEffect

Looks like the most of my questions become obsolete if I just use useQuery instead of useReadQuery but not sure if that's the right way to do it

Thanks in advance for any tips

@misha-erm
Copy link

One more thing I noticed:

When I use fetchPolicy: "cache-and-network" all my preload requests except the first one get cancelled when navigating away and back to the page. After digging a while I figured out that it's caused by react's StrictMode. Since it calls effects twice cleanup function gets triggered and aborts the active request. New one is never made.
Could be connected to #11815

@alessbell alessbell unpinned this issue Apr 29, 2024
@jerelmiller
Copy link
Member Author

Hey @misha-erm for the strict mode issue, could you try the snapshot release from #11821 and see if it fixes your issue?

npm i @apollo/client@0.0.0-pr-11821-20240501000021

If not, would you be open to creating a new issue with a reproduction so we can track this?


As for your other questions:

It looks like your first question was answered in the forum, so I'll leave that there.

Not very clear how to set up polling for such queries. Am I supposed to use useQuery somewhere down the tree or just setup it with refetch and timeouts in useEffect

We currently don't support polling with Suspense. We've discussed this a couple times as a team and we're all in agreement that we'd like to avoid it for now. Polling gets a bit strange with Suspense and React transitions and we think there could be a lot of confusion on how it should work.

As you mentioned, you should be able to use a combination of refetch with timeouts to make this work. This gives you the most control over the experience. Keep in mind that you might need to wrap the refetch with startTransition if you want to avoid showing the Suspense fallback during the network request. If you're using preloadQuery + useReadQuery, you can get refetch with the useQueryRefHandlers hook. See the doc on refetch with query preloading in the docs.

Looks like the most of my questions become obsolete if I just use useQuery instead of useReadQuery but not sure if that's the right way to do it

Just a note: preloadQuery is a suspense API, and useQuery isn't. While you might get these to work together, what you're relying on here is the fact that preloadQuery writes to the cache and useQuery would be able to pick that up before it tries to execute the network request on its own (though I suspense query deduplication would kick in here if it did try the request). These weren't really meant to work together so anything that does work is sorta coincidence (though I use this term loosely). If you're not planning on using useReadQuery, preloadQuery isn't going to be much benefit to you and you'd likely be better off just calling client.query(...) yourself, since it would result in about the same thing.

That being said, we've heard the sentiment about polling with Suspense, so we'll keep watch for the demand here. If we get a lot of asks for it, its certainly something we can explore.

Hope this helps!

@misha-erm
Copy link

@jerelmiller just checked the patch, seems to work fine 👍🏻 However I still see some random cancellations of subscriptions but they are rare and I see them without StrictMode as well. Maybe will dig deeper later and create a new issue if needed. Thank you

BTW, I decided to opt-out from strict mode after spending a day trying to understand the strange behaviour of our queries in prod env comparing to dev. Turned out I was facing this bug #11772 but it was not reproducible with StrictMode )))

About the preload stuff::

For most of my cases preloadQuery + useReadQuery works awesome but I have one page with virtualized lists where suspense doesn't work very well. Do I understand correct that by using client.query in my react-router loader + useQuery the only difference is Suspense and I still get the same benefits for network requests?

@jerelmiller
Copy link
Member Author

I have one page with virtualized lists where suspense doesn't work very well

If you're willing to go more in detail, I'd be curious about this. How would this differ from how you'd do this with e.g. useQuery?

Do I understand correct that by using client.query in my react-router loader + useQuery the only difference is Suspense and I still get the same benefits for network requests?

Short answer is: its not quite as simple as that 😆. Suspense is not the only difference, but its one of the big ones. You are right that you do get some of the same benefits for network requests, but I'll add the caveat that it requires a bit more careful management on your end to ensure you get this benefit. Its possible to accidentally introduce more than one network request with this approach since these queries aren't connected to each other. This problem compounds a bit depending on how you're using client.query inside of loader since the UX is different depending on whether you use await inside the loader or use React Router's defer utility.

Let's take an example to show you what I mean.

const loader = async ({ params }) => {
  return {
    post: await client.query({ query: GET_POST, variables: { id: params.postId } })
  }
}

const RouteComponent = () => {
  const { postId } = useParams()
  const { data, loading } = useQuery(GET_POST, { variables: { id: postId } });

  // ...
}

There are a couple things to note here:

The obvious one is that you're having to pass the query and variables in 2 places. This ensures that your useQuery will use the cached result from the client.query in your loader. If you forget to pass variables to useQuery, or you pass the wrong value, you end up making another network request to load the data, which erases the benefit of loading the data in loader.

You'll also notice that this example doesn't use data from your loader at all. I returned a value in this example, but I might as well not have since I'm not using a useLoaderData inside my route components (I'm actually not sure if React Router errors if you don't return a value, I suspect it does). And you don't really want to use the data from useLoaderData anyways since doing so means you'd miss out on cache updates (hence why you're using useQuery).

One of the big things here though is the await keyword inside that loader function. This actually has a big impact on your UX and the timing of when loader finishes relative to when your RouteComponent begins rendering. The await keyword ensures that the data is fully loaded by the time RouteComponent renders, which means the data is guaranteed to be in the cache (which is good since you want useQuery to use that cached result). On the flip-side, this tells React Router to wait to transition the page until the data is loaded, so the user will actually see the previous page for a short time after the click until the data is finished loading. This may or may not be the desired UX for your app.

If you prefer to transition the page immediately, you'd need to remove that await keyword and use defer instead. This however means that your loader will return and your RouteComponent will begin rendering before client.query has finished loading. There is no guarantee that data will be available in the cache before useQuery executes, hence the risk that you might send off a 2nd network request, though query deduplication should kick in to prevent this. Using useQuery here also means that you're having to manage the loading state inside your component. useReadQuery guarantees the data will be available after the hook resulting in a bit nicer DX.

Hopefully you're seeing how this makes the process a bit more complicated. If you find something that works for you, by all means, go for it. We can't stop you from using it. Just wanted to communicate the risks here so you can make an informed decision on how best to accomplish what you're after.

@misha-erm
Copy link

thank you for such a detailed answer 🙇🏻

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

No branches or pull requests

6 participants