Fix: add (de)hydration options to tanstack query integration #5186
Conversation
WalkthroughAdds Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Server
participant Router as SSR Router Core
participant QC as QueryClient (Server)
participant Stream as Stream/Transport
participant Client
participant QCc as QueryClient (Client)
Server->>Router: setupCoreRouterSsrQueryIntegration({ dehydrateOptions, hydrateOptions })
Router->>QC: prepare queries
Router->>QC: queryDehydrate(QC, dehydrateOptions)
QC-->>Router: DehydratedState
Router-->>Server: send Initial payload (DehydratedState)
rect rgb(245,250,240)
note over Router,QC: Streaming individual queries (per-item dehydrateOptions applied)
Router->>QC: queryDehydrate(QC, { ...dehydrateOptions, shouldDehydrateQuery: fn })
QC-->>Router: DehydratedQuery
Router->>Stream: emit { value: DehydratedQuery, hydrateOptions }
end
Server->>Client: initial + streamed payloads
Client->>QCc: queryHydrate(QCc, InitialDehydratedState, hydrateOptions)
Client->>QCc: On stream: queryHydrate(QCc, value, hydrateOptions)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/router-ssr-query-core/src/index.ts (3)
104-115: Bug: Spreading hydrateOptions into queryDehydrate (should be dehydrateOptions) and ignoring user predicatequeryDehydrate expects DehydrateOptions. Spreading hydrateOptions is a type/semantic mismatch. Also, the custom shouldDehydrateQuery should respect user‑provided dehydrateOptions.shouldDehydrateQuery, not only ogClientOptions.
Apply:
- queryStream.enqueue( - queryDehydrate(queryClient, { - ...hydrateOptions, - shouldDehydrateQuery: (query) => { - if (query.queryHash === event.query.queryHash) { - return ( - ogClientOptions.dehydrate?.shouldDehydrateQuery?.(query) ?? true - ) - } - return false - }, - }), - ) + queryStream.enqueue( + queryDehydrate(queryClient, { + ...dehydrateOptions, + shouldDehydrateQuery: (query) => { + if (query.queryHash === event.query.queryHash) { + const userShould = + dehydrateOptions?.shouldDehydrateQuery ?? + ogClientOptions.dehydrate?.shouldDehydrateQuery + return userShould?.(query) ?? true + } + return false + }, + }), + )
133-141: Avoid hydrating when the stream signals doneWhen done is true, value is undefined; calling queryHydrate with it risks runtime errors. Hydrate only when !done.
Apply:
- .then(async function handle({ done, value }) { - queryHydrate(queryClient, value, hydrateOptions) + .then(async function handle({ done, value }) { + if (!done) { + queryHydrate(queryClient, value, hydrateOptions) + } if (done) { return } const result = await reader.read() return handle(result) })
1-209: Fix queryDehydrate call to spread dehydrateOptions (not hydrateOptions)In packages/router-ssr-query-core/src/index.ts — server-side queryCache subscription where you call queryStream.enqueue(queryDehydrate(...)), replace the spread of ...hydrateOptions with ...dehydrateOptions so the dehydrate options (not hydrate options) are passed to queryDehydrate.
🧹 Nitpick comments (3)
packages/router-ssr-query-core/src/index.ts (3)
25-27: Simplify hydrateOptions type (Omit likely redundant/confusing)Omitting 'shouldDehydrateQuery' from HydrateOptions is unusual; that predicate belongs to DehydrateOptions. Recommend using HydrateOptions directly for clarity.
Apply:
- hydrateOptions?: Omit<HydrateOptions, 'shouldDehydrateQuery'> + hydrateOptions?: HydrateOptions
48-52: Unsubscribe query cache on render finish to prevent leaksThe subscription persists past render without an unsubscribe. Tie it to onRenderFinished.
Apply:
if (router.isServer) { const sentQueries = new Set<string>() const queryStream = createPushableStream() + let unsubscribe: () => void = () => {} router.options.dehydrate = async (): Promise<DehydratedRouterQueryState> => { - router.serverSsr!.onRenderFinished(() => queryStream.close()) + router.serverSsr!.onRenderFinished(() => { + try { unsubscribe() } finally { queryStream.close() } + }) const ogDehydrated = await ogDehydrate?.() @@ - queryClient.getQueryCache().subscribe((event) => { + unsubscribe = queryClient.getQueryCache().subscribe((event) => {Also applies to: 82-116
181-208: Optional: Strongly type the pushable stream payloadTyping the stream ensures enqueue/value are QueryDehydratedState and improves DX.
Apply:
-type PushableStream = { - stream: ReadableStream - enqueue: (chunk: unknown) => void +type PushableStream<T> = { + stream: ReadableStream<T> + enqueue: (chunk: T) => void close: () => void isClosed: () => boolean error: (err: unknown) => void } -function createPushableStream(): PushableStream { - let controllerRef: ReadableStreamDefaultController - const stream = new ReadableStream({ +function createPushableStream<T = QueryDehydratedState>(): PushableStream<T> { + let controllerRef: ReadableStreamDefaultController<T> + const stream = new ReadableStream<T>({ start(controller) { controllerRef = controller }, }) let _isClosed = false return { stream, - enqueue: (chunk) => controllerRef.enqueue(chunk), + enqueue: (chunk: T) => controllerRef.enqueue(chunk),And update the instantiation:
-const queryStream = createPushableStream() +const queryStream = createPushableStream<QueryDehydratedState>()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/router-ssr-query-core/src/index.ts(6 hunks)
🔇 Additional comments (4)
packages/router-ssr-query-core/src/index.ts (4)
8-12: Type-only imports look goodImporting DehydrateOptions and HydrateOptions as types is correct and treeshake‑friendly.
38-40: Plumbing new options into setup: LGTMAccepting dehydrateOptions and hydrateOptions in the setup signature is consistent with the API expansion.
59-63: Server dehydrate with dehydrateOptions: LGTMPassing dehydrateOptions into the initial queryDehydrate is correct.
123-128: Client hydration with hydrateOptions: LGTMHydrating the initial dehydrated state with hydrateOptions is correct.
luixo
force-pushed
the
fix/query-client-integration-options
branch
from
3924890 to
ea411ce
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/router-ssr-query-core/src/index.ts (1)
104-105: Consider preserving user-definedshouldDehydrateQueryin streaming.When streaming queries, the current implementation spreads
dehydrateOptionsfirst, which means any user-definedshouldDehydrateQuerywould be overridden by the local implementation. This could lead to unexpected behavior if users expect their dehydration logic to be respected during streaming.Consider preserving the user's
shouldDehydrateQuerylogic by combining it with the query hash check:queryStream.enqueue( queryDehydrate(queryClient, { - ...dehydrateOptions, - shouldDehydrateQuery: (query) => { - if (query.queryHash === event.query.queryHash) { - return ( - ogClientOptions.dehydrate?.shouldDehydrateQuery?.(query) ?? true - ) - } - return false - }, + ...dehydrateOptions, + shouldDehydrateQuery: (query) => { + // First check if this is the query we want to stream + if (query.queryHash !== event.query.queryHash) { + return false + } + // Then apply user-defined dehydration logic if provided + const userShouldDehydrate = dehydrateOptions?.shouldDehydrateQuery?.(query) + if (userShouldDehydrate !== undefined) { + return userShouldDehydrate + } + // Fall back to default options + return ogClientOptions.dehydrate?.shouldDehydrateQuery?.(query) ?? true + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/router-ssr-query-core/src/index.ts(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/router-ssr-query-core/src/index.ts
🔇 Additional comments (1)
packages/router-ssr-query-core/src/index.ts (1)
34-40: Well-structured implementation of (de)hydration options.The addition of
dehydrateOptionsandhydrateOptionsparameters provides the necessary flexibility for handling custom serialization scenarios. The implementation correctly threads these options through all relevant hydration and dehydration paths.
| * @link [Guide](https://tanstack.com/router/latest/docs/framework/react/api/router/redirectFunction) | ||
| */ | ||
| handleRedirects?: boolean | ||
| hydrateOptions?: Omit<HydrateOptions, 'shouldDehydrateQuery'> |
There was a problem hiding this comment.
Fix the hydrateOptions type definition to avoid potential issues.
The shouldDehydrateQuery property should not be omitted from HydrateOptions. According to the TanStack Query API, HydrateOptions does not include a shouldDehydrateQuery property - that property belongs to DehydrateOptions. This appears to be a copy-paste error.
Apply this diff to fix the type:
- hydrateOptions?: Omit<HydrateOptions, 'shouldDehydrateQuery'>
+ hydrateOptions?: HydrateOptions📝 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.
| hydrateOptions?: Omit<HydrateOptions, 'shouldDehydrateQuery'> | |
| hydrateOptions?: HydrateOptions |
🤖 Prompt for AI Agents
In packages/router-ssr-query-core/src/index.ts around line 25, the
hydrateOptions type currently omits 'shouldDehydrateQuery' which is incorrect
because HydrateOptions does not include that property (it's on
DehydrateOptions); change the type so hydrateOptions is either HydrateOptions or
a partial/omit that excludes only properties that actually exist on
HydrateOptions (remove the Omit<..., 'shouldDehydrateQuery'> wrapper) or
explicitly type it as HydrateOptions | Partial<HydrateOptions> depending on
desired optionality, ensuring the type no longer references
'shouldDehydrateQuery'.
| .read() | ||
| .then(async function handle({ done, value }) { | ||
| queryHydrate(queryClient, value) | ||
| queryHydrate(queryClient, value, hydrateOptions) |
There was a problem hiding this comment.
🧹 Nitpick
Add null/undefined check before hydrating streamed values.
When reading from the query stream, the value could potentially be undefined in edge cases (e.g., if the stream is closed prematurely or contains no data). This could cause queryHydrate to fail silently or throw an error.
Apply this diff to add a safety check:
- queryHydrate(queryClient, value, hydrateOptions)
+ if (value) {
+ queryHydrate(queryClient, value, hydrateOptions)
+ }📝 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.
| queryHydrate(queryClient, value, hydrateOptions) | |
| if (value) { | |
| queryHydrate(queryClient, value, hydrateOptions) | |
| } |
🤖 Prompt for AI Agents
In packages/router-ssr-query-core/src/index.ts around line 135, the call to
queryHydrate(queryClient, value, hydrateOptions) needs a null/undefined guard
because streamed values may be undefined; update the code to check if value !==
null && value !== undefined (or use value != null) before calling queryHydrate,
and skip or safely handle/return when the value is missing (optionally log or
noop) to prevent errors from attempting to hydrate an empty value.
|
can you please provide a full example where this is needed? |
Just use any non-serializable value in the query data (create a class that holds a value inside of itself). If you still need a full example - can you provide with a minimal boilerplate I should modify? |
|
setting up a serializationAdapter in start should solve this as well. what are you putting into |
Is it implemented yet?
Transformer is a custom extended SuperJSON instance. |
I tried to add Tanstack Query integration to Tanstack Start and it failed with Seroval's error (also crashed the whole app!)
Before that I had my own implementation of Tanstack Query integration and it didn't fail because I (de)serialized data, but with integration that's not an option.
This PR adds such an option.
Summary by CodeRabbit