Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

useQuery does not resume fetching when re-enabling with a function for enabled and refetchInterval #8358

Closed
SMhdAsadi opened this issue Nov 27, 2024 · 4 comments

Comments

@SMhdAsadi
Copy link

Describe the bug

When using useQuery with a dynamic enabled property (passing a function) and refetchInterval, the query correctly stops when enabled returns false. However, when enabled returns true again (for example, when re-focusing a screen or toggling the state), the query does not resume executing the queryFn as expected. Logs confirm that the enabled function is called and returns true, but no further calls to the queryFn are made, and the refetchInterval appears to be ignored after re-enabling.

This behavior is problematic for scenarios such as React Native apps where queries should be disabled on inactive screens and resume when the user navigates back.

Your minimal, reproducible example

https://snack.expo.dev/@smhdasadi/reactqueryenabledrefetchintervalbug

Steps to reproduce

  1. Open project in this Expo Snack link.
  2. Observe the query repeatedly calling queryFn at the defined refetchInterval (2 seconds).
  3. Press the button to toggle enabled to false. The query stops, and no further queryFn executions are logged.
  4. Press the button again to toggle enabled back to true. Logs show that enabled is being called and returns true, but the query does not resume execution.
  5. Observe that no further executions of queryFn are triggered.

Expected behavior

When enabled toggles back to true and refetchInterval is defined:

  • The query should resume execution.
  • The queryFn should be called at the defined refetchInterval.

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

  • OS: Android, iOS
  • Platform: React Native

Tanstack Query adapter

react-query

TanStack Query version

v5.61.5

TypeScript version

v5.3.3

Additional context

No response

@TkDodo
Copy link
Collaborator

TkDodo commented Nov 27, 2024

thanks for reporting this. Here’s the same reproduction in a stackblitz with the devtools, which is easier to debug for me: https://stackblitz.com/edit/tanstack-query-rkfxvx?file=src%2Findex.tsx&preset=node

I’ll try to fix this, but also note that we’re working on a better setting to turn these things off holistically for react-native screens. Have a look at this PR:

@TkDodo
Copy link
Collaborator

TkDodo commented Nov 27, 2024

okay got it. You’re writing to a shared mutable state (config.enabled). With the enabled function, we invoke the function from the previous option with the function from the current option:

if (
mounted &&
(this.#currentQuery !== prevQuery ||
resolveEnabled(this.options.enabled, this.#currentQuery) !==
resolveEnabled(prevOptions.enabled, this.#currentQuery) ||
nextRefetchInterval !== this.#currentRefetchInterval)
) {
this.#updateRefetchInterval(nextRefetchInterval)
}

In your example, they are both reading the shard mutable state, so they both return true, so we see no difference and don’t re-start the timer.

With the function, you want to close over a value so that calling prevOptions.enabled(query) will see the old value in the closure.

I’ve fixed the sandbox: https://stackblitz.com/edit/tanstack-query-rkfxvx?file=src%2Findex.tsx&preset=node

@TkDodo TkDodo closed this as completed Nov 27, 2024
@SMhdAsadi
Copy link
Author

I really appreciate your quick response.
Thanks for your explanations, now I understand how to use enabled function. It's all about closures then. I could fix it in a simpler way by creating a new variable in the useExampleQuery hook so that the value can hold into the closure of enabled function. It works now.
https://stackblitz.com/edit/tanstack-query-cclxsl?file=src%2Findex.tsx

I’ll try to fix this, but also note that we’re working on a better setting to turn these things off holistically for react-native screens. Have a look at this PR:

#8348

Interesting. If I'm right, we should detect if current screen is focused, and if not, set subscibed to false. How it is different with current approach where we set enabled to false?

@TkDodo
Copy link
Collaborator

TkDodo commented Nov 28, 2024

Interesting. If I'm right, we should detect if current screen is focused, and if not, set subscibed to false.

Exactly 👍

How it is different with current approach where we set enabled to false?

enabled only tells us that this observer will not trigger data fetches. The component will still be kept up-to-date when new data comes in for a query, so it might get re-rendered.

To fully opt-out of re-rendering, you can currently use notifyOnChangeProps, as shown in the docs.

The newly proposed subscribed prop fixes this holistically, because the observer will be fully unsubscribed: no fetches, no renders, not shown as active in the devtools, will not count for staleTime calculations etc. It can simplify a lot of things for us internally and for consumers as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants