feat(query-core): add structuralSharing option to useQueries #10101
Conversation
🦋 Changeset detectedLatest commit: eabd1fc The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 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 |
github-actions
bot
added
documentation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new public option Changes
Sequence Diagram(s)sequenceDiagram
participant Hook as Framework Hook\n(useQueries)
participant Observer as QueriesObserver\n(query-core)
participant Combine as CombineFn\n(user-provided)
participant Result as Combined Result
Hook->>Observer: construct(queries, combine?, structuralSharing?)
Hook->>Observer: getOptimisticResult(queries, combine, structuralSharing)
Observer->>Observer: compute per-query optimistic results
alt combine provided
Observer->>Combine: call combine(per-query results)
Combine-->>Observer: combined value
Observer->>Observer: apply structuralSharing?
Observer-->>Result: combined (possibly shared) result
else no combine
Observer-->>Result: array of per-query results
end
Observer-->>Hook: optimistic result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/solid-query/src/useQueries.ts (1)
312-332:⚠️ Potential issue | 🔴 CriticalBug:
structuralSharingis not passed toobserver.setQueries()in the Solid adapter.Both
setQueriescall sites (lines 313–320 and 324–331) manually construct the options object with onlycombine, omittingstructuralSharing. This causes the observer's internal#options.structuralSharingto beundefinedafter mount/update, resulting in subscription-driven notifications ignoring the user'sstructuralSharing: falsesetting.All other framework adapters (React, Vue, Svelte, Angular) pass the full options object including
structuralSharingtosetQueries.Proposed fix
onMount(() => { observer.setQueries( defaultedQueries(), - queriesOptions().combine - ? ({ - combine: queriesOptions().combine, - } as QueriesObserverOptions<TCombinedResult>) - : undefined, + queriesOptions() as QueriesObserverOptions<TCombinedResult>, ) }) createComputed(() => { observer.setQueries( defaultedQueries(), - queriesOptions().combine - ? ({ - combine: queriesOptions().combine, - } as QueriesObserverOptions<TCombinedResult>) - : undefined, + queriesOptions() as QueriesObserverOptions<TCombinedResult>, ) })The extra
queriesproperty inqueriesOptions()will be passed tosetQueriesbut safely ignored at runtime sinceQueriesObserverOptionsonly hascombineandstructuralSharingproperties.
🤖 Fix all issues with AI agents
In @.changeset/silver-coins-mix.md:
- Line 10: The changeset message contains a typo "strucuralSharing"; update the
changes text to read "structuralSharing" so the feature line reads:
feat(query-core): Allow disabling structuralSharing for useQueries, ensuring the
corrected spelling replaces the incorrect "strucuralSharing" token in the
changeset entry.
In `@packages/query-core/src/queriesObserver.ts`:
- Around line 33-37: The JSDoc claim that structuralSharing disables structural
sharing is inaccurate because replaceEqualDeep is only called inside the combine
branch (see combine usage around replaceEqualDeep and return of input), so
structuralSharing: false is a no-op when combine is undefined; either update the
JSDoc (and corresponding adapter docs) to state structuralSharing only applies
when combine is provided, or change the non-combine path to respect the flag by
applying the same replaceEqualDeep logic to the returned input (use the existing
replaceEqualDeep helper and the structuralSharing boolean where the function
currently returns input directly) so structuralSharing:false actually prevents
structural sharing in both code paths (adjust code around the combine check and
the final return to call replaceEqualDeep when appropriate).
🧹 Nitpick comments (1)
packages/query-core/src/__tests__/queriesObserver.test.tsx (1)
296-305: Missing test coverage forstructuralSharing: false.All existing tests pass
undefined(defaulting totrue). There are no tests verifying thatstructuralSharing: falseactually skipsreplaceEqualDeepand produces a new reference for the combined result. Consider adding a test that asserts referential inequality whenstructuralSharingisfalsewith acombinefunction, and referential equality when it'strue.
dagstuan
force-pushed
the
main
branch
4 times, most recently
from
368a0b7 to
21e9f59
Compare
Add a `structuralSharing` option to useQueries/createQueries/injectQueries that allows disabling structural sharing for the combined result. When set to `false`, the combined result will not use `replaceEqualDeep` for referential stability. Defaults to `true`.
| * Only applies when `combine` is provided. | ||
| * Defaults to `true`. | ||
| */ | ||
| structuralSharing?: boolean |
There was a problem hiding this comment.
as with the standard option on useQuery, this should also accept a function.
There was a problem hiding this comment.
Done. The signature is the same as the regular useQuery-hook and I updated all the adapters.
| --- | ||
| '@tanstack/angular-query-experimental': minor | ||
| '@tanstack/svelte-query': minor | ||
| '@tanstack/react-query': minor |
There was a problem hiding this comment.
please also do the new preact adapter
| const combined = combine(input) | ||
|
|
||
| this.#combinedResult = structuralSharing | ||
| ? replaceEqualDeep(this.#combinedResult, combined) | ||
| : combined |
There was a problem hiding this comment.
please add some tests where false is passed
There was a problem hiding this comment.
Added tests for both false, true and passing a function.
Match the useQuery logic and allow a function to override the structural sharing.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/framework/react/reference/useQueries.md`:
- Around line 27-31: Update the `structuralSharing` option docs to state the
prerequisite that it "Only applies when `combine` is provided"; specifically, in
the `structuralSharing?: boolean | ((oldData: unknown | undefined, newData:
unknown) => unknown)` description add a concise note that this setting has
effect only if `combine` is supplied, referencing the `combine` option so
readers understand the dependency between `structuralSharing` and `combine`.
🧹 Nitpick comments (1)
packages/svelte-query/src/createQueries.svelte.ts (1)
215-216: IncludingstructuralSharinginderivedCreateQueriesOptionsmay trigger unnecessary observer recreation.Since
observeris$derivedfromderivedCreateQueriesOptions(line 229-235), changing onlystructuralSharingwill recreate the entireQueriesObserver. The constructor doesn't usestructuralSharing— it's only consumed later bygetOptimisticResultand#combineResult. This is the same pre-existing trade-off ascombinechanges causing recreation, so it's not a regression, but worth noting for potential future optimization.Also applies to: 229-235
2 participants
🎯 Changes
Add a
structuralSharingoption to useQueries/createQueries/injectQueries that allows disabling structural sharing for the combined result. When set tofalse, the combined result will not usereplaceEqualDeepfor referential stability. Defaults totrue.Full disclosure: Docs were generated by Claude, all other implementation was done without AI.
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Documentation
Chores