Redundant queries invalidation when combining useQuery with fetchQuery in v5

[izri16]

Describe the bug

Consider the below code that uses fetchQuery inside useQuery. Note that this is meant to be a minimalistic example, you can also find a link for playground here https://codesandbox.io/p/sandbox/pedantic-kapitsa-jvv78v?file=%2Fsrc%2FApp.js:

import {useQuery} from '@tanstack/react-query'

const fn2 = async () => {
  const result = Math.random()
  console.log('FN_2', result)
  return result
}

const fn1 = async () => {
  const result = Math.random()
  console.log('FN_1', result)

  await queryClient.fetchQuery({
    queryKey: ['fn_2'],
    queryFn: fn2,
    staleTime: Infinity,
  })

  return result
}

export function main() {
  useQuery({
    queryKey: ['fn_1'],
    queryFn: fn1,
    staleTime: Infinity,
  })

  useQuery({
    queryKey: ['fn_2'],
    queryFn: fn2,
    staleTime: Infinity,
  })

  const refetch = async () => {
    await queryClient.invalidateQueries()
  }

  return // some UI when `refetch` can be called, e.g. on click
}

Now the problem is that when refetch is called and invalidation happens the following is being logged:

FN_1
FN_2
FN_2
FN_1

Though, given the example uses staleTime: Infinity and the react-query is meant to guarantee that duplicate requests are not fired I would expect to see this sequence instead:

FN_1
FN_2

or

FN_1
FN_2
FN_2

Even if react-query runs separate fetch for fn2, one for fetchQuery, and one for useQuery, this does not explain why fn1 is being called two times.

We noticed this behavior once updating from v3. Though it seems to go away when using cancelRefetch: false it still seems incorrect even with cancelRefetch: true. The motivation for using fetchQuery inside useQuery callback is that we often use it to reuse other query functions from imperative code and also cache their values at the same time, while avoiding duplicate requests.

Could you please confirm whether is this behavior expected and if yes point to some docs or explain why, or confirm that its a bug?

Thanks a lot.

Your minimal, reproducible example

https://codesandbox.io/p/sandbox/pedantic-kapitsa-jvv78v?file=%2Fsrc%2FApp.js

Steps to reproduce

1. Open link to sandbox
2. Open sandbox console
3. Clear sandbox console
4. Click on "Invalidate queries" button
5. You should see 4 console logs like this:

FN_1 0.49671699048416773
FN_2 0.34992822118130174
FN_2 0.3922700001128352
FN_1 0.16330643108671228

Expected behavior

I would expect to only see FN_1, FN_2 logs in the console or FN_1, FN_2, FN_2.

How often does this bug happen?

Every time

Screenshots or Videos

[

Screen.Recording.2024-03-17.at.11.33.42.mov

](url)

Platform

OS - [macOS]
browser - Chrome
version - 122.0.6261.112

Tanstack Query adapter

None

TanStack Query version

5.27.3

TypeScript version

No response

Additional context

No response

[TkDodo]

interesting. deduplication happens correctly when you use queryClient.refetchQueries instead of invalidateQueries. The only difference between the two is that invalidateQueries also marks the queries as stale. I won't have time in the near future to look into this, so any help is wanted here.

[refi93]

I made a snippet with more logs which shows that upon invalidation, the query fn2 fetched within fn1 throws becausefn2 is considered to be conflicting due to cancelRefetch=true. This error then naturally results in a repeated fetch of fn1 at the end. See logs:

invalidating...
FN_1 start 0.13452719488171527
FN_2 0.4446655762537788
FN_2 0.1959132410169293
fn_2 error {revert: , silent: true}
FN_1 start 0.7546947324390321
fn_2 result within fn_1 0.1959132410169293
FN_1 end 0.7546947324390321

So all in all, I guess the dependent queries just don't play well with the cancelRefetch flag being true.

I'm wondering - wouldn't it be a more sensible/intuitive behavior of the cancelRefetch flag to first cancel all loading queries and then proceed to refetch all queries without trying to cancel any more queries? That should prevent any similar, arguably counterintuitive issues with invalidation of dependent queries

[TkDodo]

wouldn't it be a more sensible/intuitive behavior of the cancelRefetch flag to first cancel all loading queries and then proceed to refetch all queries without trying to cancel any more queries?

Isn't that what it is doing ? E.g.:

queryClient.invalidateQueries(['todos'], { cancelRefetch: true })

will, for each matching query, cancel it if it already fetches and re-start a fetch.

I'm also curious why this doesn't happen when calling refetchQueries directly, because at the end, invalidateQueries also calls refetchQueries:

query/packages/query-core/src/queryClient.ts

Line 294 in e34bf4d

return this.refetchQueries(refetchFilters, options)

[refi93]

