perf: use stable-hash for hashing keys - #11073
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughHydration now recomputes query hashes with merged defaults and custom hash functions. Query-key hashing uses ChangesQuery-key hashing and hydration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant DehydratedState
participant hydrate
participant QueryCache
DehydratedState->>hydrate: provide queryKey and serialized queryHash
hydrate->>hydrate: merge query defaults
hydrate->>QueryCache: create query with recomputed hash
QueryCache-->>hydrate: store hydrated query
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Using `JSON.stringify` is much slower when we only need it to create a stable value hash. Instead, we can use `stable-hash`. This is added as a devDependency so it ends up in the bundle rather than being a production dependency. On my machine, some bench results: | Task name | Latency avg (ns) | Latency med (ns) | Throughput avg (ops/s) | Throughput med (ops/s) | Samples | | -- | -- | -- | -- | -- | -- | | 'hashKey dev' | '29.05 ± 0.10%' | '41.00 ± 1.00' | '27211238 ± 0.01%' | '24390244 ± 580720' | 17211565 | | 'hashKey prod' | '942.42 ± 3.32%' | '916.00 ± 1.00' | '1096146 ± 0.01%' | '1091703 ± 1191' | 530549 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/query-core/package.json`:
- Around line 62-63: Move the stable-hash package declaration from
devDependencies to dependencies in package.json, preserving its existing version
range so the runtime import in src/utils.ts is available to consumers of the
react-native and `@tanstack/custom-condition` entry points.
In `@packages/query-core/src/utils.ts`:
- Around line 237-249: Update describeKey to detect query or mutation keys
containing values that JSON.stringify serializes lossily, including undefined,
functions, symbols, NaN, Infinity, and -Infinity, and use hashKey for those keys
instead. Preserve JSON.stringify for fully representable keys and the existing
undefined top-level behavior.
- Around line 233-235: Update hashKey and the persistence hydration flow to
version or migrate dehydrated payloads whenever the hash algorithm changes,
ensuring payloads containing the pre-change queryHash are invalidated or
converted before hydrate restores them. Add a regression test that hydrates a
pre-change dehydrated payload and verifies it cannot create a duplicate query
under the new stable-hash value.
🪄 Autofix
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 Plus
Run ID: e7cae2cc-7a8c-4079-993a-2e1718b5b58b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
packages/query-core/package.jsonpackages/query-core/src/__tests__/hydration.test.tsxpackages/query-core/src/__tests__/infiniteQueryBehavior.test.tsxpackages/query-core/src/__tests__/query.test.tsxpackages/query-core/src/__tests__/utils.test.tsxpackages/query-core/src/hydration.tspackages/query-core/src/query.tspackages/query-core/src/utils.ts
| /** | ||
| * Renders a key as a human readable string for logging and error messages. | ||
| * Keys are not guaranteed to be JSON serializable, so falls back to the hash. | ||
| */ | ||
| export function describeKey(key: QueryKey | MutationKey | undefined): string { | ||
| if (key === undefined) { | ||
| return 'undefined' | ||
| } | ||
| try { | ||
| return JSON.stringify(key) | ||
| } catch { | ||
| return hashKey(key) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- relevant files ---'
git ls-files 'packages/query-core/src/utils.ts' 'packages/query-core/**' | head -80
printf '%s\n' '--- utils outline ---'
ast-grep outline packages/query-core/src/utils.ts
printf '%s\n' '--- utils relevant source ---'
sed -n '1,320p' packages/query-core/src/utils.ts
printf '%s\n' '--- hash-related definitions/usages ---'
rg -n -C 3 'stableHash|hashKey|describeKey' packages/query-core packages | head -240
printf '%s\n' '--- tests mentioning key serialization ---'
rg -n -C 3 'describeKey|hashKey|stableHash|JSON.stringify' packages/query-core --glob '*test*' --glob '*spec*' | head -240
printf '%s\n' '--- standalone JavaScript behavior probe ---'
node - <<'JS'
const values = [
undefined,
function f() {},
Symbol('x'),
null,
NaN,
Infinity,
-Infinity,
];
const keyStrings = values.map((value) => JSON.stringify(['todos', value]));
for (let i = 0; i < keyStrings.length; i++) {
for (let j = i + 1; j < keyStrings.length; j++) {
if (keyStrings[i] === keyStrings[j]) {
console.log('collision:', i, j, keyStrings[i]);
}
}
}
for (const value of values) {
console.log(typeof value, String(value), '=>', JSON.stringify(['todos', value]));
}
JSRepository: TanStack/query
Length of output: 48995
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- relevant files ---'
git ls-files 'packages/query-core/src/utils.ts' 'packages/query-core/**' | head -80
printf '%s\n' '--- utils outline ---'
ast-grep outline packages/query-core/src/utils.ts
printf '%s\n' '--- utils relevant source ---'
sed -n '1,320p' packages/query-core/src/utils.ts
printf '%s\n' '--- hash-related definitions/usages ---'
rg -n -C 3 'stableHash|hashKey|describeKey' packages/query-core packages | head -240
printf '%s\n' '--- tests mentioning key serialization ---'
rg -n -C 3 'describeKey|hashKey|stableHash|JSON.stringify' packages/query-core --glob '*test*' --glob '*spec*' | head -240
printf '%s\n' '--- standalone JavaScript behavior probe ---'
node - <<'JS'
const values = [
undefined,
function f() {},
Symbol('x'),
null,
NaN,
Infinity,
-Infinity,
];
const keyStrings = values.map((value) => JSON.stringify(['todos', value]));
for (let i = 0; i < keyStrings.length; i++) {
for (let j = i + 1; j < keyStrings.length; j++) {
if (keyStrings[i] === keyStrings[j]) {
console.log('collision:', i, j, keyStrings[i]);
}
}
}
for (const value of values) {
console.log(typeof value, String(value), '=>', JSON.stringify(['todos', value]));
}
JSRepository: TanStack/query
Length of output: 48187
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- stable-hash dependency metadata ---'
rg -n -C 3 '"stable-hash"|stable-hash@' package.json pnpm-lock.yaml yarn.lock package-lock.json packages 2>/dev/null | head -160
printf '%s\n' '--- query and mutation key types ---'
rg -n -C 5 'export type (QueryKey|MutationKey)|QueryKey =|MutationKey =' packages/query-core/src/types.ts
printf '%s\n' '--- local dependency availability ---'
find . -path '*/node_modules/stable-hash/*' -maxdepth 6 -type f -print 2>/dev/null | head -20
printf '%s\n' '--- stable-hash source from npm registry metadata ---'
curl -L --fail --silent --show-error https://registry.npmjs.org/stable-hash/latest | python3 -c 'import json,sys; d=json.load(sys.stdin); print("version:", d.get("version")); print("tarball:", d.get("dist",{}).get("tarball"))'Repository: TanStack/query
Length of output: 7818
🏁 Script executed:
#!/bin/bash
set -e
url='https://registry.npmjs.org/stable-hash/-/stable-hash-0.0.6.tgz'
printf '%s\n' '--- stable-hash package files ---'
curl -L --fail --silent --show-error "$url" | tar -tzf -
printf '%s\n' '--- stable-hash implementation ---'
curl -L --fail --silent --show-error "$url" | tar -xOzf - package/dist/index.js 2>/dev/null || \
curl -L --fail --silent --show-error "$url" | tar -xOzf - package/index.js 2>/dev/nullRepository: TanStack/query
Length of output: 985
🏁 Script executed:
#!/bin/bash
set -e
node - <<'JS'
const weakMap = new WeakMap()
let counter = 0
function stableHash(value) {
const type = typeof value
const constructor = value && value.constructor
const isDate = constructor === Date
if (Object(value) === value && !isDate && constructor !== RegExp) {
let result = weakMap.get(value)
if (result) return result
result = `${++counter}~`
weakMap.set(value, result)
if (constructor === Array) {
result = '@'
for (let index = 0; index < value.length; index++) {
result += stableHash(value[index]) + ','
}
weakMap.set(value, result)
} else if (constructor === Object) {
result = '#'
const keys = Object.keys(value).sort()
for (let index = keys.length - 1; index >= 0; index--) {
const key = keys[index]
if (value[key] !== undefined) {
result += `${key}:${stableHash(value[key])},`
}
}
weakMap.set(value, result)
}
return result
}
if (isDate) return value.toJSON()
if (type === 'symbol') return value.toString()
if (type === 'string') return JSON.stringify(value)
return '' + value
}
const values = [
undefined,
() => {},
Symbol('x'),
null,
NaN,
Infinity,
-Infinity,
]
const keys = values.map((value) => ['todos', value])
const json = keys.map((key) => JSON.stringify(key))
const hashes = keys.map((key) => stableHash(key))
console.log('JSON:', json)
console.log('stable-hash:', hashes)
console.log('JSON collision count:', json.length - new Set(json).size)
console.log('stable-hash collision count:', hashes.length - new Set(hashes).size)
JSRepository: TanStack/query
Length of output: 528
Preserve distinct query-key values in diagnostics
JSON.stringify converts undefined, functions, symbols, NaN, Infinity, and -Infinity in arrays to null. Therefore, distinct keys can produce the same diagnostic string while hashKey distinguishes them. Reject all lossy values before serialization, or use hashKey for those keys.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/query-core/src/utils.ts` around lines 237 - 249, Update describeKey
to detect query or mutation keys containing values that JSON.stringify
serializes lossily, including undefined, functions, symbols, NaN, Infinity, and
-Infinity, and use hashKey for those keys instead. Preserve JSON.stringify for
fully representable keys and the existing undefined top-level behavior.
Source: MCP tools
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/query-core/src/hydration.ts`:
- Around line 219-224: Resolve the query-specific defaults for queryKey,
including client defaultOptions.queries and matching setQueryDefaults entries,
before computing queryHash in the hydration callback. Pass those resolved
defaults to hashQueryKeyByOptions so hydration uses the same queryKeyHashFn as
QueryClient.getQueryData; add a regression test covering queryKeyHashFn
configured through new QueryClient defaultOptions.
🪄 Autofix
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 Plus
Run ID: 6ffd0f0f-ebb9-46b0-ade0-f771de85777c
📒 Files selected for processing (2)
packages/query-core/src/__tests__/hydration.test.tsxpackages/query-core/src/hydration.ts
Using
JSON.stringifyis much slower when we only need it to create a stable value hash. Instead, we can usestable-hash.This is added as a devDependency so it ends up in the bundle rather than being a production dependency.
On my machine, some bench results:
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
Bug Fixes
Tests