Skip to content

Commit

Permalink
fix(queriesObserver): do not bail-out of calling setResult for empty …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
TkDodo committed Dec 29, 2023
1 parent da38f64 commit 6ec6ee8
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 3 deletions.
10 changes: 7 additions & 3 deletions packages/query-core/src/queriesObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ export class QueriesObserver<
super()

this.#client = client
this.#queries = queries
this.#options = options
this.#queries = []
this.#observers = []

this.#setResult([])
Expand Down Expand Up @@ -108,7 +107,12 @@ export class QueriesObserver<
const hasIndexChange = newObservers.some(
(observer, index) => observer !== prevObservers[index],
)
if (prevObservers.length === newObservers.length && !hasIndexChange) {

if (
prevObservers.length === newObservers.length &&
!hasIndexChange &&
newObservers.length > 0
) {
return
}

Expand Down
40 changes: 40 additions & 0 deletions packages/react-query/src/__tests__/useQueries.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1027,6 +1027,46 @@ describe('useQueries', () => {
expect(resultChanged).toBe(1)
})

it('should only call combine with query results', async () => {
const key1 = queryKey()
const key2 = queryKey()

function Page() {
const result = useQueries({
queries: [
{
queryKey: key1,
queryFn: async () => {
await sleep(5)
return Promise.resolve('query1')
},
},
{
queryKey: key2,
queryFn: async () => {
await sleep(20)
return Promise.resolve('query2')
},
},
],
combine: ([query1, query2]) => {
return {
data: { query1: query1.data, query2: query2.data },
}
},
})

return <div>data: {JSON.stringify(result)}</div>
}

const rendered = renderWithClient(queryClient, <Page />)
await waitFor(() =>
rendered.getByText(
'data: {"data":{"query1":"query1","query2":"query2"}}',
),
)
})

it('should track property access through combine function', async () => {
const key1 = queryKey()
const key2 = queryKey()
Expand Down

0 comments on commit 6ec6ee8

Please sign in to comment.