Conversation
…emove shared 'queryClient', and use 'beforeEach'/'afterEach' for setup and teardown
📝 WalkthroughWalkthroughThe changes refactor test suites for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit cacc758
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version PreviewNo changeset entries found. Merging this PR will not cause a version bump for any packages. |
size-limit report 📦
|
… with 'queryKey()' util and fix 'act' callback type in preact-query
sukvvon
force-pushed
the
test/react-preact-query-useSuspenseQueries-inline-test-helpers
branch
from
50f9ef1 to
cacc758
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/preact-query/src/__tests__/useSuspenseQueries.test.tsx`:
- Around line 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).
In `@packages/react-query/src/__tests__/useSuspenseQueries.test.tsx`:
- Around line 111-114: The test is seeding the cache with the query function
itself (queryFn) so setQueryData treats it as an updater and stores the Promise
instead of the resolved value; change the seed to the resolved value by calling
queryFn() and awaiting it (or simply pass the literal resolved value 1) when
calling queryClient.setQueryData so the cache contains the resolved data rather
than a function/Promise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 38a141fc-f2e2-4510-99e3-fd44f3c1b72d
📒 Files selected for processing (2)
packages/preact-query/src/__tests__/useSuspenseQueries.test.tsxpackages/react-query/src/__tests__/useSuspenseQueries.test.tsx
| const key = queryKey() | ||
| const queryFn = () => sleep(1000).then(() => 1) | ||
|
|
||
| queryClient.setQueryData(query.queryKey, query.queryFn) | ||
| queryClient.setQueryData(key, queryFn) |
There was a problem hiding this comment.
🧩 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.tsxRepository: 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 -20Repository: TanStack/query
Length of output: 982
🏁 Script executed:
rg -n -A10 'useSuspenseQueries.*=' packages/preact-query/src/__tests__/useSuspenseQueries.test.tsx | head -40Repository: TanStack/query
Length of output: 606
🏁 Script executed:
fd -e ts -e tsx packages/preact-query/src --exec grep -l 'useSuspenseQueries' {} \; | head -5Repository: TanStack/query
Length of output: 463
🏁 Script executed:
fd --full-path 'packages/preact-query/src' -e ts -e tsx | grep -i suspenseRepository: TanStack/query
Length of output: 695
🏁 Script executed:
rg -n 'const onSuspend|toHaveBeenCalled\(\)' packages/preact-query/src/__tests__/useSuspenseQueries.test.tsx | head -15Repository: 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.
| 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).
| const key = queryKey() | ||
| const queryFn = () => sleep(1000).then(() => 1) | ||
|
|
||
| queryClient.setQueryData(key, queryFn) |
There was a problem hiding this comment.
🧩 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:
fd -t f 'useSuspenseQueries.test.tsx' packages/react-query/srcRepository: TanStack/query
Length of output: 120
🏁 Script executed:
sed -n '105,120p' packages/react-query/src/__tests__/useSuspenseQueries.test.tsxRepository: TanStack/query
Length of output: 578
🏁 Script executed:
rg -n 'setQueryData.*queryFn' packages/react-query/src/__tests__/useSuspenseQueries.test.tsxRepository: TanStack/query
Length of output: 104
🏁 Script executed:
sed -n '108,135p' packages/react-query/src/__tests__/useSuspenseQueries.test.tsxRepository: TanStack/query
Length of output: 732
🏁 Script executed:
rg -n 'setQueryData' packages/react-query/src/__tests__/useSuspenseQueries.test.tsx | head -20Repository: TanStack/query
Length of output: 712
Seed the cache with resolved data, not the query function.
Line 114 passes queryFn to setQueryData, but setQueryData invokes the functionalUpdate utility, which treats functions as updaters and calls them. This seeds the cache with the Promise returned by queryFn(), not with the resolved value 1. The test fails to exercise the cached-data path as intended.
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.
| const key = queryKey() | |
| const queryFn = () => sleep(1000).then(() => 1) | |
| 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/react-query/src/__tests__/useSuspenseQueries.test.tsx` around lines
111 - 114, The test is seeding the cache with the query function itself
(queryFn) so setQueryData treats it as an updater and stores the Promise instead
of the resolved value; change the seed to the resolved value by calling
queryFn() and awaiting it (or simply pass the literal resolved value 1) when
calling queryClient.setQueryData so the cache contains the resolved data rather
than a function/Promise.
1 participant
🎯 Changes
createQuery,resolveQueries,QUERY_DURATION,withSuspenseWrapper,QueriesContainer,TestComponent) and inline them directly in each test for better readabilityqueryClientfrom module scope intobeforeEachso each test gets a fresh instance, preventing cross-test state leakagebeforeAll/afterAllwithbeforeEach/afterEachfor fake timer setup/teardown to be consistent with other test files (e.g.useQuery.test.tsx)FunctionalComponentimport from preact-query test[1] as constwithqueryKey()utilactcallback type error in preact-query by usingasync () => { await ... }pattern✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit