TanStack / query

🤖 Powerful asynchronous state management, server-state utilities and data fetching for the web. TS/JS, React Query, Solid Query, Svelte Query and Vue Query.
https://tanstack.com/query
MIT License
42.36k stars 2.89k forks source link

Disabled queries (enabled: false) get affected by invalidateQueries (refetchType: "all") when they are inactive (not being rendered anymore) #8118

Closed dsumer closed 3 weeks ago

dsumer commented 1 month ago

Describe the bug

This is similar to an old bug: https://github.com/TanStack/query/issues/3642

But now this only happens, if the disabled query isn't actively rendered anymore and TanStack Query still has it in the cache.

Then the query will be executed even though it was never enabled.

Your minimal, reproducible example

https://codesandbox.io/p/sandbox/brave-khayyam-62yx75

Steps to reproduce

In the CodeSandbox you have a button to invaldate the Query and another button to toggle if the query is being actively rendered.

If it's not actively being rendered, you can see that the queries get fetched on invalidation even though it was always disabled.

Expected behavior

Disabled queries shouldn't be fetched on invalidateQueries when they never were enabled in the first place.

How often does this bug happen?

Every time

Screenshots or Videos

https://share.cleanshot.com/Msh4Bk4q

Platform

Tanstack Query adapter

react-query

TanStack Query version

v5.56.2

TypeScript version

No response

Additional context

No response

TkDodo commented 1 month ago

oh wow. problem is that enabled is an observer level property. As soon as there is no observer anymore, a query cannot be “enabled” or “disabled”. This isn’t a problem if you use the standard invalidation, but if you bypass it to refetch everything, this will hit too.

wild that this was never reported before. need to think about what we can do here (if anything) because a query cannot (per definition) be enabled/disabled - only an observer can.

dsumer commented 1 month ago

ah I see! .. my initial thought was: why put a query into the cache when it wasn't enabled in the first place?

there are probably arguments against this, which I'm not thinking about atm, but wouldn't it be okay to not even keep disabled queries in the cache (because they weren't even executed) .. which would then also fix this issue?

TkDodo commented 1 month ago

While most cases will have a query either enabled or disabled, you can temporary enable / disable a query. For example, you can have a list query that you want to disable while a dialog is open so that it doesn’t refetch until the dialog is closed. So it’s not that black & white.

However, I think the answer is simply skipToken. When enabled is passed, we lose that information once the observer unmounts. But skipToken is passed in place of the queryFn. So I think we can consider a query do be disabled if it has a queryFn that is equal to skipToken. This check would need to go here:

https://github.com/TanStack/query/blob/50315acbb39d0c18dbb8343bfd2e13c1ac588b6f/packages/query-core/src/query.ts#L258-L260

would you like to maybe contribute that + a test case?

DogPawHat commented 1 month ago

Would this get closer to a deprecation of enabled in favor of skipToken?

TkDodo commented 1 month ago

Would this get closer to a deprecation of enabled in favor of skipToken?

yeah maybe. I’ll think about it

dsumer commented 3 weeks ago

wow, thank you very much for the super fast fix @TkDodo ❤️