fix(core): do not reject promise right away - only when promise gets used
#9576
Conversation
WalkthroughThe change defers rejection of the observer's initial thenable until the tracked result's Changes
Sequence Diagram(s)sequenceDiagram
participant Consumer
participant QueryObserver
participant TrackResultProxy
participant FeatureFlag
Consumer->>QueryObserver: getResult()
QueryObserver->>TrackResultProxy: return proxied result
Consumer->>TrackResultProxy: access "promise"
TrackResultProxy->>FeatureFlag: check experimental_prefetchInRender
alt flag disabled
TrackResultProxy->>QueryObserver: call pendingThenable.reject(Error)
TrackResultProxy-->>Consumer: rejected thenable / thrown error
else flag enabled
TrackResultProxy-->>Consumer: return actual promise
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (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). (2)
🔇 Additional comments (2)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
View your CI Pipeline Execution ↗ for commit 4a51a54
☁️ Nx Cloud last updated this comment at |
|
Sizes for commit 4a51a54:
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9576 +/- ##
===========================================
+ Coverage 45.16% 59.23% +14.06%
===========================================
Files 208 137 -71
Lines 8320 5561 -2759
Branches 1873 1499 -374
===========================================
- Hits 3758 3294 -464
+ Misses 4115 1963 -2152
+ Partials 447 304 -143 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/query-core/src/queryObserver.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/query-core/src/queryObserver.ts (1)
packages/query-core/src/thenable.ts (1)
PendingThenable(37-37)
⏰ 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). (2)
- GitHub Check: Preview
- GitHub Check: Test
| console.log('tracking prop', key) | ||
| if (key === 'promise' && !this.options.experimental_prefetchInRender) { | ||
| // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition | ||
| ;(this.#currentThenable as PendingThenable<TData>).reject?.( | ||
| new Error( | ||
| 'experimental_prefetchInRender feature flag is not enabled', | ||
| ), | ||
| ) | ||
| } |
There was a problem hiding this comment.
Remove debug log; guard rejection to pending thenables; type-safe key check
- console.log in a hot Proxy.get path will spam user consoles and degrade performance. Remove before merging.
- Only reject if the thenable is still pending to avoid repeated rejections on subsequent accesses.
- Be explicit about key type to avoid symbol keys triggering unintended behavior.
Apply:
- console.log('tracking prop', key)
- if (key === 'promise' && !this.options.experimental_prefetchInRender) {
- // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
- ;(this.#currentThenable as PendingThenable<TData>).reject?.(
- new Error(
- 'experimental_prefetchInRender feature flag is not enabled',
- ),
- )
- }
+ if (
+ typeof key === 'string' &&
+ key === 'promise' &&
+ !this.options.experimental_prefetchInRender
+ ) {
+ const thenable = this.#currentThenable as PendingThenable<TData>
+ if (thenable.status === 'pending') {
+ // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
+ thenable.reject?.(
+ new Error(
+ 'experimental_prefetchInRender feature flag is not enabled',
+ ),
+ )
+ }
+ }If helpful, I can add unit tests to assert:
- No rejection occurs when result.promise is not accessed.
- Rejection occurs exactly once when accessed with the flag disabled.
- No console output is produced during normal operation.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log('tracking prop', key) | |
| if (key === 'promise' && !this.options.experimental_prefetchInRender) { | |
| // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition | |
| ;(this.#currentThenable as PendingThenable<TData>).reject?.( | |
| new Error( | |
| 'experimental_prefetchInRender feature flag is not enabled', | |
| ), | |
| ) | |
| } | |
| if ( | |
| typeof key === 'string' && | |
| key === 'promise' && | |
| !this.options.experimental_prefetchInRender | |
| ) { | |
| const thenable = this.#currentThenable as PendingThenable<TData> | |
| if (thenable.status === 'pending') { | |
| // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition | |
| thenable.reject?.( | |
| new Error( | |
| 'experimental_prefetchInRender feature flag is not enabled', | |
| ), | |
| ) | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/query-core/src/queryObserver.ts around lines 270 to 278, remove the
console.log('tracking prop') and replace the current key check with a type-safe
guard (e.g. ensure typeof key === 'string' && key === 'promise') so symbol or
non-string keys are ignored; before calling reject, verify the thenable is still
pending (guard against settled thenables) so rejection happens at most once and
only for PendingThenable instances; use optional chaining/type narrowing to call
reject only when the thenable is the pending variant.
TkDodo
deleted the
feature/8209-experimental_prefetchInRender-promise-rejection
branch
1 participant
fixes #8209
Summary by CodeRabbit
New Features
Bug Fixes
Refactor