query icon indicating copy to clipboard operation
query copied to clipboard

combine returns a new instance each time when useQueries is called without queries

Open HarmonicTons opened this issue 2 years ago • 2 comments

Describe the bug

When useQueries is used with an empty array [] for queries, the combine 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 for queries when the queries are determined dynamically. For instance:

export const useGetDevices = ({ ids }: { ids: string[] }) => {
 return useQueries({
   // queries are dynamically determined from the given ids
   // when ids is empty, queries will be []
   queries: ids.map((id) => {
     return {
       queryKey: ["get-device", id],
       queryFn: async () => getDevice(id)
     };
   }),
   combine: (results) => {
      // here we simply reduce the results into a Record
      // when ids is not empty the reduced record will be the same instance each time
      // but when ids is empty the reduced record will be a new instance of an empty object each time
      return results.reduce<Record<string, Device[]>>((acc, result) => {
        const { data } = result;
        if (!data) return acc;
        return {
          ...acc,
          [data.id]: data.content,
        };
      }, {});
    }
 });
};

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

  1. In app.tsx: set ids to a non-empty array (for instance ["1"])
  2. Click on "render" with the console opened
  3. See that the useEffect devicesRecord changed is NOT called when the component is rendered
  4. Edit app.tsx to set ids to an empty array []
  5. Click on "render" again
  6. See that now the useEffect is called each time the component is rendered

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

  • OS: macOS
  • browser: chrome, FF

Tanstack Query adapter

react-query

TanStack Query version

5.8.3

TypeScript version

No response

Additional context

No response

HarmonicTons avatar Nov 14 '23 08:11 HarmonicTons

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:

  • empty array of queries
  • combine function

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.

https://github.com/TanStack/query/blob/985146f58483f31781cee033464cba19b6c07838/packages/react-query/src/tests/useQueries.test.tsx#L981

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.

TkDodo avatar Jan 05 '24 21:01 TkDodo

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.
    }
 });
};

HarmonicTons avatar Jan 12 '24 10:01 HarmonicTons

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.

TkDodo avatar Feb 17 '24 18:02 TkDodo