Isn't that what it is doing ? E.g.:
queryClient.invalidateQueries(['todos'], { cancelRefetch: true })

I don't think so - the problem with the current implementation of invalidateQueries() (as I understand it) is that it may cancel (and fail/retry) queries triggered even after query.invalidate() was called for all queries in cache due to the asynchronous nature of query refetching, kind of a "race condition"

My understanding of what goes on with the snippet upon running queryClient.invalidateQueries({}, { cancelRefetch: true }), showcasing the problem:

1. queryClient.invalidateQueries({}, { cancelRefetch: true }) invalidates both "fn1" and "fn2" query
2. "fn1" and "fn2" are scheduled to be refetched with {cancelRefetch: true} setting
3. query "fn1" is refetched, which triggers refetching of invalidated "fn2" query
4. query "fn2" is refetched again due to step 2 (with {cancelRefetch: true} flag) but it is aborted because the "fn2" query from step 3 is still loading

I'm also curious why this doesn't happen when calling refetchQueries directly, because at the end, invalidateQueries also calls refetchQueries

My understanding is that cancelRefetch flag applies only to actual fetches (i.e. invocations of queryFn), but because the queries are not invalidated, the queryClient.fetchQuery() call within fn1 grabs the data of the fn2 query just from the cache (as can be seen here) and the refetch of fn2 triggered by the refetchQueries({}, {cancelRefetch: false}) call has no reason to fail

does it make sense?

[TkDodo]

yeah this makes sense, thanks for looking into it. Because all queries are invalidated upfront, both fn1 and fn2 are invalid by the time fn1 refetches. Then, it will also refetch fn2 from within its queryFn.

let's take a step back. What's the reason for using queryClient.fetchQuery for fn2 inside the queryFn of fn1 ?

[refi93]

let's take a step back. What's the reason for using queryClient.fetchQuery for fn2 inside the queryFn of fn1 ?

The snippet could be probably modified to use select avoiding the problem altogether, but in practice (i.e. our project) we heavily rely on fetching queries within queries to combine results from multiple simpler queries. Previously we've been combining the queries through the useMemo() hook, but it turned out to have pretty bad impact on performance, especially for queries used in many components, because useMemo() is cached only within each individual component where it is used (as opposed to the query cache which is global)

[TkDodo]

we heavily rely on fetching queries within queries to combine results from multiple simpler queries.

my recommendation would be to switch to useQueries and use combine in v5 to combine it into a single result.

The problem with calling fetchQuery inside the queryFn is that you won't have an active observer for that query. So if data for fn2 changes (e.g. by calling setQueryData on it or explicitly refetching only fn2, but not fn1), your component that is is subscribed only to fn1 will not update accordingly. useQueries avoids that problem because you get one observer per query; they are still cached separately, and your component will always show the correct state.

[refi93]

my recommendation would be to switch to useQueries and use combine in v5 to combine it into a single result.

The problem with calling fetchQuery inside the queryFn is that you won't have an active observer for that query. So if data for fn2 changes (e.g. by calling setQueryData on it or explicitly refetching only fn2, but not fn1), your component that is is subscribed only to fn1 will not update accordingly. useQueries avoids that problem because you get one observer per query; they are still cached separately, and your component will always show the correct state.

Thanks for the suggestion. We are aware of this implication and we are solving (to a good enough extent for us) the issue of inactive observers by using staleTime: Infinity and invalidating such queries at once.

useQueries() and the combine option introduced in v5 (to which we updated just recently) seems promising and could simplify/improve at least some of our queries though I suspect it may not be usable for cases where we are combining data from (sequentially) dependent queries as the API of useQueries() seems to be meant for mutually independent queries only, or do you perhaps have some recommendation/example even for such cases?

[TkDodo]

the issue of inactive observers by using staleTime: Infinity and invalidating such queries at once.

inactive observers are garbage collected after gcTime has elapsed, so you might lose data for fn2 before you want to.

I suspect it may not be usable for cases where we are combining data from (sequentially) dependent queries as the API of useQueries() seems to be meant for mutually independent queries only

that's a good point. You can set enabled manually for each entry, but probably not by depending on another value returned from useQueries.

---

if you don't want to change anything, I think the best move is to go from invalidateQueries to refetchQueries and pass type: 'active' to it.

[refi93]

if you don't want to change anything, I think the best move is to go from invalidateQueries to refetchQueries and pass type: 'active' to it.

yes, that's kind of what we do already, just by setting cancelRefetch: false instead and we can live with that workaround.

Nevertheless, I believe that the current behavior of invalidateQueries() with cancelRefetch: true flag is broken or at least counterintuitive in the sense that it potentially cancels also queries triggered after the invalidation was performed as shown in the examples we provided, and I'd advocate for having that fixed