fix(query-core): replaceEqualDeep max depth#10032
Merged
TkDodo merged 4 commits intoTanStack:mainfrom Jan 13, 2026
Merged
Conversation
🦋 Changeset detectedLatest commit: 7c287aa The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
📝 WalkthroughWalkthroughAdds an optional Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (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)
Comment |
|
View your CI Pipeline Execution ↗ for commit 7c287aa
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/query-core/src/utils.ts (1)
273-274: Consider extracting the magic number to a named constant.A named constant improves readability and makes the limit self-documenting and easier to adjust if needed.
♻️ Suggested refactor
Add near the top of the file (around line 89 with other constants):
const MAX_REPLACE_EQUAL_DEPTH = 500Then update the guard:
- if (depth > 500) return b + if (depth > MAX_REPLACE_EQUAL_DEPTH) return b
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/query-core/src/utils.ts
⏰ 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
🔇 Additional comments (3)
packages/query-core/src/utils.ts (3)
267-268: LGTM on the signature change.Adding an optional parameter with a default value maintains backward compatibility. Existing callers will continue to work without modification.
308-308: LGTM on depth propagation.Correctly increments depth on each recursive call to track nesting level.
267-314: Add tests for the depth limit behavior.The
replaceEqualDeepfunction includes a depth limit (depth > 500) to prevent stack overflow on deeply nested structures, but no tests exist to verify this critical safety feature. Add tests covering:
- Behavior when depth reaches 500 vs exceeds 500
- Deeply nested structures that trigger the limit
- Structural sharing stops correctly at the boundary
More templates
@tanstack/angular-query-experimental
@tanstack/eslint-plugin-query
@tanstack/query-async-storage-persister
@tanstack/query-broadcast-client-experimental
@tanstack/query-core
@tanstack/query-devtools
@tanstack/query-persist-client-core
@tanstack/query-sync-storage-persister
@tanstack/react-query
@tanstack/react-query-devtools
@tanstack/react-query-next-experimental
@tanstack/react-query-persist-client
@tanstack/solid-query
@tanstack/solid-query-devtools
@tanstack/solid-query-persist-client
@tanstack/svelte-query
@tanstack/svelte-query-devtools
@tanstack/svelte-query-persist-client
@tanstack/vue-query
@tanstack/vue-query-devtools
commit: |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/query-core/src/__tests__/query.test.tsx:
- Around line 1061-1076: The getter in the test object `data` is declared as
`get foo(): void` which is incorrect — remove the explicit `: void` return type
(or change it to a correct type such as `string` or `any`) on the `get foo`
accessor in the `data` object so TypeScript won't flag a mismatch; keep the
getter body unchanged if you still want the recursion/serialization-failure
behavior used by `queryFn` and `queryClient.prefetchQuery`.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/query-core/src/__tests__/query.test.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-22T09:06:05.219Z
Learnt from: sukvvon
Repo: TanStack/query PR: 9892
File: packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx:331-335
Timestamp: 2025-11-22T09:06:05.219Z
Learning: In TanStack/query test files, when a queryFn contains side effects (e.g., setting flags for test verification), prefer async/await syntax for clarity; when there are no side effects, prefer the .then() pattern for conciseness.
Applied to files:
packages/query-core/src/__tests__/query.test.tsx
📚 Learning: 2025-11-02T22:52:33.071Z
Learnt from: DogPawHat
Repo: TanStack/query PR: 9835
File: packages/query-core/src/__tests__/queryClient.test-d.tsx:242-256
Timestamp: 2025-11-02T22:52:33.071Z
Learning: In the TanStack Query codebase, the new `query` and `infiniteQuery` methods support the `select` option for data transformation, while the legacy `fetchQuery` and `fetchInfiniteQuery` methods do not support `select` and should reject it at the type level.
Applied to files:
packages/query-core/src/__tests__/query.test.tsx
🧬 Code graph analysis (1)
packages/query-core/src/__tests__/query.test.tsx (1)
packages/query-core/src/utils.ts (1)
sleep(376-380)
🪛 Biome (2.1.2)
packages/query-core/src/__tests__/query.test.tsx
[error] 1067-1067: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
⏰ 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
Comment on lines
+1061
to
+1076
| const initialData = { | ||
| foo: 'bar', | ||
| } | ||
|
|
||
| const data = { | ||
| get foo(): void { | ||
| return this.foo | ||
| }, | ||
| } | ||
|
|
||
| queryFn.mockImplementation(() => sleep(10).then(() => data)) | ||
|
|
||
| queryClient.prefetchQuery({ | ||
| queryKey: key, | ||
| queryFn, | ||
| initialData: structuredClone(data), | ||
| initialData, |
Contributor
There was a problem hiding this comment.
Simplified test data looks good; fix the getter return type.
The initialData simplification is appropriate. However, the static analysis correctly flags that the getter at line 1067 has an incorrect return type—void indicates no return value, but the getter attempts to return this.foo.
Since this getter is designed to cause infinite recursion (triggering serialization failure), consider removing the type annotation or using a more accurate type.
Proposed fix
const data = {
- get foo(): void {
+ get foo(): string {
return this.foo
},
}Alternatively, remove the type annotation entirely and let TypeScript infer:
const data = {
- get foo(): void {
+ get foo() {
return this.foo
},
}📝 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.
Suggested change
| const initialData = { | |
| foo: 'bar', | |
| } | |
| const data = { | |
| get foo(): void { | |
| return this.foo | |
| }, | |
| } | |
| queryFn.mockImplementation(() => sleep(10).then(() => data)) | |
| queryClient.prefetchQuery({ | |
| queryKey: key, | |
| queryFn, | |
| initialData: structuredClone(data), | |
| initialData, | |
| const initialData = { | |
| foo: 'bar', | |
| } | |
| const data = { | |
| get foo(): string { | |
| return this.foo | |
| }, | |
| } | |
| queryFn.mockImplementation(() => sleep(10).then(() => data)) | |
| queryClient.prefetchQuery({ | |
| queryKey: key, | |
| queryFn, | |
| initialData, |
🧰 Tools
🪛 Biome (2.1.2)
[error] 1067-1067: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
🤖 Prompt for AI Agents
In @packages/query-core/src/__tests__/query.test.tsx around lines 1061 - 1076,
The getter in the test object `data` is declared as `get foo(): void` which is
incorrect — remove the explicit `: void` return type (or change it to a correct
type such as `string` or `any`) on the `get foo` accessor in the `data` object
so TypeScript won't flag a mismatch; keep the getter body unchanged if you still
want the recursion/serialization-failure behavior used by `queryFn` and
`queryClient.prefetchQuery`.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #10032 +/- ##
===========================================
+ Coverage 45.84% 59.63% +13.79%
===========================================
Files 200 129 -71
Lines 8525 5740 -2785
Branches 1968 1587 -381
===========================================
- Hits 3908 3423 -485
+ Misses 4157 2000 -2157
+ Partials 460 317 -143 🚀 New features to boost your workflow:
|
TkDodo
approved these changes
Jan 13, 2026
Fixes the max depth for replaceEqualDeep in query-core.
Merged
Contributor
|
🎉 This PR has been released! Thank you for your contribution! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Changes
Add max depth for recursive
replaceEqualDeepto avoid insanely deep objects (500 nested levels) slowing/freezing the main thread. Beyond 500, we just return the new object.The function is still callable w/ the same parameters as before. It should have now impact on users, except for those that have insanely deep objects.
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.