test(svelte-query/HydrationBoundary): add test for hydrating queries to the cache on context#10179
Open
sukvvon wants to merge 2 commits intoTanStack:mainfrom
Open
test(svelte-query/HydrationBoundary): add test for hydrating queries to the cache on context#10179sukvvon wants to merge 2 commits intoTanStack:mainfrom
sukvvon wants to merge 2 commits intoTanStack:mainfrom
Conversation
…to the cache on context
|
Contributor
📝 WalkthroughWalkthroughAdds a new Svelte test component and a test verifying hydration: a prefetched query is dehydrated, passed into a HydrationBoundary, initially renders cached data, then updates when the fresh query resolves. Changes
Sequence Diagram(s)sequenceDiagram
participant Test
participant QueryClient
participant DehydratedState
participant BaseExample
participant HydrationBoundary
participant QueryFn
Test->>QueryClient: prefetch(['string']) (resolves after 10ms)
QueryClient-->>Test: dehydrate() -> DehydratedState
Test->>BaseExample: render(props: {dehydratedState, queryFn})
BaseExample->>QueryClient: create client & set context
BaseExample->>HydrationBoundary: render(state=DehydratedState)
HydrationBoundary->>QueryClient: restore cached data
BaseExample->>QueryFn: fallback fetch (resolves after 20ms)
Note over BaseExample,HydrationBoundary: initial render shows cached value
Test->>Test: advance timers (21ms)
QueryFn-->>BaseExample: fresh result
BaseExample->>HydrationBoundary: update render with fresh data
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit 1afcfd1
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/svelte-query/tests/HydrationBoundary/BaseExample.svelte (2)
2-2:QueryClientis used only as a type — preferimport type.Since
QueryClientis only referenced in the type annotation, this should be a type-only import.♻️ Suggested change
- import { QueryClient } from '@tanstack/query-core' + import type { QueryClient } from '@tanstack/query-core'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/svelte-query/tests/HydrationBoundary/BaseExample.svelte` at line 2, The import of QueryClient is used only as a type and should be a type-only import; replace the value import "import { QueryClient } from '@tanstack/query-core'" with a type import "import type { QueryClient } from '@tanstack/query-core'" wherever QueryClient is used in type annotations (e.g., in the HydrationBoundary/BaseExample.svelte component), ensuring no runtime import remains.
28-34:options={undefined}andqueryClient={undefined}are redundant since both props have explicitundefineddefaults onHydrationBoundary.In Svelte 5, omitting optional props with
undefineddefaults is equivalent to explicitly passingundefined. These bindings add noise to the test fixture without adding clarity.♻️ Suggested change
<HydrationBoundary state={dehydratedState} - options={undefined} - queryClient={undefined} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/svelte-query/tests/HydrationBoundary/BaseExample.svelte` around lines 28 - 34, The HydrationBoundary usage redundantly passes options={undefined} and queryClient={undefined} even though HydrationBoundary already defaults those props to undefined; remove the explicit options and queryClient bindings from the HydrationBoundary component in BaseExample.svelte so it only passes state={dehydratedState} (and the children) to simplify the test fixture while preserving behavior.
🤖 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/svelte-query/tests/HydrationBoundary/HydrationBoundary.svelte.test.ts`:
- Around line 27-43: The test calls queryClient.clear() at the end of the test
body which will be skipped if an assertion throws; either wrap the test body in
a try/finally and call queryClient.clear() in finally, or refactor so
queryClient is declared in the surrounding scope (e.g. let queryClient;) and
assign it inside the test, then add an afterEach that calls queryClient.clear()
and vi.useRealTimers() to guarantee cleanup for the test "should hydrate queries
to the cache on context" (referencing queryClient.clear(), vi.useRealTimers(),
and the test block).
---
Nitpick comments:
In `@packages/svelte-query/tests/HydrationBoundary/BaseExample.svelte`:
- Line 2: The import of QueryClient is used only as a type and should be a
type-only import; replace the value import "import { QueryClient } from
'@tanstack/query-core'" with a type import "import type { QueryClient } from
'@tanstack/query-core'" wherever QueryClient is used in type annotations (e.g.,
in the HydrationBoundary/BaseExample.svelte component), ensuring no runtime
import remains.
- Around line 28-34: The HydrationBoundary usage redundantly passes
options={undefined} and queryClient={undefined} even though HydrationBoundary
already defaults those props to undefined; remove the explicit options and
queryClient bindings from the HydrationBoundary component in BaseExample.svelte
so it only passes state={dehydratedState} (and the children) to simplify the
test fixture while preserving behavior.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/svelte-query/tests/HydrationBoundary/BaseExample.sveltepackages/svelte-query/tests/HydrationBoundary/HydrationBoundary.svelte.test.ts
Comment on lines
27
to
43
| it('should hydrate queries to the cache on context', async () => { | ||
| const dehydratedState = JSON.parse(stringifiedState) | ||
| const queryClient = new QueryClient() | ||
|
|
||
| const rendered = render(BaseExample, { | ||
| props: { | ||
| queryClient, | ||
| dehydratedState, | ||
| queryFn: () => sleep(20).then(() => 'string'), | ||
| }, | ||
| }) | ||
|
|
||
| expect(rendered.getByText('data: stringCached')).toBeInTheDocument() | ||
| await vi.advanceTimersByTimeAsync(21) | ||
| expect(rendered.getByText('data: string')).toBeInTheDocument() | ||
| queryClient.clear() | ||
| }) |
Contributor
There was a problem hiding this comment.
queryClient.clear() inside the test body won't run if an assertion fails.
If getByText('data: stringCached') or getByText('data: string') throws, cleanup is skipped. Move it to afterEach (alongside vi.useRealTimers()) or use a try/finally block.
🛡️ Proposed fix — move cleanup to afterEach
+ let queryClient: QueryClient
+
afterEach(() => {
vi.useRealTimers()
+ queryClient.clear()
})
it('should hydrate queries to the cache on context', async () => {
const dehydratedState = JSON.parse(stringifiedState)
- const queryClient = new QueryClient()
+ queryClient = new QueryClient()
const rendered = render(BaseExample, {
props: {
queryClient,
dehydratedState,
queryFn: () => sleep(20).then(() => 'string'),
},
})
expect(rendered.getByText('data: stringCached')).toBeInTheDocument()
await vi.advanceTimersByTimeAsync(21)
expect(rendered.getByText('data: string')).toBeInTheDocument()
- queryClient.clear()
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/svelte-query/tests/HydrationBoundary/HydrationBoundary.svelte.test.ts`
around lines 27 - 43, The test calls queryClient.clear() at the end of the test
body which will be skipped if an assertion throws; either wrap the test body in
a try/finally and call queryClient.clear() in finally, or refactor so
queryClient is declared in the surrounding scope (e.g. let queryClient;) and
assign it inside the test, then add an afterEach that calls queryClient.clear()
and vi.useRealTimers() to guarantee cleanup for the test "should hydrate queries
to the cache on context" (referencing queryClient.clear(), vi.useRealTimers(),
and the test block).
… component instead of receiving via props
sukvvon
force-pushed
the
test/svelte-query-HydrationBoundary
branch
from
8bc1d99 to
1afcfd1
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/svelte-query/tests/HydrationBoundary/BaseExample.svelte (3)
29-30:options={undefined}andqueryClient={undefined}are redundant — omit them.In Svelte 5 runes mode, fallback values declared via
$props()destructuring are used when the parent does not set a given prop or the value isundefined. So explicitly passingundefinedis functionally identical to omitting these props, but it adds noise.♻️ Proposed cleanup
<HydrationBoundary state={dehydratedState} - options={undefined} - queryClient={undefined} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/svelte-query/tests/HydrationBoundary/BaseExample.svelte` around lines 29 - 30, Remove the redundant explicit undefined prop passes: delete the attributes options={undefined} and queryClient={undefined} from the component invocation in BaseExample.svelte so the component's fallback values (from $props() destructuring) are used; search for the attributes "options" and "queryClient" in that file and remove them, and run tests to confirm nothing else relies on an explicit undefined value.
21-33: Remove redundantundefinedprop assignments.Lines 29-30 explicitly pass
options={undefined}andqueryClient={undefined}, but HydrationBoundary already defaults these toundefined. Remove the explicit assignments:<HydrationBoundary state={dehydratedState} - options={undefined} - queryClient={undefined} > <div>data: {query.data ?? 'undefined'}</div> </HydrationBoundary>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/svelte-query/tests/HydrationBoundary/BaseExample.svelte` around lines 21 - 33, The HydrationBoundary JSX is passing redundant explicit undefined props; remove the explicit options={undefined} and queryClient={undefined} from the HydrationBoundary element so it relies on the component's defaults (leave state={dehydratedState} and the children intact); target the HydrationBoundary element in BaseExample.svelte to remove those two prop assignments.
29-30: Omitoptions={undefined}andqueryClient={undefined}— they are redundant.The
HydrationBoundarycomponent defaults bothoptionsandqueryClienttoundefined. In Svelte 5, explicitly passingprop={undefined}is equivalent to omitting the prop—both use the default value. Removing these lines is cleaner and reduces noise in the test.♻️ Proposed cleanup
<HydrationBoundary state={dehydratedState} - options={undefined} - queryClient={undefined} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/svelte-query/tests/HydrationBoundary/BaseExample.svelte` around lines 29 - 30, Remove the redundant explicit undefined props on the HydrationBoundary usage: delete options={undefined} and queryClient={undefined} from the HydrationBoundary component in BaseExample.svelte since HydrationBoundary defaults those to undefined; ensure no other logic relies on those props being explicitly passed and run tests to confirm behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/svelte-query/tests/HydrationBoundary/BaseExample.svelte`:
- Around line 29-30: Remove the redundant explicit undefined prop passes: delete
the attributes options={undefined} and queryClient={undefined} from the
component invocation in BaseExample.svelte so the component's fallback values
(from $props() destructuring) are used; search for the attributes "options" and
"queryClient" in that file and remove them, and run tests to confirm nothing
else relies on an explicit undefined value.
- Around line 21-33: The HydrationBoundary JSX is passing redundant explicit
undefined props; remove the explicit options={undefined} and
queryClient={undefined} from the HydrationBoundary element so it relies on the
component's defaults (leave state={dehydratedState} and the children intact);
target the HydrationBoundary element in BaseExample.svelte to remove those two
prop assignments.
- Around line 29-30: Remove the redundant explicit undefined props on the
HydrationBoundary usage: delete options={undefined} and queryClient={undefined}
from the HydrationBoundary component in BaseExample.svelte since
HydrationBoundary defaults those to undefined; ensure no other logic relies on
those props being explicitly passed and run tests to confirm behavior unchanged.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/svelte-query/tests/HydrationBoundary/BaseExample.sveltepackages/svelte-query/tests/HydrationBoundary/HydrationBoundary.svelte.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/svelte-query/tests/HydrationBoundary/HydrationBoundary.svelte.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Changes
Add a test for
HydrationBoundarythat verifies hydrated queries are available in the cache on context, improving coverage forHydrationBoundary.svelte(0% → 100%) anduseHydrate.ts(0% → 100%).The test follows the same pattern as the react-query
HydrationBoundary.test.tsx'should hydrate queries to the cache on context'test case.✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
Tests
Documentation