Infinite refetching when using `usePrefetchQuery` (`ensureQueryData`) with Suspense about query HOT 6 OPEN

If you comment out the prefetch, the suspense query does settle though.

Oh yeah, the ensureQueryData is definitely involved. I think I saw the useSuspenseQuery retry 3 times before failing. What I believe is happening (but haven't verified all the way) is that the ensureQueryData just fires the request, it doesn't care about retries or anything. Then useSuspenseQuery sees there is already a fetch in flight and throws that existing promise. When that resolves, we rerender, triggering a new fetch via ensureQueryData, etc.

At least something along those lines.

from query.

Do you think it also makes sense to add an empty .catch handler to ensureQueryData to avoid unhandled promise rejections, especially in the render phase?

yes, that looks good.

The docs mention this hook eventually moving into the library, so perhaps a battle-tested version of it could be exported at some point?

Maybe at some point; for now, I think it's enough if we update the example to the version you have proposed. I think ideally, prefetching would happen on route level or in event handlers instead of during render.

Would you like to update the docs?

from query.

Thanks for the bug report and the ping, makes sense this fails and I agree we should update the docs.

The reason it fails is a bit more straightforward then mentioned above though. It's not because of an extra render because of the ErrorBoundary or anything like that, it's simply because ensureQueryData (and fetchQuery that it relies on) never throws in render and ErrorBoundary only catches errors thrown in render. The endless loop is simply from endless Suspending by the useSuspenseQuery I think.

(The throwOnError is by necessity part of the react-query package, not part of the core package where fetchQuery lives.)

I think ideally, prefetching would happen on route level or in event handlers instead of during render.

I agree with this, but I've also seen enough codebases where this is hard to implement and a (working) usePrefetch is at least a step in the right direction. 😄

from query.

yeah I think this is what happens when you think the rules of react don't apply / we can bend them. Doing something during render isn't really safe. In case of ErrorBoundaries specifically, react re-renders the component another time (I think to get the error stack trace or something). In that case, the side-effect matters, because it will trigger another fetch, thus going infinite.

To really only trigger it once, we would need to wrap the call to ensureQueryData into an if statement:

if (!queryClient.getQueryState(arg.queryKey)) {
  queryClient.ensureQueryData(arg);
}

because we need to only call it once per query, not per query data. As you've seen, in case of error, the query is created, but data isn't there yet.

I think at least, we should change the docs around this pattern, or maybe even remove it completely from the docs. @Ephem FYI

from query.

Thanks for your quick reply!

I think at least, we should change the docs around this pattern, or maybe even remove it completely from the docs. @Ephem FYI

Do you think it also makes sense to add an empty .catch handler to ensureQueryData to avoid unhandled promise rejections, especially in the render phase?

if (!queryClient.getQueryState(arg.queryKey)) {
  queryClient.ensureQueryData(arg).catch(() => {
    // Avoid unhandled promise rejection
  });
}

The docs mention this hook eventually moving into the library, so perhaps a battle-tested version of it could be exported at some point?

I think that prefetching in render is fine, e.g. https://react.dev/reference/react-dom/prefetchDNS does that, but what’s not fine is setState getting called repeatedly as part of this process somewhere 😅

from query.

The endless loop is simply from endless Suspending by the useSuspenseQuery I think.

If you comment out the prefetch, the suspense query does settle though.

from query.