fix(react): allow using enabled when using useQuery().promise
#7
Conversation
|
View your CI Pipeline Execution ↗ for commit 2f69183.
☁️ Nx Cloud last updated this comment at |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8501 +/- ##
===========================================
+ Coverage 46.25% 63.04% +16.78%
===========================================
Files 199 135 -64
Lines 7530 4835 -2695
Branches 1719 1358 -361
===========================================
- Hits 3483 3048 -435
+ Misses 3668 1543 -2125
+ Partials 379 244 -135 |
WalkthroughThe changes introduce new test cases to both the core and React layers of the query package, focusing on the behavior of queries when toggling the Changes
Sequence Diagram(s)sequenceDiagram
participant Component
participant useQuery
participant QueryCache
participant QueryObserver
Component->>useQuery: Render with enabled: false
useQuery->>QueryCache: Check for cache entry
QueryCache-->>useQuery: Return cache entry (possibly undefined)
useQuery->>QueryObserver: Subscribe (no fetch)
Component->>useQuery: Update enabled: true
useQuery->>QueryCache: Inspect cache entry state
alt Should fetch (no data or pending/idle)
useQuery->>QueryCache: fetchOptimistic()
else Already fetching or data present
useQuery->>QueryCache: Subscribe to existing promise
end
QueryCache-->>useQuery: Promise/data
useQuery-->>Component: Return data or suspense
Poem
✨ Finishing Touches
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. 🪧 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 (
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![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/react-query/src/useBaseQuery.ts (1)
144-152: Improved granular query fetch control with detailed state inspection.The code now examines the cache entry's internal state properties instead of simply checking if the entry exists. This allows for more precise control over when to fetch data during render, especially when toggling the
enabledflag.As noted in the comment, this logic might be better placed in
getOptimisticResult()in the future. Consider creating a follow-up issue to track this potential refactor if issue TanStack#8507 doesn't already cover it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/query-core/src/__tests__/queryObserver.test.tsx(3 hunks)packages/react-query/src/__tests__/useQuery.promise.test.tsx(1 hunks)packages/react-query/src/useBaseQuery.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/query-core/src/__tests__/queryObserver.test.tsx (3)
packages/query-core/src/__tests__/utils.ts (1)
queryKey(24-27)packages/query-core/src/queryObserver.ts (1)
QueryObserver(43-759)packages/query-core/src/thenable.ts (1)
pendingThenable(42-82)
packages/react-query/src/useBaseQuery.ts (1)
packages/query-core/src/query.ts (1)
promise(193-195)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test
🔇 Additional comments (4)
packages/react-query/src/useBaseQuery.ts (1)
153-157: Improved conditional query fetching with the newshouldFetchcondition.The code now uses a more precise condition to determine when to fetch data or subscribe to an existing promise. This is particularly important for correctly handling queries when their
enabledstate changes.packages/react-query/src/__tests__/useQuery.promise.test.tsx (1)
1381-1447: Well-structured test forenabledstate changes with suspense.This test effectively validates that the
enabledflag works correctly with React Suspense:
- Initially the query doesn't run when disabled
- When enabled, the query shows the loading state
- The query function runs exactly once
- Data is properly rendered after fetching completes
The test is thorough and directly addresses the issue in the PR title regarding using
enabledwithuseQuery().promise.packages/query-core/src/__tests__/queryObserver.test.tsx (2)
1-12: Appropriate imports added to support new tests.The addition of
waitForfrom@testing-library/domandpendingThenablefrom the thenable module provides the necessary utilities for the new test.
1238-1270: Valuable test ensuring promise reuse when togglingenabledstate.This test verifies a critical behavior: when toggling a query from disabled to enabled, the same promise instance should be reused. This is important for React Suspense to work correctly with the promise returned by
useQuery().The test complements the React-level test by verifying the core behavior at the QueryObserver level, ensuring consistent promise handling throughout the library.
Closes TanStack#8499
Summary by CodeRabbit