query icon indicating copy to clipboard operation
query copied to clipboard

`useQueries` results are not stable even with memorization attempts

Open C3ntraX opened this issue 5 months ago • 7 comments

Describe the bug

I'm not able to memorize the result of useQueries. Every time the component renders, the useQueries creates a new result object which is not stable.

I tracked the issues here: https://github.com/TanStack/query/issues/2991 but this didn't solved the issue

Even your example on https://tanstack.com/query/latest/docs/framework/react/reference/useQueries is not stable:

// Your example: --- FAIL
  const ids = [1, 2, 3];
  const results = useQueries({
    queries: ids.map((id) => ({
      queryKey: ["post", id],
      queryFn: () => fetchPost(id),
      staleTime: Infinity,
    })),
  });

  // Try to memorize --- FAIL
  const queries = useMemo(() => {
    return [4, 5, 6].map((id) => ({
      queryKey: ["post", id],
      queryFn: () => fetchPost(id),
      staleTime: Infinity,
    }));
  }, []);

  const results2 = useQueries({
    queries: queries,
  });

  // Another try to memorize --- FAIL
  const queriesComplete = useMemo(() => {
    return {
      queries: [7, 8, 9].map((id) => ({
        queryKey: ["post", id],
        queryFn: () => fetchPost(id),
        staleTime: Infinity,
      })),
    };
  }, []);

const results3 = useQueries(queriesComplete);

Your minimal, reproducible example

https://codesandbox.io/p/devbox/vp3mys?file=%2Fsrc%2FApp.jsx%3A13%2C16

Updated*

Steps to reproduce

  1. Open the sandbox
  2. type text within the input
  3. See, that the counters going up, thats because the useEffect deps are not stable

Expected behavior

I need a way to get a stable array results and the inner result too which coming from useQuery.

All my components rerender, even if they don't need to. My workaround was to put the entire useQueries in a lower memorized component and lift all states upwards which is really bad.

You may even want to add some documentation, if you found a way to solve this. The useQueries hooks is really annoying to use currently.

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

Windos

Tanstack Query adapter

react-query

TanStack Query version

5.18.1

TypeScript version

4.4.4

Additional context

No response

C3ntraX avatar Feb 05 '24 14:02 C3ntraX

The problem here is also, that combine does also run every time and we have no chance of memorization for calcuations :/

You may even add a second paramter to combine which is used as dependencyArray like parameter, because useMemo can also be used for this. combine is really useless, without memorization and dependencyarray parameter

I have to declare a memorized component for every useQueries to avoid performance issues

C3ntraX avatar Feb 11 '24 20:02 C3ntraX

I need a way to get a stable array results and the inner result too which coming from useQuery.

why do you need that? The returned array is a new array every time. Each object inside it is also new, but the data property is stable.

combine is one way to address it, you can memoize function calls to combine separately with something like reselect, memoize-one or or other memoization techniques.

Your minimal, reproducible example

Your devbox link 404s for me

TkDodo avatar Feb 13 '24 12:02 TkDodo

The devbox was private and should now be accessible.

why do you need that? The returned array is a new array every time. Each object inside it is also new, but the data property is stable.

Yeah the individual query result for each query may stable but the returned array is not stable and since you could have O(n) queries would mean, you have to iterate over them if you don't want to use combine. The result array of useQueries has to be stable or you have to calculate the actual result every time some if some state changes and can't use the useMemo hook.

With combine, its running all the time even if no result of query in useQueries has changed. That does not seem right, because the combine should return the memorized result of combine instead.

Additional, with the memorization attempt, what I actually mean is that you could use other variables in the combine function, which should only trigger the recumputation of the result of combine if the variables used in combine have changed.

CentraXx avatar Feb 14 '24 10:02 CentraXx

I updated the devbox and added the simulation of combine problem

CentraXx avatar Feb 14 '24 10:02 CentraXx

The result array of useQueries has to be stable or you have to calculate the actual result every time some if some state changes and can't use the useMemo hook.

to calculate the "actual result" - that's why we introduced combine

what I actually mean is that you could use other variables in the combine function, which should only trigger the recomputation of the result of combine if the variables used in combine have changed.

yeah we have that for select, but haven't added it to combine yet.

TkDodo avatar Feb 14 '24 13:02 TkDodo

yeah we have that for select, but haven't added it to combine yet.

yeah it would be nice.

Addtional, I think my approch with the variables coming from outside is false. I think the right approch would be :

combine: (result, context) => result context: { variableFromOutside } <--- this object may come from useMemo or some a internal shallow compare?

with this, its like the context used for mutations but we can memorize the context, so if the the variables coming from the context and are changed changes or any result from useQueries change, then we execute the combine function again. I think, this would be a nice solution to avoid the usage from other libs, which people may not know of / how to use them

C3ntraX avatar Feb 15 '24 09:02 C3ntraX

I think the only thing missing is to remember the combine function and not execute it when the identity of the function is the same, so that consumers can memoize it with useCallback. This would be the same as how select works:

https://github.com/TanStack/query/blob/b32ad24f5d2ac79eb8dee32047195e2bb9267b2b/packages/query-core/src/queryObserver.ts#L461-L481

do you want to contribute this ?

TkDodo avatar Feb 17 '24 10:02 TkDodo

Yeah a useCallback makes actually more sense. Whats left is the results paramter to be stable (this would also be a benefit if you don't want to use combine), so that the useCallback function is only executed if the results object has changed or the useCallback func has changed.

Actually, with this solution, people always have to use the useCallback to prevent recalculation and unstable results from combine which is actually not that great.

I didn't even know that select can be used with useCallback, because I found no documentation for this.

C3ntraX avatar Feb 18 '24 16:02 C3ntraX

Ran into the same issue here.

For the time being my workaround is to piggyback on react's deps list to check if there are any changes in the query results, since actual data, isFetching and all other props of query result are already memoized.

If anyone needs this, here is a wrapper that returns same useQueries result, just memoized:

const useQueriesMemoized = (...args) => {
  const queries = useQueries(...args);
  const memoizedQueriesResult = useMemo(() => {
    return queries;
  }, [...queries.flatMap((x) => Object.keys(x).concat(Object.values(x)))]);
  return memoizedQueriesResult;
};

https://codesandbox.io/p/sandbox/determined-glade-l4rqg5

Usage:

const arrayOfQueryResults = useQueriesMemoized({
  queries: ids.map((id) => ({
    queryKey: ["post", id],
    queryFn: () => fetchPost(id),
    staleTime: Infinity,
  })),
});

Woyken avatar Mar 22 '24 14:03 Woyken