Conversation
📝 WalkthroughWalkthroughAdds a suspense-aware guard in QueriesObserver to skip recomputing Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit 5cb48ea
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview2 package(s) bumped directly, 22 bumped as dependents. 🟩 Patch bumps
|
size-limit report 📦
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/query-core/src/queriesObserver.ts`:
- Around line 252-261: The guard in `#shouldSkipCombine` reads suspense from
cached this.#observerMatches[].defaultedQueryOptions which can be stale; change
it to read the current suspense flag from the live options or recompute the
match for the specific index. Specifically, update `#shouldSkipCombine` to use
this.#options (the current options array) for the suspense check (e.g.
this.#options?.queries[index]?.suspense or the equivalent shape your API uses)
or call the routine that produces an up-to-date defaultedQueryOptions for that
index instead of referencing this.#observerMatches.defaultedQueryOptions, while
keeping the existing this.#result[index]?.data check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6da215a7-ff54-4f3f-be87-4883532d7c8a
📒 Files selected for processing (4)
.changeset/wild-rabbits-jump.mdpackages/query-core/src/__tests__/queriesObserver.test.tsxpackages/query-core/src/queriesObserver.tspackages/react-query/src/__tests__/useSuspenseQueries.test.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/query-core/src/__tests__/queriesObserver.test.tsx`:
- Around line 503-504: The test calls queryClient.resetQueries({ queryKey: key
}) and immediately asserts combine was called, which can race with
queued/batched notify delivery; fix by flushing pending notifications after
resetQueries before asserting: either wrap the reset in React's act (e.g.,
act(() => { queryClient.resetQueries({ queryKey: key }) })) or await a
microtask/macrotask drain (e.g., await Promise.resolve() or await new
Promise(setImmediate)) right after calling queryClient.resetQueries and before
expect(combine).toHaveBeenCalledTimes(1), and apply the same change for the
second occurrence around line 546/547.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9826816e-9182-4144-82dd-b9003c51afad
📒 Files selected for processing (2)
packages/query-core/src/__tests__/queriesObserver.test.tsxpackages/query-core/src/queriesObserver.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/query-core/src/queriesObserver.ts
fixes #10129
Summary by CodeRabbit
Bug Fixes
Tests
Chores