fix(svelte-query): allow per-query persisters in createQuery#10885
fix(svelte-query): allow per-query persisters in createQuery#10885JetProc wants to merge 1 commit into
Conversation
Query persisters are allowed as query options in core, but the Svelte createQuery accessor path let the persister type participate in query key and data inference. That could make a valid per-query persister reject overload resolution or leave result data as unknown. This keeps the runtime path unchanged and limits the type relaxation to the createQuery option surface, with a regression test covering the reported usage. Constraint: Match TanStack Query's Svelte package boundaries and keep the fix type-only Rejected: Relax CreateBaseQueryOptions | it also affected internal defaultQueryOptions and infinite query typing Confidence: high Scope-risk: narrow Directive: Keep queryFn as the source of createQuery result data inference when persister is present Tested: pnpm run test:pr Tested: corepack pnpm nx run @tanstack/svelte-query:test:types Tested: corepack pnpm nx run @tanstack/svelte-query:test:lib Tested: corepack pnpm nx run @tanstack/svelte-query:test:eslint Tested: corepack pnpm nx run @tanstack/svelte-query:test:build
📝 WalkthroughWalkthroughThis PR fixes the type definition for ChangesPer-query persister override support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/svelte-query/tests/createQuery/createQuery.test-d.ts (1)
11-25: ⚡ Quick winConsider using the
QueryPersistertype directly for stronger type validation.The test manually constructs a persister signature rather than using
QueryPersister<string[], ['todos']>from@tanstack/query-core. While the hand-crafted signature validates the basic fix, using the actualQueryPersistertype would ensure the test catches any subtle incompatibilities between the type definition intypes.tsand real persister implementations.♻️ Suggested refactor to use QueryPersister type
- const persister = undefined as unknown as <T, TQueryKey extends QueryKey>( - queryFn: (context: QueryFunctionContext<TQueryKey>) => T | Promise<T>, - context: QueryFunctionContext<TQueryKey>, - query: Query, - ) => Promise<T> + const persister: QueryPersister<string[], ['todos']> = + undefined as anyAlternatively, if you want to keep the generic formulation to test that the type accepts generic persisters:
+ import type { QueryPersister } from '`@tanstack/query-core`' + - const persister = undefined as unknown as <T, TQueryKey extends QueryKey>( - queryFn: (context: QueryFunctionContext<TQueryKey>) => T | Promise<T>, - context: QueryFunctionContext<TQueryKey>, - query: Query, - ) => Promise<T> + const persister = undefined as unknown as QueryPersister<any, any>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/svelte-query/tests/createQuery/createQuery.test-d.ts` around lines 11 - 25, Replace the hand-constructed persister function type in the test with the official QueryPersister type from `@tanstack/query-core` to ensure correct, stronger type checking: import QueryPersister and declare persister as QueryPersister<string[], ['todos']> (used in the createQuery call) so the test validates real persister compatibility with createQuery and the types defined in types.ts.packages/svelte-query/src/types.ts (1)
45-50: ⚡ Quick winType safety trade-off: Svelte’s
persister?: QueryPersister<any, any>erases persister/query coupling
packages/svelte-query/src/types.tswidenspersistertoQueryPersister<any, any>, which removes the type-level link between the persister’squeryFn(and key/pageParam expectations) and the query’sTQueryFnData/TQueryKey/TPageParam—so TypeScript won’t catch mismatches thatquery-core’spersister?: QueryPersister<TQueryFnData, NoInfer<TQueryKey>, TPageParam>would. The persister shape is still enforced, but its generic safety is effectively gone. Consider documenting this trade-off (and/or investigating a stricter alternative that preserves inference) since other query packages don’t appear to use the sameQueryPersister<any, any>erasure.export type CreateQueryOptions<...> = OmitKeyof< CreateBaseQueryOptions<...>, 'persister' > & { persister?: QueryPersister<any, any> }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/svelte-query/src/types.ts` around lines 45 - 50, The persister generic is being erased by using QueryPersister<any, any> which loses the link to the query's generics; update the CreateQueryOptions type in packages/svelte-query/src/types.ts (the CreateQueryOptions and referenced CreateBaseQueryOptions/persister entry) to preserve type coupling by typing persister as QueryPersister<TQueryFnData, NoInfer<TQueryKey>, TPageParam> (or the equivalent generic parameters used by query-core) instead of any, and import/alias NoInfer if required; if preserving inference is not feasible, add a concise code comment above CreateQueryOptions documenting this deliberate trade-off and why the generics were widened.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/svelte-query/src/types.ts`:
- Around line 45-50: The persister generic is being erased by using
QueryPersister<any, any> which loses the link to the query's generics; update
the CreateQueryOptions type in packages/svelte-query/src/types.ts (the
CreateQueryOptions and referenced CreateBaseQueryOptions/persister entry) to
preserve type coupling by typing persister as QueryPersister<TQueryFnData,
NoInfer<TQueryKey>, TPageParam> (or the equivalent generic parameters used by
query-core) instead of any, and import/alias NoInfer if required; if preserving
inference is not feasible, add a concise code comment above CreateQueryOptions
documenting this deliberate trade-off and why the generics were widened.
In `@packages/svelte-query/tests/createQuery/createQuery.test-d.ts`:
- Around line 11-25: Replace the hand-constructed persister function type in the
test with the official QueryPersister type from `@tanstack/query-core` to ensure
correct, stronger type checking: import QueryPersister and declare persister as
QueryPersister<string[], ['todos']> (used in the createQuery call) so the test
validates real persister compatibility with createQuery and the types defined in
types.ts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 249ea428-b8d0-4657-961b-5aa46f920c61
📒 Files selected for processing (3)
.changeset/svelte-query-persister-options.mdpackages/svelte-query/src/types.tspackages/svelte-query/tests/createQuery/createQuery.test-d.ts
1 participant
🎯 Changes
Closes #9729.
createQueryaccepts the core query option surface through its Svelte accessor, but a query-levelpersistercould participate in query key and data inference. That could make valid per-query persister values fail overload resolution or leavedatainferred asunknown.This keeps
persisterfrom drivingCreateQueryOptionsinference while leaving the runtime behavior unchanged. A type regression test covers passing a per-query persister directly tocreateQuery.✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
createQuerytype definitions to support per-query persister configuration.