feat(tanstack-query): default query/mutation options #1260
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds an experimental defaults system for TanStack Query utilities: defines Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Router as createRouterUtils
participant Procedure as createProcedureUtils
participant Utils as Procedure Utils (closure)
Note over App,Router: App initializes router with experimental_defaults
App->>Router: createRouterUtils(client, { experimental_defaults })
Router->>Procedure: createProcedureUtils(client, { path, experimental_defaults: perPropDefaults })
Note over Procedure: createProcedureUtils stores defaults in returned utils
App->>Utils: call util (e.g., queryKey(options))
Utils->>Utils: merged = { ...defaultsForThisUtil, ...options }
Utils-->>App: return key/options computed from merged
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
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 |
Summary of ChangesHello @unnoq, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces an excellent experimental feature for setting default options for TanStack Query utilities, which will improve reusability and simplify code. The implementation is generally well-done, with thorough unit and type tests. However, I've identified a significant design issue with infiniteKey and infiniteOptions where the current function signatures prevent the use of defaults for required parameters, limiting the feature's utility for infinite queries. I've provided detailed feedback and code suggestions to address this, ensuring the feature is consistent and fully functional across all utilities.
More templates
@orpc/ai-sdk
@orpc/arktype
@orpc/client
@orpc/contract
@orpc/experimental-durable-iterator
@orpc/hey-api
@orpc/interop
@orpc/json-schema
@orpc/nest
@orpc/openapi
@orpc/openapi-client
@orpc/otel
@orpc/experimental-pino
@orpc/experimental-publisher
@orpc/experimental-publisher-durable-object
@orpc/experimental-ratelimit
@orpc/react
@orpc/react-query
@orpc/experimental-react-swr
@orpc/server
@orpc/shared
@orpc/solid-query
@orpc/standard-server
@orpc/standard-server-aws-lambda
@orpc/standard-server-fastify
@orpc/standard-server-fetch
@orpc/standard-server-node
@orpc/standard-server-peer
@orpc/svelte-query
@orpc/tanstack-query
@orpc/trpc
@orpc/valibot
@orpc/vue-colada
@orpc/vue-query
@orpc/zod
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR adds support for default query/mutation options in the tanstack-query integration, allowing users to specify default configurations for various utility functions at the router or procedure level. This addresses issue #1051.
Key Changes
- Added
experimental_ProcedureUtilsDefaultsinterface to define default options for all utility methods - Added
experimental_RouterUtilsDefaultstype to support nested default configurations - Modified all utility functions to merge defaults with user-provided options using shallow object spread
- Added comprehensive test coverage for default options functionality
- Added type tests to ensure type safety of default options
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/tanstack-query/src/procedure-utils.ts | Added defaults interface and implementation to merge defaults with user options in all utility methods |
| packages/tanstack-query/src/procedure-utils.test.ts | Added comprehensive test suite for default options functionality |
| packages/tanstack-query/src/procedure-utils.test-d.ts | Added type tests to validate default options type definitions |
| packages/tanstack-query/src/router-utils.ts | Added support for propagating defaults through nested routers using the get utility |
| packages/tanstack-query/src/router-utils.test.ts | Added test for router-level defaults with incorrect expectation for nested access |
| packages/tanstack-query/src/router-utils.test-d.ts | Added type tests for router-level defaults |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Deploying orpc with
|
| Latest commit: |
1ac9fd7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://55a9bb4d.orpc-1qh.pages.dev |
| Branch Preview URL: | https://unnoq-issue1051.orpc-1qh.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/tanstack-query/src/procedure-utils.ts (1)
267-272: Consider documenting the shallow merge behavior.The spread operator performs a shallow merge, meaning nested objects (like
context) are completely replaced rather than deep-merged. For example, if defaults specify{ context: { a: 1 } }and user provides{ context: { b: 2 } }, the result is{ context: { b: 2 } }(not{ a: 1, b: 2 }).This is likely intentional and simpler, but it may be worth documenting this behavior for users who expect deep merging of options like
context.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/tanstack-query/src/procedure-utils.test-d.ts(2 hunks)packages/tanstack-query/src/procedure-utils.test.ts(1 hunks)packages/tanstack-query/src/procedure-utils.ts(6 hunks)packages/tanstack-query/src/router-utils.test-d.ts(2 hunks)packages/tanstack-query/src/router-utils.test.ts(1 hunks)packages/tanstack-query/src/router-utils.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
packages/tanstack-query/src/procedure-utils.test-d.ts (1)
packages/tanstack-query/src/procedure-utils.ts (2)
experimental_ProcedureUtilsDefaults(149-245)ProcedureUtils(26-147)
packages/tanstack-query/src/procedure-utils.test.ts (1)
packages/tanstack-query/src/procedure-utils.ts (1)
createProcedureUtils(260-449)
packages/tanstack-query/src/procedure-utils.ts (2)
packages/tanstack-query/src/types.ts (7)
QueryKeyOptions(37-39)QueryOptionsIn(41-44)experimental_StreamedKeyOptions(54-56)experimental_StreamedQueryOutput(15-15)experimental_LiveQueryOutput(16-16)InfiniteOptionsIn(65-68)MutationOptionsIn(79-81)packages/tanstack-query/src/key.ts (1)
generateOperationKey(3-12)
packages/tanstack-query/src/router-utils.ts (1)
packages/shared/src/array.ts (1)
toArray(1-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: lint
- GitHub Check: test
- GitHub Check: publish-commit
- GitHub Check: Cloudflare Pages
🔇 Additional comments (23)
packages/tanstack-query/src/procedure-utils.test.ts (6)
349-395: LGTM! Comprehensive test coverage for queryKey defaults.The tests correctly verify that defaults are applied when no options are provided and that user options properly override defaults. The custom queryKey test case is a good addition to ensure queryKey defaults work correctly.
397-425: LGTM! queryOptions defaults tests look correct.Tests verify both the
staleTimepassthrough and input default application, with proper override behavior.
427-483: LGTM! experimental_streamedKey and experimental_streamedOptions tests are well-structured.Both key generation and options defaults are tested with proper override verification.
485-541: LGTM! experimental_liveKey and experimental_liveOptions tests are consistent.The pattern follows the same structure as other key/options tests, maintaining consistency.
543-608: LGTM! infiniteKey and infiniteOptions tests handle the unique signature well.The tests correctly verify that defaults with queryKey work, and that user-provided
inputandinitialPageParamoverride defaults. The staleTime override test for infiniteOptions is also properly structured.
610-664: LGTM! mutationKey and mutationOptions tests complete the coverage.Testing callback defaults (onSuccess) is a good choice as it validates that function-type defaults work correctly.
packages/tanstack-query/src/router-utils.test.ts (1)
61-93: LGTM! Critical test for defaults scoping behavior.This test correctly validates that:
- Defaults are passed to the correct procedure utilities (line 80-83)
- Nested routes that don't have explicit defaults don't inherit parent defaults (lines 89-92)
The scoping behavior tested here is essential for the feature's correctness.
packages/tanstack-query/src/router-utils.ts (4)
15-20: Well-designed recursive type for router defaults.The conditional type correctly maps
Clienttypes toexperimental_ProcedureUtilsDefaultsand nested clients to recursiveexperimental_RouterUtilsDefaults. The optional markers (?) on line 19 are appropriate since not all routes need defaults.
22-28: Good addition of the TODO comment for v2 cleanup.The generic interface with default type parameter maintains backward compatibility while enabling type-safe defaults.
43-46: Confirmexperimental_defaultsis intentionally passed to root procedure utils.When
experimental_defaultsis provided at the router level (not nested), it gets passed directly tocreateProcedureUtils. This means if someone provides{ queryOptions: { staleTime: 1000 } }at the root, it would apply to the root client if it's also a procedure. Verify this is the intended behavior per the PR objectives.
59-63: Confirmed: Thegetutility correctly returnsundefinedfor missing keys.The
getfunction inpackages/shared/src/object.ts(lines 83-95) is implemented to returnundefinedwhen keys don't exist. If the traversal encounters a non-object value or a missing key, the function returnsundefined(line 88 explicitly handles non-objects; line 94 implicitly returnsundefinedwhen accessing a missing key). This ensures child routes won't accidentally inherit parent defaults.packages/tanstack-query/src/procedure-utils.ts (7)
149-245: Well-documented defaults interface with comprehensive coverage.Each utility method has a corresponding defaults field with proper JSDoc links. The use of
Partial<>wrappers is appropriate since all default fields should be optional.
247-258: Good addition of TODO comment for v2 cleanup.The generic interface pattern with default type parameters maintains backward compatibility while enabling type-safe defaults.
274-299: LGTM! queryOptions correctly merges defaults and delegates to queryKey.The defaults merge happens before calling
utils.queryKey(optionsIn), ensuring the key generation also respects defaults. The spread order in the return object (...optionsIn, queryKey) correctly prioritizes the computed queryKey.
301-342: LGTM! experimental_streamedKey and experimental_streamedOptions follow the established pattern.Defaults merging is consistent with other methods.
344-382: LGTM! experimental_liveKey and experimental_liveOptions implementations are consistent.The pattern matches other utility methods.
384-419: LGTM! infiniteKey and infiniteOptions handle defaults correctly.The defaults merge before invoking
optionsIn.input(optionsIn.initialPageParam)ensures user-provided values take precedence.
421-445: LGTM! mutationKey and mutationOptions complete the implementation.All utility methods now consistently support experimental defaults.
packages/tanstack-query/src/router-utils.test-d.ts (3)
6-8: LGTM! Proper imports for type testing.The imports include both the types being tested and the factory function.
41-66: Thorough type tests for RouterUtilsDefaults structure.The tests correctly verify:
- Router-level defaults are NOT procedure defaults (lines 44-46, 48-50)
- Leaf procedure defaults have the correct type (lines 52-54, 60-65)
- Optional chaining on nested access reflects the
?in the type definition (line 56)
68-84: Good integration test for createRouterUtils with defaults.The test verifies that:
- Deeply nested defaults are type-safe (nested.ping.mutationOptions)
- Callback types are correctly inferred (output type in onSuccess)
- Return type matches RouterUtils regardless of defaults configuration
packages/tanstack-query/src/procedure-utils.test-d.ts (2)
5-5: Import ofexperimental_ProcedureUtilsDefaultsis appropriateThe added type-only import is required for the new defaults tests below and keeps the runtime surface unchanged.
750-939: I'll verify the review comment by checking whether thenoUnusedLocalssetting is configured and examining the code to confirm the concern about unused_invalidvariables.
<function_calls>
#!/bin/bashCheck TypeScript config for noUnusedLocals setting
find . -name "tsconfig.json" -type f | head -20
</function_calls>Let me search for the TypeScript configuration and check the noUnusedLocals setting:
<function_calls>
#!/bin/bashFind all tsconfig.json files and check for noUnusedLocals setting
fd "tsconfig.json" -x cat {} + | grep -A 5 -B 5 "noUnusedLocals" || echo "noUnusedLocals setting not found in any tsconfig files"
</function_calls>Now let me examine the actual test file to see the unused
_invalidvariables:
<function_calls>
#!/bin/bashGet the test file and show lines around the _invalid declarations
cat -n packages/tanstack-query/src/procedure-utils.test-d.ts | sed -n '750,939p' | grep -A 2 -B 2 "_invalid"
</function_calls><function_calls>
#!/bin/bashCount the number of _invalid declarations in the specified line range
sed -n '750,939p' packages/tanstack-query/src/procedure-utils.test-d.ts | grep -c "const _invalid"
</function_calls><function_calls>
#!/bin/bashCheck the entire file for references to _invalid to confirm they're unused
cd packages/tanstack-query/src && grep -n "_invalid" procedure-utils.test-d.ts | head -30
</function_calls><function_calls>
#!/bin/bashCheck if there are any eslint directives or ts-ignore comments near _invalid declarations
sed -n '750,939p' packages/tanstack-query/src/procedure-utils.test-d.ts | grep -B 1 "_invalid" | head -40
</function_calls>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/content/docs/integrations/tanstack-query.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: publish-commit
- GitHub Check: test
- GitHub Check: lint
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
apps/content/docs/integrations/tanstack-query.md (2)
229-268: Verify the duplication mentioned in the AI summary.The AI summary indicates the "Default Options" section appears in two locations within the document, which would create maintenance burden. Please confirm whether the content is actually duplicated elsewhere in the file and consolidate if necessary.
229-268: Documentation is clear and well-structured.The "Default Options" section effectively explains the experimental feature with concrete examples showing configuration, usage, and override behavior. The placement after "Reactive Options" is logical, and the experimental naming convention appropriately signals the feature's maturity level.
The example clearly demonstrates configuring defaults across different operation types (query, infinite, mutation), making it practical for users.
Fixes #1051
Summary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.