#1775 caused useQueries to return unstable values between render cycles

[szszoke]

Describe the bug
The array returned by useQueries is not stable between render cycles which makes it unusable for anything down the line that uses shallow equality to determine if something had changed.

Small snippet to illustrate the bug

const responses = useQueries(preparedQueries);

console.log("changed?");
useEffect(() => console.log("changed!"), [responses]);

Console output

changed?
changed!
changed?
changed!
...

Expected behavior
The return value of useQueries is stable between render cycles as long as the queries did not change. The returned value can be used as a dependency for hooks like useMemo/useEffect which use shallow equality checks to determine if a dependency had changed since the last render

Desktop (please complete the following information):

• OS: Linux
• Browser: Firefox
• Version: 86

Additional context
The new behavior was introduced via #1775.

Relevant code lines:

https://github.com/tannerlinsley/react-query/blob/f2137dc4e4553256c4ebc1891b548fe35efe9231/src/react/useQueries.ts#L29

https://github.com/tannerlinsley/react-query/blob/f2137dc4e4553256c4ebc1891b548fe35efe9231/src/core/queriesObserver.ts#L67

[szszoke]

Versions after 3.8.3 are affected.

[TkDodo]

it was mentioned here, in 3.9.0 for concurrent mode safety:

Unfortunately I was not able to keep the referential integrity of the result object between renders because we do not know which optimistic result will eventually get committed.

is .data in each item referentially stable? I get that this might not be of much help if you are having a variable amount of queries

[szszoke]

it was mentioned here, in 3.9.0 for concurrent mode safety

Then I didn't do my homework. I apologize for that.

is .data in each item referentially stable?

I don't actually know that because my code was relying on the returned array being referentially stable.

I think I will be able to work around this issue for now by using Promise.all and queryClient.fetchQuery inside an effect.

The documentation for useQueries should probably mention that the returned array is not stable between render cycles.

[TkDodo]

The documentation for useQueries should probably mention that the returned array is not stable between render cycles.

yes, would you like to do that? It's also the case for useQuery. If we mention that, we should also mention that data is stable, very much so actually, even if background refetches occur, and even for selecting a slice of the data, you will get the same reference unless something changed.

[szszoke]

It would likely be useful to have a page where the referential stability of the hooks is discussed. With useQuery it is sort of implied that unless your query parameters changed or the data becomes stale in your cache, you get the same cached data between render cycles.

I can try writing a new page, or just extend the existing pages.

Maybe it is better to extend the existing pages.

What do you think?

[TkDodo]

sure, please open a PR that addresses it in the docs, extending the existing pages is fine. I'll close this issue.

[jcready]

That is quite a huge (see: disastrous) change that was introduced. It's unclear to me what the use for useQueries is if it doesn't return a stable result. And it's not mentioned in the docs at all. And this ticket was closed.

Edit: also a breaking change in a minor version is not a great look

[TkDodo]

It's unclear to me what the use for useQueries is if it doesn't return a stable result.

The use case is to run a variable amount of queries. I don't see what referentially stable return values have to do with this?

also a breaking change in a minor version is not a great look

I don't see the library guaranteeing to return the same array in every render, so it's technically not a breaking change ;)

---

On a more serious note, I see that this change is more severe for useQueries then it is for useQuery. With useQuery, you can just destruct data and get a stable reference:

const { data } = useQuery()

while with useQueries, you would get a new array every time, and even if you .map over it to get just the data, it would still be a new array every time:

const allData = useQueries(...).map(it => it.data)

---

@jcready maybe you can outline your use-case a bit, what you are doing with the result of useQueries that requires referential stability, and maybe we can find a solution to the problem?

[jcready]

The use case is to run a variable amount of queries. I don't see what referentially stable return values have to do with this?

You don't see what referentially stable return values for useQueries has to do with a ticket titled "#1775 caused useQueries to return unstable values between render cycles"?

I don't see the library guaranteeing to return the same array in every render, so it's technically not a breaking change ;)

That's not what defines a breaking change. In fact, the maintainers of this library were well aware of the breaking change and introduced it in a minor version bump anyways:

Unfortunately I was not able to keep the referential integrity of the result object between renders because we do not know which optimistic result will eventually get committed.

🎉 This PR is included in version 3.9.0 🎉

Just look at all the tickets that point back to #1775

maybe you can outline your use-case a bit, what you are doing with the result of useQueries that requires referential stability, and maybe we can find a solution to the problem?

The use-case is pretty straight forward. Use the result of useQueries in pretty much any other react hook that accepts a dependencies array: useEffect, useLayoutEffect, useCallback, useMemo, and all other react hooks that use those hooks as building blocks.

For anyone looking for a way to work around this breaking change this is what I've had to setup:

1. First grab a copy of the useMemoCompare hook
2. Create some helper functions:

   export function compareQuery<D>(a: UseQueryResult<D>, b: UseQueryResult<D>): boolean {
       return (
           a.data === b.data &&
           a.status === b.status &&
           a.dataUpdatedAt === b.dataUpdatedAt &&
           a.errorUpdatedAt === b.errorUpdatedAt
       );
   }
   
   export function compareQueries<D>(a: UseQueryResult<D>[], b: UseQueryResult<D>[]): boolean {
       return a === b || (a.length === b.length && a.every((q, i) => compareQuery(q, b[i])));
   }

3. Then you can use it with a useQueries result like this:

   const stableQueryResults = useMemoCompare(
       useQueries(/*...*/),
       compareQueries
   );
   
   const allData = useMemo(() => stableQueryResults.map(it => it.data), [stableQueryResults]);

This solution is definitely sub-optimal and may not handle every case, but it's better than the alternative: running even more expensive operations on the unstable useQueries result.

[TkDodo]

yes, I think the tradeoff of making the library adhere to the "rules of react" (no side effects during render), also dubbed as "works in concurrent mode" trumped the referential stability. I'm sorry if this has caused a headache for you. Its debatable if it is considered a breaking change imo, but that ship has sailed. Maybe we can revisit the implementation once React 18 comes out and we have access to things like useSyncExternalStore

the largest use case for useQueries is still rendering the result of multiple queries:

const queries = useQueries(...)

queries.map({ data } => <Component data={data} />)

since data on each element is still stable, these cases are not affected. Also, performing expensive aggregations via the select option on each query should still be optimized / only executed when something changed:

const queries = useQueries(
    something.map(x => ({ queryKey: x, queryFn: fetchX, select: expensiveTransformation })
)

maybe this can help you move the expensive part of your calculation into select which then potentially leaves you with a non-expensive aggregation (again, depends).

finally, still not ideal, but if the number of queries is known, you can destruct data and work with multiple of those.

[ivanjeremic]

Maybe we can revisit the implementation once React 18 comes out and we have access to things like useSyncExternalStore

Is latest react-query now making use of useSyncExternalStore internally?

[TkDodo]

Is latest react-query now making use of useSyncExternalStore internally?

v4 does, yes.