Query mutation options in core - #10973
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (22)
🚧 Files skipped from review as they are similar to previous changes (22)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughAdds framework-agnostic ChangesFramework-agnostic queryOptions and mutationOptions
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to This change adds shared query and mutation option helpers with associated type and framework coverage; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Caller
participant CoreHelpers
participant FrameworkAdapter
participant QueryObserver
Caller->>CoreHelpers: create queryOptions or mutationOptions
CoreHelpers-->>Caller: typed options with preserved inference
Caller->>FrameworkAdapter: pass core options
FrameworkAdapter->>QueryObserver: create observer with options
QueryObserver-->>Caller: typed query or mutation result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
| export { queryOptions } from './queryOptions' | ||
| export type { CoreQueryOptions } from './queryOptions' | ||
| export { mutationOptions } from './mutationOptions' | ||
| export type { CoreMutationOptions } from './mutationOptions' |
There was a problem hiding this comment.
since adapters re-export the core and then export their own things, they won’t have access to the queryOptions from the core, right? is that on purpose?
There was a problem hiding this comment.
Yes. I don't think someone wanting to define shared query options for multiple frameworks would import from a framework specific package.
| const { data } = useQuery(options) | ||
| expectTypeOf(data).toEqualTypeOf< | ||
| { id: string; title: string } | undefined | ||
| >() |
There was a problem hiding this comment.
with this, what’s the difference between useQuery(queryOptions) and useQuery(coreQueryOption) ?
in other words, if they are compatible, why would e.g. the react adapter need a separate queryOptions and not just re-export and let users use the one from the core?
There was a problem hiding this comment.
queryOptions from each library could extend the coreQueryOption, like with deferStream in Solid or RefOrGetter pattern in Vue.
|
I avoided changing the types of the adapters to reuse the new types in core to keep the changes of this PR small (only test changes for most adapters). The code query options could be something that adapters completely reuse. |
I guess you mean re-use on type-level, because at runtime, they all don’t do anything? |
Yes |
|
okay, let’s do it 🔥 . can you fix the conflicts ? |
ff39c7b to
51a48d8
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
🎯 Changes
Adds
queryOptionsandmutationOptionsto the core library.This was requested at #10735 and #9258 discussions. Having those function in core is useful for defining shared query options for a framework agnostic layer, or for server side usage. Specially for using query keys for type-safe interactions with the core imperative API.
Most of the frameworks accept the shared interface from core without changes. The exception is Vue, that require a small change in the interface since
MaybeRefOrGetterwasn't compatible. I added runtime test for Vue to demonstrate that it works with the core query options, with the same shape of() => coreOptionsas Solid/Angular/Lit/Svelte.About docs: There is a lot of outdated docs, so I excluded the doc generation in the PR. I didn't add docs mentioning that core has this new functions.
Pending questions:
CoreQueryOptionsandCoreMutationOptionsok names for the interface the newqueryOptionsandmutationOptionsrequires? The core library already hasQueryOptionsandMutationOptions. The newCoreQueryOptionsandCoreMutationOptionsare the equivalent ofQueryOptionsandMutationOptionsexported by each individual adapter. The new types can't replace the base options alredy defined in core.CoreQueryOptionsandCoreMutationOptionsbe excluded from the re-export from core? How? Each adapter exportsqueryOptionsandmutationOptions, so a consumer can't import the implementation defined in core if the consumer hasn't installed@tanstack/query-coredirectly, but they can consumeCoreQueryOptionsandCoreMutationOptions.useQueryinterface of Vue had to be extended, there is probably something that can be done there to simplify the types.✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
Release Notes
queryOptionsandmutationOptionshelpers for reusable query and mutation configurations.