Skip to content

Conversation

@ymc9
Copy link
Member

@ymc9 ymc9 commented Nov 6, 2025

Summary by CodeRabbit

  • Refactor
    • Optimized internal type definitions to reduce code duplication and improve maintainability across the TanStack Query client. No changes to existing functionality or public APIs.

Copilot AI review requested due to automatic review settings November 6, 2025 06:00
@coderabbitai
Copy link

coderabbitai bot commented Nov 6, 2025

Walkthrough

The PR refactors the WithOptimistic<T> type definition by introducing an internal helper type WithOptimisticFlag<T> to consolidate the $optimistic property augmentation logic. The exported type now delegates to this helper for both array and non-array cases, reducing code duplication.

Changes

Cohort / File(s) Summary
Type refactoring for optimistic updates
packages/clients/tanstack-query/src/utils/types.ts
Introduced internal WithOptimisticFlag<T> type to centralize $optimistic property definition. Refactored exported WithOptimistic<T> to use the helper type for both array elements and non-array types, replacing inline property augmentation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5–10 minutes

  • Isolated type-level refactoring in a single file with no runtime logic changes
  • Straightforward DRY improvement by extracting a reusable helper type
  • Low complexity; mainly involves verifying that the new type structure maintains equivalent behavior to the original

Possibly related PRs

Poem

🐰 A rabbit hops through types so fine,
Extracting helpers, line by line,
Where $optimistic flags now gleam,
One helper type fulfills the dream!

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main change: refactoring the WithOptimistic type to use an internal helper type for cleaner optimistic typing in the tanstack-query package.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/optimistic-typing

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 56acc1c and 8509bdb.

📒 Files selected for processing (1)
  • packages/clients/tanstack-query/src/utils/types.ts (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: CodeQL analysis (javascript-typescript)
  • GitHub Check: claude-review
  • GitHub Check: build-test (20.x, postgresql)
  • GitHub Check: build-test (20.x, sqlite)
🔇 Additional comments (2)
packages/clients/tanstack-query/src/utils/types.ts (2)

43-43: Excellent refactoring - reduces duplication and improves readability.

The refactored WithOptimistic type is much cleaner and delegates to the helper type for both array elements and non-array types, eliminating duplication of the $optimistic property definition.


34-41: Good refactoring with improved primitive type handling.

The new WithOptimisticFlag helper consolidates the $optimistic property logic and introduces an extends object check that gracefully handles primitive types. This is an improvement over the previous implementation where primitives would resolve to never (e.g., string & { $optimistic?: boolean } = never).

The type is used in react.ts, vue.ts, and svelte.ts where it wraps query result types with WithOptimistic<T>. In the ORM context, T represents model instances (objects), so this defensive check maintains backward compatibility while preventing edge-case type errors if primitives are ever passed.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link

claude bot commented Nov 6, 2025

Claude finished @ymc9's task —— View job


PR Review: Optimistic Typing Fix

Reviewing the typing changes in the TanStack Query package.

Todo List:

  • Read repository guidelines (CLAUDE.md)
  • Examine the modified file and understand the changes
  • Analyze the git diff to see specific modifications
  • Review TypeScript types for correctness and best practices
  • Check for potential bugs or issues
  • Assess performance and security implications
  • Provide comprehensive feedback

Review Summary

This PR refactors the WithOptimistic<T> type implementation by extracting a helper type WithOptimisticFlag<T> to improve code organization and reusability. The changes are well-structured and represent a good refactoring practice.

Code Quality & Best Practices

Excellent refactoring that improves maintainability:

  • DRY Principle: Eliminates code duplication by extracting the common optimistic flag logic into WithOptimisticFlag<T>
  • Single Responsibility: The helper type has a clear, focused purpose
  • Readability: The separated concerns make the type logic easier to understand
  • Consistency: Maintains the same external API while improving internal structure

Type Safety & Correctness

The type logic is sound and maintains backward compatibility:

  • Conditional Types: Proper use of T extends object to ensure the optimistic flag is only added to object types
  • Array Handling: Correctly preserves array structure while applying the flag to array elements
  • Generic Constraints: Appropriate use of generics maintains type safety
  • API Preservation: External consumers of WithOptimistic<T> are unaffected

Performance Considerations

No performance implications - this is a compile-time only change:

  • TypeScript types are erased at runtime, so this refactoring has zero runtime impact
  • The type complexity remains the same from the compiler's perspective
  • No changes to the generated JavaScript code

Security Considerations

No security concerns identified:

  • Pure type-level refactoring with no runtime behavior changes
  • No exposure of sensitive data or introduction of vulnerabilities
  • The $optimistic flag is used for UI state management, which is appropriate

Testing Coverage

Existing tests should continue to pass:

  • The change preserves the exact same type behavior
  • Tests in /home/runner/work/zenstack-v3/zenstack-v3/packages/clients/tanstack-query/test/react-query.test.tsx verify $optimistic flag behavior
  • No additional test coverage needed for this refactoring

🔍 Code Analysis

Before (lines 34-49):

export type WithOptimistic<T> =
    T extends Array<infer U>
        ? Array<U & { $optimistic?: boolean; }>
        : T & { $optimistic?: boolean; };

After (lines 34-43):

type WithOptimisticFlag<T> = T extends object
    ? T & { $optimistic?: boolean; }
    : T;

export type WithOptimistic<T> = T extends Array<infer U> 
    ? Array<WithOptimisticFlag<U>> 
    : WithOptimisticFlag<T>;

💡 Minor Suggestions

  1. Type Constraint Enhancement: Consider adding a more specific constraint to WithOptimisticFlag:

    type WithOptimisticFlag<T> = T extends Record<string, unknown>
        ? T & { $optimistic?: boolean; }
        : T;

    This would be more explicit about expecting object types with string keys.

  2. JSDoc Documentation: Consider adding JSDoc to the helper type:

    /**
     * Adds an optional $optimistic flag to object types
     */
    type WithOptimisticFlag<T> = // ...

Overall Assessment

This is a high-quality refactoring that:

  • ✅ Improves code organization without changing behavior
  • ✅ Reduces duplication and improves maintainability
  • ✅ Maintains full backward compatibility
  • ✅ Follows TypeScript best practices
  • ✅ Has no runtime or security implications

Recommendation: Approve - This change represents good engineering practices and should be merged.


Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the WithOptimistic type utility to improve code maintainability by extracting the common optimistic flag logic into a separate helper type. The change eliminates code duplication by introducing WithOptimisticFlag<T> that handles adding the $optimistic property to objects, which is then reused in both the array and non-array branches of WithOptimistic<T>.

  • Introduced WithOptimisticFlag<T> helper type to encapsulate the logic for adding the $optimistic property
  • Simplified WithOptimistic<T> to use the new helper type for both array and non-array cases

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ymc9 ymc9 merged commit 2946115 into dev Nov 6, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants