Improvements to methods in Tanstack React Query package #6529
Conversation
|
This PR was not deployed automatically as @Nick-Lucas does not have access to the Railway project. In order to get automatic PR deploys, please add @Nick-Lucas to the project inside the project settings page. |
WalkthroughThe changes span multiple files across the tanstack-react-query package and its upgrade module. Key updates include renaming the exported type Changes
Sequence Diagram(s)sequenceDiagram
participant Component as React Component
participant TRPCProvider as TRPCProvider
participant ClientContext as TRPCClientContext
Component->>TRPCProvider: Render within Provider
TRPCProvider->>ClientContext: Provide TRPC Client instance
Component->>ClientContext: Invoke useTRPCClient()
ClientContext-->>Component: Return TRPC client instance
sequenceDiagram
participant Component as React Component
participant Mutation as useMutation
participant QueryClient as React Query Client
Component->>Mutation: Trigger mutation (onMutate)
Mutation->>QueryClient: Cancel queries & apply optimistic update
alt Mutation Fails
Mutation->>QueryClient: onError restores previous state
else Mutation Succeeds
Mutation->>QueryClient: onSettled invalidates queries
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Pro Plan! Simply use the command 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (14)
🔇 Additional comments (1)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
@trpc/client
@trpc/next
@trpc/react-query
@trpc/server
@trpc/tanstack-react-query
@trpc/upgrade
commit: |
Nick-Lucas
changed the title
|
Looking for this PR! My usage of |
juliusmarminge
left a comment
juliusmarminge
left a comment
There was a problem hiding this comment.
Feel free to revert my last commit
![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
🧹 Nitpick comments (1)
packages/tanstack-react-query/src/internals/types.ts (1)
21-25: Consider usingundefinedinstead ofvoidin union types.The static analysis tool correctly flags that using
voidin a union type can be confusing. While functionally similar in this context, usingundefinedis more conventional and semantically clearer.Apply this diff to address the lint warning:
/** * @remark `void` is here due to https://github.com/trpc/trpc/pull/4374 */ type CursorInput = { cursor?: any }; -export type OptionalCursorInput = CursorInput | void; +export type OptionalCursorInput = CursorInput | undefined;🧰 Tools
🪛 Biome (1.9.4)
[error] 25-25: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
packages/tanstack-react-query/src/internals/createOptionsProxy.ts(10 hunks)packages/tanstack-react-query/src/internals/types.ts(2 hunks)packages/tanstack-react-query/test/queryKeyable.test.tsx(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/tanstack-react-query/test/queryKeyable.test.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
packages/tanstack-react-query/src/internals/types.ts
[error] 25-25: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: E2E-tests (vercel-edge-runtime)
- GitHub Check: E2E-tests (tanstack-start)
- GitHub Check: E2E-tests (standalone-server)
- GitHub Check: E2E-tests (soa)
- GitHub Check: E2E-tests (next-prisma-websockets-starter)
- GitHub Check: E2E-tests (next-prisma-todomvc)
- GitHub Check: E2E-tests (next-prisma-starter)
- GitHub Check: E2E-tests (next-minimal-starter)
- GitHub Check: E2E-tests (next-formdata)
- GitHub Check: E2E-tests (minimal-react)
- GitHub Check: E2E-tests (fastify-server)
- GitHub Check: E2E-tests (express-server)
- GitHub Check: E2E-tests (express-minimal)
- GitHub Check: E2E-tests (cloudflare-workers)
- GitHub Check: E2E-tests (.test/ssg)
- GitHub Check: E2E-tests (.test/internal-types-export)
- GitHub Check: E2E-tests (.test/diagnostics-big-router)
- GitHub Check: E2E-tests (.experimental/next-app-dir)
- GitHub Check: test
- GitHub Check: Lint and auto-fix
🔇 Additional comments (8)
packages/tanstack-react-query/src/internals/types.ts (2)
7-9: Great addition of theWithRequiredutility type.This utility type is a valuable addition that enforces required properties in object types, enhancing type safety throughout the codebase. It's correctly implemented using mapped types.
30-32: Clean refactoring of theExtractCursorTypetype.This change improves code organization by reusing the
CursorInputtype rather than repeating the same structure, making the code more maintainable.packages/tanstack-react-query/src/internals/createOptionsProxy.ts (6)
48-69: Good renaming fromDecorateQueryKeyabletoDecorateRouterKeyable.The interface rename and method changes (
queryKey→pathKey,queryFilter→pathFilter) align better with their purpose of working with router paths. The documentation updates clearly explain the functionality and provide relevant links to official documentation.
96-151: Well-structuredDecorateInfiniteQueryProcedureinterface.This new interface adds proper support for infinite queries with well-documented methods that align with the PR objectives. The implementation is type-safe and comprehensive, covering query keys, options, and filters specifically for infinite queries.
152-196: Good extension ofDecorateQueryProcedurefromDecorateRouterKeyable.This change correctly establishes the inheritance relationship between query procedures and router keyable functionality, improving the overall type hierarchy and preventing code duplication.
228-231: Smart conditional type for infinite query procedures.The conditional inclusion of
DecorateInfiniteQueryProcedurebased on whether the input extendsOptionalCursorInputelegantly handles the requirement mentioned in the PR objectives of making infinite methods only appear when a cursor is present.
346-354: Simplified implementation of path-related methods.The
pathKeyimplementation now correctly callsgetQueryKeyInternalwith just the path parameter, addressing the PR objective of supportinggetQueryKeyInternalwithout needing second and third parameters for route methods.
386-394: Good implementation of infinite query methods.The new methods for infinite queries are well implemented, properly using the internal utilities and maintaining consistency with the existing query methods. These changes directly address the PR objectives of adding
infiniteQueryKeyandinfiniteQueryFilter.
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)
packages/tanstack-react-query/src/internals/createOptionsProxy.ts(10 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: E2E-tests (vercel-edge-runtime)
- GitHub Check: E2E-tests (tanstack-start)
- GitHub Check: E2E-tests (standalone-server)
- GitHub Check: E2E-tests (soa)
- GitHub Check: E2E-tests (next-prisma-websockets-starter)
- GitHub Check: E2E-tests (next-prisma-todomvc)
- GitHub Check: E2E-tests (next-prisma-starter)
- GitHub Check: E2E-tests (next-minimal-starter)
- GitHub Check: E2E-tests (next-formdata)
- GitHub Check: E2E-tests (minimal-react)
- GitHub Check: E2E-tests (fastify-server)
- GitHub Check: E2E-tests (express-server)
- GitHub Check: E2E-tests (express-minimal)
- GitHub Check: E2E-tests (cloudflare-workers)
- GitHub Check: E2E-tests (.test/ssg)
- GitHub Check: E2E-tests (.test/internal-types-export)
- GitHub Check: E2E-tests (.test/diagnostics-big-router)
- GitHub Check: E2E-tests (.experimental/next-app-dir)
- GitHub Check: test
🔇 Additional comments (6)
packages/tanstack-react-query/src/internals/createOptionsProxy.ts (6)
96-151: Good addition: New interface for infinite query proceduresAdding the
DecorateInfiniteQueryProcedureinterface with dedicated methods for infinite queries improves type safety and makes it clearer when and how to use infinite queries.The JSDoc comments are thorough and include links to relevant documentation, which is excellent for developer experience.
229-231: Fixed lint issue: Using Record<string, never> instead of {}You've correctly replaced
{}withRecord<string, never>as recommended in previous reviews to address the lint warning about using empty object types.
346-354: Functionality improvement: Simplified path key methodsThe implementation of
pathKeynow usesgetQueryKeyInternalwith fewer parameters, making it easier to invalidate all queries for a route without needing input parameters.This aligns with the PR objective to "allow for the complete invalidation of routes and single procedures without requiring input."
386-394: New feature: Added infinite query key and filter methodsThe addition of
infiniteQueryKeyandinfiniteQueryFiltermethods provides dedicated support for infinite queries, making them first-class citizens in the API.This implementation matches the PR objective to "introduce infiniteQueryKey and infiniteQueryFilter."
263-267:Details
❓ Verification inconclusive
Ensure backward compatibility
With the transition from query-based to router/path-based terminology in the API, ensure that sufficient deprecation notices and migration paths are provided for users upgrading from previous versions.
Consider adding temporary backwards compatibility shims if this is a breaking change.
🏁 Script executed:
#!/bin/bash # Check for any migrations or upgrade scripts fd "upgrade|migration" --type f packages/ # Check if there are any re-export patterns that might provide backwards compatibility rg -A 5 -B 5 "export.*DecorateQueryKeyable|export.*type.*=.*DecorateRouterKeyable" packages/Length of output: 138
Backward Compatibility Verification Needed
The transition from query-based to router/path-based terminology is clear, but the automated checks for upgrade or migration scripts and re-export patterns did not reveal the existence of deprecation notices or backward compatibility shims. Please manually verify that:
- Sufficient deprecation warnings and migration guides are provided to support users upgrading from previous versions.
- Temporary backward compatibility shims or re-export patterns are implemented where necessary to bridge the terminology change.
This verification is essential to ensure that users are well-informed and that the migration path is smooth.
241-260:Details
❓ Verification inconclusive
API renaming: DecoratedRouterRecord
Renaming
DecoratedProcedureUtilsRecordtoDecoratedRouterRecordmaintains consistency with the other interface renamings in this PR.Ensure this rename is documented in any migration guides.
🏁 Script executed:
#!/bin/bash # Check if this type is exported and used in other files that might need updating rg "DecoratedProcedureUtilsRecord|DecoratedRouterRecord" packages/ --type tsLength of output: 78
Manual Verification Required: Update Migration Documentation for DecoratedRouterRecord
- The renaming from
DecoratedProcedureUtilsRecordtoDecoratedRouterRecordhas been implemented for API consistency.- The automated grep search in the
packages/directory did not return any additional references. However, please manually verify that:
- Any usage of the old name in migration guides and external documentation is updated.
- The exported type in
packages/tanstack-react-query/src/internals/createOptionsProxy.tsis consistent with upstream dependencies.
| export interface DecorateRouterKeyable { | ||
| /** | ||
| * Calculate the TanStack Query Key for a Route | ||
| * Calculate the TanStack Query Key for any path, could be used to invalidate every procedure beneath this path | ||
| * | ||
| * @see https://tanstack.com/query/latest/docs/framework/react/guides/query-keys | ||
| * @see https://trpc.io/docs/client/tanstack-react-query/usage#queryKey | ||
| */ | ||
| queryKey: () => TRPCQueryKey; | ||
| pathKey: () => TRPCQueryKey; | ||
|
|
||
| /** | ||
| * Calculate a TanStack Query Filter for a Route | ||
| * Calculate a TanStack Query Filter for any path, could be used to manipulate every procedure beneath this path | ||
| * | ||
| * @see https://tanstack.com/query/latest/docs/framework/react/guides/filters | ||
| * @see https://trpc.io/docs/client/tanstack-react-query/usage#queryFilter | ||
| */ | ||
| queryFilter: (input?: undefined, filters?: QueryFilters) => QueryFilters; | ||
| pathFilter: ( | ||
| filters?: QueryFilters<unknown, unknown, unknown, TRPCQueryKey>, | ||
| ) => WithRequired< | ||
| QueryFilters<unknown, unknown, unknown, TRPCQueryKey>, | ||
| 'queryKey' | ||
| >; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
API change: Renamed interface and methods
The interface has been renamed from DecorateQueryKeyable to DecorateRouterKeyable, with methods renamed from queryKey/queryFilter to pathKey/pathFilter. This better represents their purpose for invalidating entire routes rather than specific queries.
Make sure this API change is well-documented in migration guides to help users update their code accordingly.
🏁 Script executed:
#!/bin/bash
# Check if there are upgrade guides or migration docs for these API changes
fd "migration|upgrade" --type f -e md -e mdx packages/Length of output: 56
API Change Notification: Migration Guide Update Required
The updated code in packages/tanstack-react-query/src/internals/createOptionsProxy.ts now renames the interface from DecorateQueryKeyable to DecorateRouterKeyable and changes the method names from queryKey/queryFilter to pathKey/pathFilter for better clarity regarding route invalidation. However, our search for migration or upgrade guides (using a file search for "migration" or "upgrade" within the packages/ directory) returned no results.
Please add or update the migration documentation to detail these API changes so that users understand how to update their code accordingly. Consider including:
- A clear explanation of the API renaming.
- A code diff snippet highlighting the changes.
- Guidance for users on how to adapt their existing code.
|
This pull request has been locked because we are very unlikely to see comments on closed issues. If you think, this PR is still necessary, create a new one with the same branch. Thank you. |
Closes #
🎯 Changes
What changes are made in this PR? Is it a feature or a bug fix?
✅ Checklist
Summary by CodeRabbit
New Features
Refactor
Documentation
Tests