fix(tanstack): query result $optimistic typing #372
Conversation
|
Caution Review failedThe pull request is closed. 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 a generic Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Areas requiring attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
|
Claude finished @ymc9's task —— View job Code Review AnalysisI've completed my analysis of this PR that fixes Todo List
✅ Overall AssessmentThis is a well-implemented typing fix that correctly addresses the missing 📝 Code Quality & Best PracticesExcellent:
Type Design:
🔍 Implementation DetailsKey Changes:
🧪 Test CoverageComprehensive testing:
The infinite query exclusion is intentional and correct - optimistic updates don't apply to paginated data. 🐛 Potential IssuesNone identified. The implementation is solid:
⚡ Performance ConsiderationsExcellent:
🔒 Security ConcernsNone. This is a TypeScript-only change with no security implications. 💡 SuggestionsThe implementation is already excellent, but minor notes:
🎯 SummaryThis PR successfully fixes a typing gap in the TanStack query integration. The Recommendation: ✅ Approve |
There was a problem hiding this comment.
Pull Request Overview
This PR adds TypeScript type support for an $optimistic field on query results to indicate when data is from optimistic updates. The WithOptimistic<T> utility type adds an optional $optimistic?: boolean field to single objects and array elements, but intentionally excludes this field from infinite query results.
- Introduces a new
WithOptimistic<T>utility type that adds$optimistic?: booleanfield to objects and array elements - Updates
ModelQueryResulttypes across React, Vue, and Svelte to wrap results withWithOptimistic<T> - Adds typing tests confirming
$optimisticis available onuseFindFirstanduseFindManyresults but NOT onuseInfiniteFindManyresults
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/clients/tanstack-query/src/utils/types.ts | Defines the new WithOptimistic<T> utility type |
| packages/clients/tanstack-query/src/react.ts | Updates ModelQueryResult and ModelSuspenseQueryResult to use WithOptimistic<T> |
| packages/clients/tanstack-query/src/vue.ts | Updates ModelQueryResult to use WithOptimistic<T> |
| packages/clients/tanstack-query/src/svelte.ts | Updates ModelQueryResult to use WithOptimistic<T> |
| packages/clients/tanstack-query/test/react-typing-test.ts | Adds type tests verifying $optimistic field availability on appropriate query results |
| packages/clients/tanstack-query/test/vue-typing-test.ts | Adds type tests verifying $optimistic field availability on appropriate query results |
| packages/clients/tanstack-query/test/svelte-typing-test.ts | Adds type tests verifying $optimistic field availability on appropriate query results |
Comments suppressed due to low confidence (3)
packages/clients/tanstack-query/src/react.ts:110
- The
ModelInfiniteQueryResulttype is not updated to useWithOptimistic<T>while regularModelQueryResultandModelSuspenseQueryResultare. While the test cases confirm this is intentional (using@ts-expect-errorto verify$optimisticis not available), the inconsistency could be confusing. Consider adding a code comment explaining why infinite query results intentionally don't include the$optimisticfield.
export type ModelInfiniteQueryResult<T> = UseInfiniteQueryResult<T, DefaultError> & { queryKey: QueryKey };
packages/clients/tanstack-query/src/vue.ts:95
- The
ModelInfiniteQueryResulttype is not updated to useWithOptimistic<T>whileModelQueryResultis. Consider adding a code comment explaining why infinite query results intentionally don't include the$optimisticfield to clarify this design decision for future maintainers.
export type ModelInfiniteQueryResult<T> = UseInfiniteQueryReturnType<T, DefaultError> & { queryKey: QueryKey };
packages/clients/tanstack-query/src/svelte.ts:107
- The
ModelInfiniteQueryResulttype is not updated to useWithOptimistic<T>whileModelQueryResultis. Consider adding a code comment explaining why infinite query results intentionally don't include the$optimisticfield to maintain consistency with the design decision across all framework adapters.
export type ModelInfiniteQueryResult<T> = Readable<
UnwrapStore<CreateInfiniteQueryResult<T, DefaultError>> & {
queryKey: QueryKey;
}
>;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Claude finished @ymc9's task —— View job Claude finished @ymc9's task —— View job Code Review AnalysisI've completed my analysis of this PR that fixes Todo List
✅ Overall AssessmentThis is a well-implemented typing fix that correctly addresses the missing 📝 Code Quality & Best PracticesExcellent:
Type Design:
🔍 Implementation DetailsKey Changes:
🧪 Test CoverageComprehensive testing:
The infinite query exclusion is intentional and correct - optimistic updates don't apply to paginated data. 🐛 Potential IssuesNone identified. The implementation is solid:
⚡ Performance ConsiderationsExcellent:
🔒 Security ConcernsNone. This is a TypeScript-only change with no security implications. 💡 SuggestionsThe implementation is already excellent, but minor notes:
🎯 SummaryThis PR successfully fixes a typing gap in the TanStack query integration. The Recommendation: ✅ Approve |
Summary by CodeRabbit
New Features
Tests