Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/wild-rabbits-jump.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@tanstack/react-query': patch
'@tanstack/query-core': patch
---

fix(suspense): skip calling combine when queries would suspend
76 changes: 76 additions & 0 deletions packages/query-core/src/__tests__/queriesObserver.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,82 @@ describe('queriesObserver', () => {
expect(newCombined.count).toBe(2)
})

it('should skip combine notifications while suspense queries have no data', async () => {
const key = queryKey()
const combine = vi.fn((results: Array<QueryObserverResult>) =>
results.map((result) => result.data),
)
const query = {
queryKey: key,
queryFn: () => sleep(10).then(() => 'data'),
staleTime: Infinity,
suspense: true,
}

queryClient.setQueryData(key, 'data')

const observer = new QueriesObserver<Array<unknown>>(queryClient, [query], {
combine,
})

const [rawResult, getCombinedResult] = observer.getOptimisticResult(
[query],
combine,
)
expect(getCombinedResult(rawResult)).toEqual(['data'])
expect(combine).toHaveBeenCalledTimes(1)

const unsubscribe = observer.subscribe(() => undefined)

void queryClient.resetQueries({ queryKey: key })
expect(combine).toHaveBeenCalledTimes(1)
Comment thread
TkDodo marked this conversation as resolved.

unsubscribe()
})

it('should skip combine notifications after suspense is enabled without structural changes', async () => {
const key = queryKey()
const combine = vi.fn((results: Array<QueryObserverResult>) =>
results.map((result) => result.data),
)
const query = {
queryKey: key,
queryFn: () => sleep(10).then(() => 'data'),
staleTime: Infinity,
suspense: false,
}

queryClient.setQueryData(key, 'data')

const observer = new QueriesObserver<Array<unknown>>(queryClient, [query], {
combine,
})

const [rawResult, getCombinedResult] = observer.getOptimisticResult(
[query],
combine,
)
expect(getCombinedResult(rawResult)).toEqual(['data'])
expect(combine).toHaveBeenCalledTimes(1)

const unsubscribe = observer.subscribe(() => undefined)

observer.setQueries(
[
{
...query,
suspense: true,
},
],
{ combine },
)

void queryClient.resetQueries({ queryKey: key })
expect(combine).toHaveBeenCalledTimes(1)

unsubscribe()
})

it('should handle queries being removed with stable combine reference', () => {
const combine = vi.fn((results: Array<QueryObserverResult>) => ({
count: results.length,
Expand Down
20 changes: 17 additions & 3 deletions packages/query-core/src/queriesObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,17 @@ export class QueriesObserver<
return input as any
}

#shouldSkipCombine(): boolean {
return (
this.#options?.combine !== undefined &&
this.#observers.some((observer, index) => {
return (
observer.options.suspense && this.#result[index]?.data === undefined
)
})
)
Comment thread
TkDodo marked this conversation as resolved.
}

#findMatchingObservers(
queries: Array<QueryObserverOptions>,
): Array<QueryObserverMatch> {
Expand Down Expand Up @@ -294,11 +305,14 @@ export class QueriesObserver<

#notify(): void {
if (this.hasListeners()) {
const previousResult = this.#combinedResult
const newTracked = this.#trackResult(this.#result, this.#observerMatches)
const newResult = this.#combineResult(newTracked, this.#options?.combine)
const shouldSkipCombine = this.#shouldSkipCombine()
const previousResult = this.#combinedResult
const newResult = shouldSkipCombine
? previousResult
: this.#combineResult(newTracked, this.#options?.combine)

if (previousResult !== newResult) {
if (shouldSkipCombine || previousResult !== newResult) {
notifyManager.batch(() => {
this.listeners.forEach((listener) => {
listener(this.#result)
Expand Down
68 changes: 68 additions & 0 deletions packages/react-query/src/__tests__/useSuspenseQueries.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,74 @@ describe('useSuspenseQueries', () => {
expect(spy).toHaveBeenCalled()
})

it('should not call combine while reset queries are pending again', async () => {
const consoleMock = vi
.spyOn(console, 'error')
.mockImplementation(() => undefined)
const key = queryKey()
let shouldError = false

function Page() {
const data = useSuspenseQueries({
queries: [
{
queryKey: key,
queryFn: () =>
sleep(10).then(() => {
if (shouldError) {
throw new Error('Suspense Error Bingo')
}

return 'data'
}),
retry: false,
},
],
combine: (result) => result.map((query) => query.data.toUpperCase()),
})

return (
<div>
<button
onClick={() => void queryClient.resetQueries({ queryKey: key })}
>
reset
</button>
<div>data: {data.join(',')}</div>
</div>
)
}

const rendered = renderWithClient(
queryClient,
<ErrorBoundary fallbackRender={() => <div>error boundary</div>}>
<React.Suspense fallback="loading">
<Page />
</React.Suspense>
</ErrorBoundary>,
)

expect(rendered.getByText('loading')).toBeInTheDocument()

await act(() => vi.advanceTimersByTimeAsync(10))
expect(rendered.getByText('data: DATA')).toBeInTheDocument()

shouldError = true

expect(() => {
fireEvent.click(rendered.getByText('reset'))
}).not.toThrow()

await act(() => vi.advanceTimersByTimeAsync(10))
expect(rendered.getByText('error boundary')).toBeInTheDocument()

expect(consoleMock.mock.calls[0]?.[1]).toStrictEqual(
new Error('Suspense Error Bingo'),
)

consoleMock.mockRestore()
})

it('should handle duplicate query keys without infinite loops', async () => {
const key = queryKey()
const localDuration = 10
Expand Down
Loading