Repository navigation
invalidateQueries not invalidating outgoing first request #8530
Description
Activity
seems like this is on purpose -
cancelRefetchis only taken into account when we havedataalready:query/packages/query-core/src/query.ts
Lines 357 to 359 in 5edd617
I guess that’s why it’s called
cancelRefetchand notcancelFetch😅 . This is the only place wherecancelRefetchis checked, so I think removing thedatacheck 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
cancelFetchor justcancel.Maybe the best we can do now is change the documentation to reflect what is actually happening ?
I also felt the name
cancelRefetchwas 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
cancelRefetchwould 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
cancelRefetchitself.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
cancelInitialFetchoption, which would befalseby 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}/starsthat 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 😅
Reacted by Lucas GenzelisReacted by Lucas GenzelisI ran into the same issue. My use case for this is. I have a query which has
staleTime: Infinityaka it should only refetch on manual invalidation, I invalidate the query as messages come in from a network stream.Reacted by Lucas GenzelisI 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.
Reacted by Lucas GenzelisThanks 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
invalidatecancel 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 }), whereinvalidateOnlyIfCachedDataPresentdefaults totrueto keep the current behavior as is.
Describe the bug
Per the docs, the default behavior of
invalidateQueriesshould 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
>>>> query startin the console.>>>> query starteach time you click it.>>>> 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
Tanstack Query adapter
react-query
TanStack Query version
5.64.1
TypeScript version
5.7.2
Additional context
No response