-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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 returns a new instance each time when useQueries is called without queries #6369
Comments
The initial fix for #6369 (#6431) introduced a regression where we'd always call combine with an empty array initially. Turns out it was actually on purpose that we didn't call combine in the constructor, but delayed that until the `setQueries` call - because there, we already have complete query result structures available However, there is an optimization that bails out early without calling this.#setResult if "nothing changed"; however, if we call useQueries with an empty array, that means nothing every changes, and the #combinedResult is actually never set, leading to structural sharing never having anything to share with, thus yielding a new reference every time (this is the root issue for #6369). This fix reverts those changes (to avoid calling combine with an empty array every time, thus fixing #6612) and makes sure #setResult is always invoked before bailing out, so that #combinedResult is set correctly even for this case
…queries (#6614) * fix(core): make sure to call setResult before the early bail-out The initial fix for #6369 (#6431) introduced a regression where we'd always call combine with an empty array initially. Turns out it was actually on purpose that we didn't call combine in the constructor, but delayed that until the `setQueries` call - because there, we already have complete query result structures available However, there is an optimization that bails out early without calling this.#setResult if "nothing changed"; however, if we call useQueries with an empty array, that means nothing every changes, and the #combinedResult is actually never set, leading to structural sharing never having anything to share with, thus yielding a new reference every time (this is the root issue for #6369). This fix reverts those changes (to avoid calling combine with an empty array every time, thus fixing #6612) and makes sure #setResult is always invoked before bailing out, so that #combinedResult is set correctly even for this case * fix: add a special length check to the bail-out case we only want to stop bailing out if we have no queries; moving the #setResult call unconditionally before the bail-out leads to more re-renders for other useQueries calls now, we only stop the bail-out when the special empty case sets in, which is what fixes #6612
sorry I had to revert both fixes that I merged for this, because it introduced regressions for other cases. Right now, I think this issues is the least problematic, as it requires:
and then, it "only" yields a new referential identity on re-renders, which shouldn't be that problematic because you have no queries anyways. Still, I'll keep this open in case someone wants to look into it. There's a commented out test here that should pass when fixed.
I've also added tests for the other cases that broke when I tried to fix this so that we don't re-introduce those regressions again. |
Thank you for the informations @TkDodo :) If anyone with the same issue gets here from google looking for a solution: you can avoid having a new reference at each render by defining the empty object / array outside of the hook. For instance: const emptyRecord = {};
export const useGetDevices = ({ ids }: { ids: string[] }) => {
return useQueries({
queries: ids.map((id) => {
return {
queryKey: ["get-device", id],
queryFn: async () => getDevice(id)
};
}),
combine: (results) => {
return results.reduce<Record<string, Device[]>>((acc, result) => {
const { data } = result;
if (!data) return acc;
return {
...acc,
[data.id]: data.content,
};
}, emptyRecord); // here replace "{}" with an empty object defined outside of the hook.
}
});
}; |
I don't think we will be fixing this, but given that it has a workaround and the issue is minor, I'll close it. |
Describe the bug
When
useQueries
is used with an empty array[]
forqueries
, thecombine
function will return a new instance each time the component carrying the hook is rendered. This can cause loop-rendering issues. It is possible to end up with an empty array forqueries
when the queries are determined dynamically. For instance:More details in this discussion
Your minimal, reproducible example
https://codesandbox.io/s/react-query-combine-empty-array-7n29nr?file=/src/useGetDevices.ts:523-773
Steps to reproduce
ids
to a non-empty array (for instance["1"]
)devicesRecord changed
is NOT called when the component is renderedids
to an empty array[]
Expected behavior
The useEffect should not be triggered each time the component is rendered
How often does this bug happen?
Every time
Screenshots or Videos
No response
Platform
Tanstack Query adapter
react-query
TanStack Query version
5.8.3
TypeScript version
No response
Additional context
No response
The text was updated successfully, but these errors were encountered: