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.72k stars 2.93k forks source link

useQuery is not reactive to its queryClient parameter #8256

Closed debel27 closed 1 week ago

debel27 commented 3 weeks ago

Describe the bug

When calling useQuery(opts, queryClient), the hook will (correctly) use the provided queryClient argument as the query client.

However, when calling the hook again with a different QueryClient instance, the hook will keep using the initial query client instance.

Your minimal, reproducible example

https://codesandbox.io/p/sandbox/boring-ellis-rk8smt?workspaceId=1f057848-e245-4f2a-94be-0866c95684db

Steps to reproduce

See the codesanbox for a reproducing example.

In a nutshell, it boils down to

const [queryClient, setQueryClient] = useState(initialQueryClient);

// ...

/* when `queryClient` gets updated to `newQueryClient`, 
 * the hook keeps using `initialQueryClient` internally */
useQuery(opts, queryClient);

// ...

const handleQueryClientChange(newQueryClient) {
  setQueryClient(newQueryClient);
}

Expected behavior

I expected useQuery to be reactive to changes in its queryClient parameter.

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

Tanstack Query adapter

None

TanStack Query version

v5.55.0

TypeScript version

v5.1.6

Additional context

As a workaround, we are setting a key prop to the React component that calls useQuery. The key is updated whenever the queryClient parameter changes. This makes useQuery reset and use the new query client.

This workaround is not ideal for us, because it forces us to "lift the state up", breaking the abstraction of the component.

TkDodo commented 2 weeks ago

yeah I don’t think we really support swapping out the queryClient on the fly. What would be a use-case for that?

debel27 commented 2 weeks ago

I understand how swapping out the queryClient can look odd. This is certainly an advanced use case.

The short answer is that our queryClient is tied to an external "store" instance, like so:

function createQueryClient(store) {
  const queryCache = new QueryCache({
    onSuccess(data) {
      // logic that puts data in `store`
    },
  });
  return new QueryClient({ queryCache });
}

You might be wondering what the purpose of this store instance is, considering that TanStack Query has its own cache already. I can elaborate on that, if you find the topic relevant.

Also, our store instance can get re-created in a subsequent React render. We must therefore call createQueryClient(store) again whenever that happens, in other to get a queryClient that now refers to the new store.

However, since useQuery ignores queryClient updates, we are in a situation where our new store instance remains empty, because the initial queryClient instance is linked to the initial store instance, which is now obsolete.

Tricks like storeRef.current inside onSuccess will not cut it, because we also need to purge the queryCache entirely when the store changes. The store and the queryClient really are tied together.

Again, I am aware this must appear as an unusual use case. Let me know if you need any more information.

debel27 commented 1 week ago

@TkDodo would it be OK if I created a PR for this?

The PR would fix usages of ref and state that assume queryClient will never change. Here is an example to illustrate.

Before

const [observer] = React.useState(
  () => new Observer(client, defaultedOptions),
)

After

Following React recommendation to re-set the state during render:

function useObserver(client, options) {
  const [previousClient, setPreviousClient] = React.useState(client)
  const [observer, setObserver] = React.useState(
    () => new Observer(client, options)
  );

  if (client !== previousClient) {
    setPreviousClient(client)
    const newObserver = new Observer(client, options)
    setObserver(newObserver)
    return newObserver
  }

  return observer
}

We cannot use useMemo in this example because we do not want observer to re-set when defaultedOptions changes

TkDodo commented 1 week ago

I don’t think it’s that easy because this doesn’t unsubscribe the previous observer, so it will still stick around with a reference to the old queryClient. I don’t think this is something I’d want to support. Setting a key on the QueryClientProvider for when you swap out the client seems like the best workaround imo.