Define when suspense queries should be refetched and when they should throw

[trevorwhealy]

Describe the bug
When a user does client-side navigation to return to a previously visited page (react-router-dom), the useQuery hook is not invoked again, even if the query has been marked as invalid. This is an issue because the server value could change in a different browser session or device and the useQuery data value would no longer be in sync.

To Reproduce
Created a reproducible environment for testing the bug here:
https://codesandbox.io/s/ping-pong-react-query-client-guqhn
Important files: src/index.js and src/components/Demo.js

Steps to reproduce the behavior:

1. Visit Page 1, note the number of pings, and click the increment button.
2. In the mutation's onSuccess handler, we invalidate the queryCache and navigate to Page 2.
3. Open a new tab and simulate the ping mutation by simply hitting this URL a few times:
   -- https://1675g.sse.codesandbox.io/ping-increase
4. Navigate back to Page 1 via Link or browser back button and note that the value is only incremented by 1.
5. If you refresh Page 1, you'll notice that the value now reflects the actual server value.

Expected behavior
On return to Page 1 from Page 2, I would expect the useQuery hook to fetch the latest server value.

[tannerlinsley]

Can't remember what I changed, but is this example working for you as expected?

https://codesandbox.io/s/ping-pong-react-query-client-forked-pbc4e?file=/src/components/Demo.js

[trevorwhealy]

@tannerlinsley thanks for the really quick reply! I had the same problem - maybe I'm not doing a good job of illustrating.

When I navigate back to Page 1 from Page 2, you'll notice that the Query stays in an Inactive state:

I was expecting it to re-fetch this query when the component re-mounts.

[trevorwhealy]

I think that would require the query to suspend if it is in the isFetching state in addition to the isLoading state

[boschni]

Think there are two issues here:

1. Devtools is not notified when observers unsubscribe (if you reselect the query then it shows the correct state).
2. There is some specific code to prevent refetching when suspense is enabled. This is probably to prevent double fetches because the default stale time is 0 and the hook does not know if it is rendering right after a suspense or after some page switch. Unfortunately this means suspended queries are never refetched. Is my assumption correct @tannerlinsley ?

[tannerlinsley]

Yep I think you’re right on both counts Tanner Linsley

…

On Sep 19, 2020, 11:17 AM -0600, Niek Bosch ***@***.***>, wrote: Think there are two issues here: 1. Devtools is not notified when observers unsubscribe (if you reselect the query then it shows the correct state). 2. There is some specific code to prevent refetching when suspense is enabled. This is probably to prevent double fetches because the default stale time is 0 and the hook does not know if it is rendering right after a suspense or after some page switch. Unfortunately this means suspended queries are never refetched. Is my assumption correct @tannerlinsley ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[boschni]

I think we need to define the behavior when using suspense. Currently it will only trigger suspense when it is loading for the first time. Any additional mounts will not trigger a refetch, which does not sound desirable. Maybe we should set a default stale time when suspense is enabled? Then the query will at least be refetched again if it re-mounts after some time. Then the other question is, when do we want to trigger suspense? Only on first load, or also on refetches? Or does it need to be configureable?

[flybayer]

I'm pretty sure that once useEffect() fires, you know for sure that it has fully mounted. From my understanding, useEffect will not be called multiple times during suspense rendering/re-rendering. It only triggers once the render has fully finished.

[boschni]

The main problem is that when a component is mounted, it starts a fetch, and because of the way suspense works, the component immediately gets unmounted. When the fetch is complete, the component will mount again, but because the default stale time is 0, it will trigger another fetch because the data is already considered stale. The double fetching is something we want to avoid.

Currently I can think of these solutions:

1. Mention in the docs that when using suspense, it is recommended to also set a staleTime to prevent double fetches.
2. Set some default stale time when suspense is enabled and staleTime is not set.
3. Require users to assign some application wide unique ID to a query which can be used to track a specific query.

Besides this issue, if users also want to use suspend when refetching, maybe we should add a suspendOnRefetch option?

[tannerlinsley]

#1 sounds like the fastest way forward for now.
#2 is interesting, but finding a default that would work for everyone would be difficult given all of the async rendering happening with suspense.
#3 would definitely solve it, but that DX sounds pretty lame :(

In previous versions of RQ, we had some mutable properties on the query itself that would allow observers to detect if the last fetch was because of a suspension, and thus not trigger the fetch again on mount. I wonder if that same idea could work better now that the observers have become a bit smarter.

[Alexandredc]

Just found out this issue. I'm working on react-native project with react-query. Adding stale time doesn't trigger a new invokation of useQuery when a component is already suspended once :/

[nihgwu]

@tannerlinsley any plan to fix this issue, I think this issue just block the use of suspense with react-query, as the query will never invalidate, that breaks the SWR(stale while revalidate) which is the biggest benefit of react-query

think about it, you have a query in one page, and then navigate to another page, the query should get invalidate and send a request again, but it doesn't when suspense enabled

[flybayer]

Our current workaround in Blitz is to call invalidateQueries on page navigation events.

[boschni]

Is anyone still having trouble with suspense in v3?

[flybayer]

I haven't had a chance to try v3 yet

[nihgwu]

I tried it's fixed in V3

[nghiepdev]

@boschni
I had tried 3.11.0 but the suspense issue still again #1582

[rickyalmeidadev]

I faced the issue #1582 in version 3.17.0.

I built a simple demo in Code Sandbox where it is possible to reproduce it. Any plans to have it fixed?

[TkDodo]

Is this the same as #2367?