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

combine stable reference is not called with correct parameters #8781

Open
lcswillems opened this issue Mar 9, 2025 · 4 comments
Open

combine stable reference is not called with correct parameters #8781

lcswillems opened this issue Mar 9, 2025 · 4 comments

Comments

@lcswillems
Copy link

Describe the bug

export default function Page() {
  const [n, setN] = useState(0);

  const queries = useQueries({
    queries: [...Array(n).keys()].map((i) => ({
      queryKey: [i],
      queryFn: () => i,
    })),
    combine,
  });

  console.log(n, queries.length);

  return <button onClick={() => setN(n + 1)}>Increase</button>;
}

const combine = (results: any) => results;

Clicking a few times the "Increase" button would give those logs:

0 0
1 0
1 1
2 1
2 2
3 2
...

Which is not the correct behavior.

When I'm not using a stable reference, i.e. directly passing combine:

export default function Page() {
  const [n, setN] = useState(0);

  const queries = useQueries({
    queries: [...Array(n).keys()].map((i) => ({
      queryKey: [i],
      queryFn: () => i,
    })),
    combine: (results) => results,
  });

  console.log(n, queries.length);

  return <button onClick={() => setN(n + 1)}>Increase</button>;
}

I get those logs:

0 0
1 1
1 1
2 2
2 2
3 3
...

Which is the correct behavior.

Your minimal, reproducible example

https://codesandbox.io/p/devbox/dank-smoke-7h8ssj?file=%2Fsrc%2Findex.tsx

Steps to reproduce

Open the repro. Click on "Increase". Look at the logs.

Expected behavior

0 0
1 0
1 1
2 1
2 2
3 2
...

How often does this bug happen?

None

Screenshots or Videos

No response

Platform

Linux
Chrome

Tanstack Query adapter

react-query

TanStack Query version

5.59.20

TypeScript version

No response

Additional context

No response

@joseph0926
Copy link
Contributor

Isn't this a natural behavior caused by the function reference value not changing?

Looking at the “if” statement in the link, it looks like the intended behavior.


Wouldn't it probably go something like this?

(I used a translator)

  1. You have a React component state (n) initially at n = 1
  2. You click a button, updating the state to n = 2
  3. A re-render is immediately triggered by React (the current state is n = 2)
  4. A new array of queries with length 2 is created and passed into useQueries
  5. Internally, React Query’s QueriesObserver.setQueries() method is called, updating the internal observers and their options based on the new queries
  6. However, React Query does not immediately recalculate the results. Instead, it initially returns an optimistic result based on the previously cached state (from when n was still 1). Thus, the UI temporarily reflects the old state (queries array length = 1)
  7. Afterwards, React Query’s internal observers asynchronously detect the updated query states, perform the actual recalculations, and update the internal state with the correct results
  8. Finally, in the next render cycle, React Query passes the updated results (queries array length = 2) back to the component, causing the UI to reflect the latest state—but delayed by one render cycle

@lcswillems
Copy link
Author

lcswillems commented Mar 10, 2025

Hey @joseph0926 , thank you for your answer!

First, I believe that the behavior when passing a stable reference and when not passing a stable reference should be the same. That alone is already an issue to me.

Second, the result of useQuery should reflect the queries passed to useQuery. If I change the queries, it should reflect this in the results. So basically step 6 is not correct to me.

Similarly, right now, when I change the key of a query, it immediately sets back isPending to true and data to undefined. It doesn't return me the data of the previous query for one render cycle, and then update in the next one. And I think this is the correct behavior.

@joseph0926
Copy link
Contributor

@lcswillems
After hearing your explanation, I think you're right!
React Query API shouldn't depend on referential stability for the correctness of results

@TkDodo
Copy link
Collaborator

TkDodo commented Mar 28, 2025

First, I believe that the behavior when passing a stable reference and when not passing a stable reference should be the same. That alone is already an issue to me.

This is merely a performance optimization. When the reference is stable, you can’t close over values, so there’s no need to re-run combine on a render if data hasn’t changed - we can re-use the cached value.

For inline functions, that’s not the case. It’s really the same as we do with select on useQuery: https://tanstack.com/query/v5/docs/framework/react/guides/render-optimizations#memoization

However, it can of course be that we got something wrong in this caching. I don’t have the time right now to look into it. A PR with a failing test case would already be very helpful.

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

3 participants