Skip to content

Commit

Permalink
fix(core): do not invoke combine in constructor (#7215)
Browse files Browse the repository at this point in the history
because the component might suspend, in which case we have nothing to combine
  • Loading branch information
TkDodo committed Apr 2, 2024
1 parent a4d843a commit 53cdfce
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 17 deletions.
25 changes: 9 additions & 16 deletions packages/query-core/src/queriesObserver.ts
Expand Up @@ -39,27 +39,21 @@ export class QueriesObserver<
#result!: Array<QueryObserverResult>
#queries: Array<QueryObserverOptions>
#observers: Array<QueryObserver>
#options?: QueriesObserverOptions<TCombinedResult>
#combinedResult!: TCombinedResult
#combinedResult?: TCombinedResult

constructor(
client: QueryClient,
queries: Array<QueryObserverOptions>,
options?: QueriesObserverOptions<TCombinedResult>,
_options?: QueriesObserverOptions<TCombinedResult>,
) {
super()

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

this.#setResult([])
this.setQueries(queries, options)
}

#setResult(value: Array<QueryObserverResult>) {
this.#result = value
this.#combinedResult = this.#combineResult(value, this.#options?.combine)
this.setQueries(queries)
}

protected onSubscribe(): void {
Expand Down Expand Up @@ -87,11 +81,10 @@ export class QueriesObserver<

setQueries(
queries: Array<QueryObserverOptions>,
options?: QueriesObserverOptions<TCombinedResult>,
_options?: QueriesObserverOptions<TCombinedResult>,
notifyOptions?: NotifyOptions,
): void {
this.#queries = queries
this.#options = options

notifyManager.batch(() => {
const prevObservers = this.#observers
Expand All @@ -117,7 +110,7 @@ export class QueriesObserver<
}

this.#observers = newObservers
this.#setResult(newResult)
this.#result = newResult

if (!this.hasListeners()) {
return
Expand All @@ -137,8 +130,8 @@ export class QueriesObserver<
})
}

getCurrentResult(): TCombinedResult {
return this.#combinedResult
getCurrentResult(): Array<QueryObserverResult> {
return this.#result
}

getQueries() {
Expand Down Expand Up @@ -254,7 +247,7 @@ export class QueriesObserver<
#onUpdate(observer: QueryObserver, result: QueryObserverResult): void {
const index = this.#observers.indexOf(observer)
if (index !== -1) {
this.#setResult(replaceAt(this.#result, index, result))
this.#result = replaceAt(this.#result, index, result)
this.#notify()
}
}
Expand Down
43 changes: 42 additions & 1 deletion packages/react-query/src/__tests__/useSuspenseQueries.test.tsx
Expand Up @@ -10,7 +10,7 @@ import {
import { act, render } from '@testing-library/react'
import * as React from 'react'
import { useSuspenseQueries } from '..'
import { createQueryClient, sleep } from './utils'
import { createQueryClient, queryKey, renderWithClient, sleep } from './utils'
import type { UseSuspenseQueryOptions } from '..'

type NumberQueryOptions = UseSuspenseQueryOptions<number>
Expand Down Expand Up @@ -141,4 +141,45 @@ describe('useSuspenseQueries', () => {
expect(onQueriesResolution).toHaveBeenCalledTimes(2)
expect(onQueriesResolution).toHaveBeenLastCalledWith([3, 4, 5, 6])
})

it('should only call combine after resolving', async () => {
const spy = vi.fn()
const key = queryKey()

function Page() {
const data = useSuspenseQueries({
queries: [1, 2, 3].map((value) => ({
queryKey: [...key, { value }],
queryFn: async () => {
await sleep(value * 10)
return { value: value * 10 }
},
})),
combine: (result) => {
spy(result)
return 'data'
},
})

return <h1>{data}</h1>
}

const rendered = renderWithClient(
queryClient,
<React.Suspense fallback="loading...">
<Page />
</React.Suspense>,
)

await act(() => vi.advanceTimersByTimeAsync(10))

rendered.getByText('loading...')

expect(spy).not.toHaveBeenCalled()

await act(() => vi.advanceTimersByTimeAsync(30))
rendered.getByText('data')

expect(spy).toHaveBeenCalled()
})
})

0 comments on commit 53cdfce

Please sign in to comment.