invalidateQueries not invalidating outgoing first request
Describe the bug
Per the docs, the default behavior of invalidateQueries should be that if a query gets invalidated while it's being fetched, then the queryFn should be called again (i.e., the query should be restarted). However, currently that's only the case if the request has already been executed in the past (so, if the current query execution is a refech). If we're talking about a query that has never been run before, then invalidating it mid-flight does nothing.
Your minimal, reproducible example
https://stackblitz.com/edit/tanstack-query-igjd7d57?file=src%2Findex.tsx
Steps to reproduce
- Open the devtools, so that you can see the console.log output
- Go to the MWE
- You'll see
>>>> query startin the console. - Before the query finishes (5 seconds), click the "Invalidate Query" button as many times as you want. You'll see nothing in the console: this is the bug, you should see
>>>> query starteach time you click it. - After the query finishes, the behavior is correct: if you click the button, and click it again, you'll see
>>>> query starteach time you click it.
Expected behavior
The query should be invalidated even if it's fetching for the first time.
How often does this bug happen?
Every time
Screenshots or Videos
No response
Platform
- OS: Macos
- Browser: Chrome
Tanstack Query adapter
react-query
TanStack Query version
5.64.1
TypeScript version
5.7.2
Additional context
No response
seems like this is on purpose - cancelRefetch is only taken into account when we have data already:
https://github.com/TanStack/query/blob/5edd617eac7d27ef4850b101f31281d6079c9de2/packages/query-core/src/query.ts#L357-L359
I guess that’s why it’s called cancelRefetch and not cancelFetch 😅 . This is the only place where cancelRefetch is checked, so I think removing the data check should do what you want. However, I think that would be a breaking change.
I wrote the docs where it says:
Per default, a currently running request will be cancelled before a new request is made
When set to false, no refetch will be made if there is already a request running.
because I thought that’s what’s happening, and I always wondered why it’s not named cancelFetch or just cancel.
Maybe the best we can do now is change the documentation to reflect what is actually happening ?
I also felt the name cancelRefetch was misleading, and it ended up being pretty accurate xD
I believe the current behavior is pretty unintuitive, and given that it also doesn't do what the docs state, I'd say changing it would be a fix and not a breaking change. But of course that's just my just opinion... if anyone relies on the "buggy" behavior, then yes, it'd be a breaking change for them 🙈 (mandatory xkcd reference ).
I wonder why that condition ( if (this.state.data !== undefined) is there in the first place, I can't think of a scenario where this would be desired. Maybe it's there to prevent the case..
- A query fires and we show a loading spinner
- Before the request finishes, we get e.g. a ws event that invalidates the request
- We invalidate the request, so that it is restarted
- Iterate 2-3 a few times
- Bad Ux: user sees the spinner for a long time
?
If that was the original intention, maybe a better strategy would be to accept the new data (instead of canceling the request) but still fire the new request in the background, as soon as "invalidateQueries" is called. 🤔
given that it also doesn't do what the docs state, I'd say changing it would be a fix and not a breaking change.
It was like that since v3, then I added those sentences to the docs somewhen last year only. I think it would also be unintuitive if the setting called cancelRefetch would cancel more than a refetch, like also canceling an initial fetch :/
maybe a better strategy would be to accept the new data (instead of canceling the request) but still fire the new request in the background, as soon as "invalidateQueries" is called.
I can also only guess the intent, but yes, I think getting data earlier is better and frequent calls to an imperative refetch would delay that. Your suggestion would mean we would have to queue up the refetch, which is something that we don’t currently do. It also sounds unrelated to cancelRefetch itself.
I think it would also be unintuitive if the setting called cancelRefetch would cancel more than a refetch, like also canceling an initial fetch :/
Yes, I totally agree there. Maybe a good middle ground would be to add a cancelInitialFetch option, which would be false by default?
Your suggestion would mean we would have to queue up the refetch, which is something that we don’t currently do.
Scrap that suggestion then, it seems it'd increase complexity a lot 😅
What’s your use-case for needing to cancel the initial fetch ?
Yes, we could add that option and then later consolidate it to a cancelFetch: boolean | 'refetchOnly' option, where the default could be 'refetchOnly', but I'd like to know why this could be a good idea :)
Sure! My use case (for invalidation in general) is the same: the BE sends ws messages to the web client, which mark that some data is stale.
To use an example from the docs, let's say github exposes an endpoint GET /{repo-id}/stars that gives you the list of stars from the repo, and also sends a ws message 'stars_count_updated'. I want to display the stars count, and always have the latest count value. So, whenever a client receives the 'stars_count_updated', it knows it needs to refresh the count: if a request is already in flight, it needs to be refetched (doesn't matter whether it's a "fetch" or a "refetch"), since the value the server is sending us in the reply might already be stale.
Let me invert the question: when would it be a good idea to cancel a refetch but not a fetch? 🤔
Would you like me to give a PR a go? Or do you prefer to ponder this a bit more first? :)
Sorry, I’m still thinking about this 😅
I ran into the same issue. My use case for this is. I have a query which has staleTime: Infinity aka it should only refetch on manual invalidation, I invalidate the query as messages come in from a network stream.
I think a good workaround would be to call queryClient.cancelQueries() manually before invalidating it if you want to really ensure that all fetches get cancelled.
I don’t think I want to add this as an option as it’s quite the edge case. In most situations, you will want data as soon as possible.
Thanks for giving it some consideration @TkDodo :) However, I'm still curious and would like to understand the rationale behind the current behavior. Don't you think that having invalidate cancel refetches but not "fetches" is unintuitive? Also, when would that be the desired behavior for the user? ("cancel this query and restart it, but only if it's a refetch, otherwise ignore the call to invalidate")
the difference is that when it’s a refetch, you already have data that can be shown to the user. While you do the initial fetch, you show the pending fallback. If you get to an invalidation while you’re doing the initial fetch, you would prolong showing the pending fallback to the user.
the only situation where this seems to matter is websocket subscriptions, where I still don’t fully understand the scenario:
- A query fires and we show a loading spinner
- Before the request finishes, we get e.g. a ws event that invalidates the request
Shouldn’t the websocket subscription only be set-up after you have fired the initial query? And if you have a global subscription, calling cancelQueries() in one place should be an acceptable workaround.
But we can think about globally changing this for v6. There won’t be any changes in v5 though, sorry.
Oh, no apologies needed Tk, I just wanted to understand your reasoning :) Makes sense, I understand now why the current behavior might be desirable, my only criticism is that I see it as unintuitive.
if you have a global subscription, calling cancelQueries() in one place should be an acceptable workaround.
yep, it's a global subscription in my case. What I don't like about that workaround is that it'd only work for the initial fetch but not for a refetch. So, I'd have to check "is X query refetching? Then invalidate. Otherwise, is it fetching for the first time? Then cancel it". If you ever want to change this, I believe a flag passed to invalidateQueries makes the most sense. It could be something like invalidateQueries(..., { invalidateOnlyIfCachedDataPresent: boolean }), where invalidateOnlyIfCachedDataPresent defaults to true to keep the current behavior as is.