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
232 changes: 165 additions & 67 deletions packages/preact-query/src/__tests__/useSuspenseQueries.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { queryKey, sleep } from '@tanstack/query-test-utils'
import { act, fireEvent, render } from '@testing-library/preact'
import type { FunctionalComponent } from 'preact'
import { Suspense, startTransition, useTransition } from 'preact/compat'
import { useEffect, useRef, useState } from 'preact/hooks'
import {
Expand All @@ -24,33 +23,18 @@ import type { UseSuspenseQueryOptions } from '..'
import { ErrorBoundary } from './ErrorBoundary'
import { renderWithClient } from './utils'

type NumberQueryOptions = UseSuspenseQueryOptions<number>

const QUERY_DURATION = 1000

const createQuery: (id: number) => NumberQueryOptions = (id) => ({
queryKey: [id],
queryFn: () => sleep(QUERY_DURATION).then(() => id),
})
const resolveQueries = async () => {
await vi.advanceTimersByTimeAsync(QUERY_DURATION)
}

const queryClient = new QueryClient()

describe('useSuspenseQueries', () => {
let queryClient: QueryClient
const onSuspend = vi.fn()
const onQueriesResolution = vi.fn()

beforeAll(() => {
beforeEach(() => {
vi.useFakeTimers()
})

afterAll(() => {
vi.useRealTimers()
queryClient = new QueryClient()
})

afterEach(() => {
vi.useRealTimers()
queryClient.clear()
onSuspend.mockClear()
onQueriesResolution.mockClear()
Expand All @@ -64,89 +48,199 @@ describe('useSuspenseQueries', () => {
return <div>loading</div>
}

const withSuspenseWrapper = <T extends object>(
Component: FunctionalComponent<T>,
) => {
function SuspendedComponent(props: T) {
return (
<Suspense fallback={<SuspenseFallback />}>
<Component {...props} />
</Suspense>
it('should suspend on mount', () => {
function Page() {
const queriesResults = useSuspenseQueries(
{
queries: [1, 2].map((id) => ({
queryKey: [id],
queryFn: () => sleep(1000).then(() => id),
})),
combine: (results) => results.map((r) => r.data),
},
queryClient,
)
}

return SuspendedComponent
}

function QueriesContainer({
queries,
}: {
queries: Array<NumberQueryOptions>
}) {
const queriesResults = useSuspenseQueries(
{ queries, combine: (results) => results.map((r) => r.data) },
queryClient,
)

useEffect(() => {
onQueriesResolution(queriesResults)
}, [queriesResults])

return null
}
useEffect(() => {
onQueriesResolution(queriesResults)
}, [queriesResults])

const TestComponent = withSuspenseWrapper(QueriesContainer)
return null
}

it('should suspend on mount', () => {
render(<TestComponent queries={[1, 2].map(createQuery)} />)
render(
<Suspense fallback={<SuspenseFallback />}>
<Page />
</Suspense>,
)

expect(onSuspend).toHaveBeenCalledOnce()
})

it('should resolve queries', async () => {
render(<TestComponent queries={[1, 2].map(createQuery)} />)
function Page() {
const queriesResults = useSuspenseQueries(
{
queries: [1, 2].map((id) => ({
queryKey: [id],
queryFn: () => sleep(1000).then(() => id),
})),
combine: (results) => results.map((r) => r.data),
},
queryClient,
)

await act(resolveQueries)
useEffect(() => {
onQueriesResolution(queriesResults)
}, [queriesResults])

return null
}

render(
<Suspense fallback={<SuspenseFallback />}>
<Page />
</Suspense>,
)

await act(async () => {
await vi.advanceTimersByTimeAsync(1000)
})

expect(onQueriesResolution).toHaveBeenCalledTimes(1)
expect(onQueriesResolution).toHaveBeenLastCalledWith([1, 2])
})

it('should not suspend on mount if query has been already fetched', () => {
const query = createQuery(1)
const key = queryKey()
const queryFn = () => sleep(1000).then(() => 1)

queryClient.setQueryData(query.queryKey, query.queryFn)
queryClient.setQueryData(key, queryFn)
Comment on lines +115 to +118
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C5 --iglob 'packages/**/queryClient.ts' '\bsetQueryData\b|functionalUpdate'
rg -n -C4 --iglob 'packages/**/utils.ts' '\bfunctionalUpdate\b'

Repository: TanStack/query

Length of output: 5743


🏁 Script executed:

cat -n packages/query-core/src/utils.ts | sed -n '98,110p'

Repository: TanStack/query

Length of output: 511


🏁 Script executed:

cat -n packages/preact-query/src/__tests__/useSuspenseQueries.test.tsx | sed -n '110,125p'

Repository: TanStack/query

Length of output: 700


🏁 Script executed:

cat -n packages/preact-query/src/__tests__/useSuspenseQueries.test.tsx | sed -n '114,135p'

Repository: TanStack/query

Length of output: 765


🏁 Script executed:

rg -n 'already fetched|already cached' packages/preact-query/src/__tests__/useSuspenseQueries.test.tsx

Repository: TanStack/query

Length of output: 139


🏁 Script executed:

cat -n packages/preact-query/src/__tests__/useSuspenseQueries.test.tsx | sed -n '114,145p'

Repository: TanStack/query

Length of output: 1076


🏁 Script executed:

rg -n 'suspend|Suspense|throw' packages/preact-query/src/__tests__/useSuspenseQueries.test.tsx | head -20

Repository: TanStack/query

Length of output: 982


🏁 Script executed:

rg -n -A10 'useSuspenseQueries.*=' packages/preact-query/src/__tests__/useSuspenseQueries.test.tsx | head -40

Repository: TanStack/query

Length of output: 606


🏁 Script executed:

fd -e ts -e tsx packages/preact-query/src --exec grep -l 'useSuspenseQueries' {} \; | head -5

Repository: TanStack/query

Length of output: 463


🏁 Script executed:

fd --full-path 'packages/preact-query/src' -e ts -e tsx | grep -i suspense

Repository: TanStack/query

Length of output: 695


🏁 Script executed:

rg -n 'const onSuspend|toHaveBeenCalled\(\)' packages/preact-query/src/__tests__/useSuspenseQueries.test.tsx | head -15

Repository: TanStack/query

Length of output: 364


Pass resolved data to setQueryData, not the query function.

Line 118 passes queryFn to setQueryData, which treats it as an updater function (via functionalUpdate). This executes queryFn() immediately and caches the Promise it returns, rather than caching the resolved data. The test intent—verifying behavior when data is "already fetched"—requires resolved data in the cache, not a pending Promise.

🧪 Proposed fix
-    queryClient.setQueryData(key, queryFn)
+    queryClient.setQueryData(key, 1)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const key = queryKey()
const queryFn = () => sleep(1000).then(() => 1)
queryClient.setQueryData(query.queryKey, query.queryFn)
queryClient.setQueryData(key, queryFn)
const key = queryKey()
const queryFn = () => sleep(1000).then(() => 1)
queryClient.setQueryData(key, 1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/preact-query/src/__tests__/useSuspenseQueries.test.tsx` around lines
115 - 118, The test incorrectly passes the query function to
queryClient.setQueryData (queryClient.setQueryData(key, queryFn)), which treats
it as an updater and caches a Promise; change it to pass the resolved data
instead by invoking and awaiting the function (or directly supplying the
resolved value) so setQueryData receives the resolved result for the key
(reference symbols: key, queryFn, queryClient.setQueryData).


render(<TestComponent queries={[query]} />)
function Page() {
const queriesResults = useSuspenseQueries(
{
queries: [{ queryKey: key, queryFn }],
combine: (results) => results.map((r) => r.data),
},
queryClient,
)

useEffect(() => {
onQueriesResolution(queriesResults)
}, [queriesResults])

return null
}

render(
<Suspense fallback={<SuspenseFallback />}>
<Page />
</Suspense>,
)

expect(onSuspend).not.toHaveBeenCalled()
})

it('should not break suspense when queries change without resolving', async () => {
const initQueries = [1, 2].map(createQuery)
const nextQueries = [3, 4, 5, 6].map(createQuery)
const initQueries = [1, 2].map((id) => ({
queryKey: [id],
queryFn: () => sleep(1000).then(() => id),
}))
const nextQueries = [3, 4, 5, 6].map((id) => ({
queryKey: [id],
queryFn: () => sleep(1000).then(() => id),
}))

function Page({
queries,
}: {
queries: Array<UseSuspenseQueryOptions<number>>
}) {
const queriesResults = useSuspenseQueries(
{
queries,
combine: (results) => results.map((r) => r.data),
},
queryClient,
)

useEffect(() => {
onQueriesResolution(queriesResults)
}, [queriesResults])

const { rerender } = render(<TestComponent queries={initQueries} />)
return null
}

rerender(<TestComponent queries={nextQueries} />)
const { rerender } = render(
<Suspense fallback={<SuspenseFallback />}>
<Page queries={initQueries} />
</Suspense>,
)

await act(resolveQueries)
rerender(
<Suspense fallback={<SuspenseFallback />}>
<Page queries={nextQueries} />
</Suspense>,
)

await act(async () => {
await vi.advanceTimersByTimeAsync(1000)
})

expect(onSuspend).toHaveBeenCalled()
// the test for onQueriesResolution is React-specific and not applicable to Preact
})

it('should suspend only once per queries change', async () => {
const initQueries = [1, 2].map(createQuery)
const nextQueries = [3, 4, 5, 6].map(createQuery)
const initQueries = [1, 2].map((id) => ({
queryKey: [id],
queryFn: () => sleep(1000).then(() => id),
}))
const nextQueries = [3, 4, 5, 6].map((id) => ({
queryKey: [id],
queryFn: () => sleep(1000).then(() => id),
}))

function Page({
queries,
}: {
queries: Array<UseSuspenseQueryOptions<number>>
}) {
const queriesResults = useSuspenseQueries(
{
queries,
combine: (results) => results.map((r) => r.data),
},
queryClient,
)

useEffect(() => {
onQueriesResolution(queriesResults)
}, [queriesResults])

return null
}

const { rerender } = render(<TestComponent queries={initQueries} />)
const { rerender } = render(
<Suspense fallback={<SuspenseFallback />}>
<Page queries={initQueries} />
</Suspense>,
)

await act(resolveQueries)
await act(async () => {
await vi.advanceTimersByTimeAsync(1000)
})

rerender(<TestComponent queries={nextQueries} />)
rerender(
<Suspense fallback={<SuspenseFallback />}>
<Page queries={nextQueries} />
</Suspense>,
)

await act(resolveQueries)
await act(async () => {
await vi.advanceTimersByTimeAsync(1000)
})

expect(onSuspend).toHaveBeenCalledTimes(2)
expect(onQueriesResolution).toHaveBeenCalledTimes(2)
Expand Down Expand Up @@ -263,12 +357,16 @@ describe('useSuspenseQueries', () => {
})

describe('useSuspenseQueries 2', () => {
let queryClient: QueryClient

beforeEach(() => {
vi.useFakeTimers()
queryClient = new QueryClient()
})

afterEach(() => {
vi.useRealTimers()
queryClient.clear()
})

it('should suspend all queries in parallel', async () => {
Expand Down
Loading
Loading