feat(db): shared live-query observer + migrate all five adapters (RFC #1623 step 3)#1642
Open
kevin-dp wants to merge 4 commits into
Open
feat(db): shared live-query observer + migrate all five adapters (RFC #1623 step 3)#1642kevin-dp wants to merge 4 commits into
kevin-dp wants to merge 4 commits into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds a shared ChangesShared live-query observer migration
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
More templates
@tanstack/angular-db
@tanstack/browser-db-sqlite-persistence
@tanstack/capacitor-db-sqlite-persistence
@tanstack/cloudflare-durable-objects-db-sqlite-persistence
@tanstack/db
@tanstack/db-ivm
@tanstack/db-sqlite-persistence-core
@tanstack/electric-db-collection
@tanstack/electron-db-sqlite-persistence
@tanstack/expo-db-sqlite-persistence
@tanstack/node-db-sqlite-persistence
@tanstack/offline-transactions
@tanstack/powersync-db-collection
@tanstack/query-db-collection
@tanstack/react-db
@tanstack/react-native-db-sqlite-persistence
@tanstack/rxdb-db-collection
@tanstack/solid-db
@tanstack/svelte-db
@tanstack/tauri-db-sqlite-persistence
@tanstack/trailbase-db-collection
@tanstack/vue-db
commit: |
Contributor
|
Size Change: +1.06 kB (+0.85%) Total Size: 126 kB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
|
Size Change: 0 B Total Size: 4.22 kB ℹ️ View Unchanged
|
kevin-dp
changed the title
kevin-dp
commented
Jul 7, 2026
| knownGaps: [`config-object-input`], | ||
| // The config-object gap is now closed here: the observer starts sync on the | ||
| // resolved collection regardless of the config path, so a bare `{ query }` | ||
| // syncs. (PR #1638 also fixes the source path directly.) |
Contributor
Author
There was a problem hiding this comment.
This comment is not needed. It only makes sense with the scope of this PR but no longer makes sense when we will merge it into main.
Contributor
Author
There was a problem hiding this comment.
Removed in cf92efd — the comment was scoped to this PR and wouldn't make sense on main.
| knownGaps: [`config-object-input`], | ||
| // The config-object gap is now closed here: the observer starts sync on the | ||
| // resolved collection regardless of the config path, so a bare `{ query }` | ||
| // syncs. (PR #1638 also fixes the source path directly.) |
Contributor
Author
There was a problem hiding this comment.
let's remove this comment, it only makes sense within this PR but in main it won't make sense anymore.
Contributor
Author
There was a problem hiding this comment.
Removed in cf92efd — the comment was scoped to this PR and wouldn't make sense on main.
kevin-dp
force-pushed
the
refactor/extract-adapter-helpers
branch
from
92f19fa to
84895f6
Compare
kevin-dp
force-pushed
the
refactor/live-query-observer
branch
from
4c86c7f to
a7e3c5e
Compare
kevin-dp
force-pushed
the
refactor/extract-adapter-helpers
branch
from
84895f6 to
84f4e93
Compare
kevin-dp
force-pushed
the
refactor/live-query-observer
branch
2 times, most recently
from
3b3c92e to
cf92efd
Compare
kevin-dp
force-pushed
the
refactor/extract-adapter-helpers
branch
from
84f4e93 to
4cf25ae
Compare
kevin-dp
force-pushed
the
refactor/live-query-observer
branch
from
cf92efd to
68ea1ca
Compare
Collaborator
Code reviewFound 1 issue:
db/packages/db/src/live-query-observer.ts Lines 178 to 184 in 68ea1ca db/packages/db/src/collection/lifecycle.ts Lines 282 to 289 in 68ea1ca db/packages/db/src/collection/lifecycle.ts Lines 149 to 151 in 68ea1ca I red-tested this locally with a temporary observer test on this branch: after subscribing, unsubscribing before first ready, subscribing again, and then calling Suggested fix: either make 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
Contributor
Author
|
Good catch — reproduced it exactly (subscribe → unsubscribe-before-ready → subscribe → |
KyleAMathews
approved these changes
Jul 7, 2026
Add createLiveQueryObserver to @tanstack/db. Given a resolved collection (or null for disabled), it owns the shared lifecycle: start sync, subscribe with initial state, the loading→ready notify, a stable per-revision snapshot for wholesale consumers, and delivery of the raw ChangeMessage[] for granular consumers (deferInitialNotify defers the initial notify for useSyncExternalStore consumers like React). React, Vue, Svelte, Solid, and Angular all materialize from the observer, removing their duplicated subscribe/status/ready-race plumbing while keeping native reactivity: Vue/Svelte/Solid apply the change deltas granularly to their reactive maps; React/Angular consume the snapshot wholesale. Observer unit tests cover the wholesale and granular paths, disabled, deferred-notify, and dispose. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
onFirstReady returns no unsubscribe and detach() couldn't remove it, so a subscribe → unsubscribe-before-ready → subscribe sequence left a stale ready callback that also fired on markReady — the current listener saw two synthetic ready notifications instead of one. Guard the callback with an attach-generation token so only the current attachment's callback notifies. Adds a regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
kevin-dp
force-pushed
the
refactor/live-query-observer
branch
from
8ee8e8e to
e957121
Compare
![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: 5
🧹 Nitpick comments (3)
packages/db/src/live-query-observer.ts (1)
24-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTighten the collection generic instead of exposing
any.The observer API can use
Collection<T, TKey>here; the default utility generic keeps the API precise without leakingany.Proposed typing cleanup
- collection: Collection<T, TKey, any> | undefined + collection: Collection<T, TKey> | undefined ... - private readonly collection: Collection<T, TKey, any> | null + private readonly collection: Collection<T, TKey> | null ... - collection: Collection<T, TKey, any> | null, + collection: Collection<T, TKey> | null, ... - collection: Collection<T, TKey, any> | null | undefined, + collection: Collection<T, TKey> | null | undefined,As per coding guidelines, “Avoid using
anytypes; useunknowninstead when the type is truly unknown, and provide proper type annotations for return values.”Also applies to: 86-100, 248-253
🤖 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/db/src/live-query-observer.ts` at line 24, The observer typings are leaking `any` through the collection generic, so tighten the `LiveQueryObserver` API to use `Collection<T, TKey>` (or the existing default utility generic) instead of `Collection<T, TKey, any>`. Update the `collection` field and the related observer methods/types in `live-query-observer` that reference this shape, including the other affected spots noted in the diff, so the generic stays precise without exposing `any`.Source: Coding guidelines
packages/db/tests/live-query-observer.test.ts (1)
45-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the
as anycasts from observer tests.These casts hide whether the new public factory accepts the collection types it is expected to support.
As per coding guidelines, “Avoid using
anytypes; useunknowninstead when the type is truly unknown.”Also applies to: 60-60, 92-92, 121-123, 138-138
🤖 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/db/tests/live-query-observer.test.ts` at line 45, The observer tests are masking the real type compatibility of createLiveQueryObserver by casting sources to any; update the affected test cases to pass properly typed collection/source values instead of any so the public factory is exercised against its supported collection types. Use the existing test helpers and types around createLiveQueryObserver, makeSource, and the other affected observer assertions to infer the correct typings, and replace any remaining any casts in the referenced test blocks with explicit, appropriate types.Source: Coding guidelines
packages/react-db/src/useLiveQuery.ts (1)
435-439: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid erasing the hook return type with
any.The new
as anybypasses the overload contracts at the return boundary. Please cast to the hook’s concrete result type or add a typed adapter fromLiveQuerySnapshotto the existing public return shape. As per coding guidelines, “Avoid usinganytypes” and “Always provide the most precise return type annotation.”🤖 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/react-db/src/useLiveQuery.ts` around lines 435 - 439, The return in useLiveQuery is erasing the hook’s type contract with as any, which bypasses the overload guarantees. Update the useSyncExternalStore result to preserve the concrete hook return type by casting to the existing public snapshot/result type or introducing a typed adapter from LiveQuerySnapshot, and keep the return aligned with the function’s declared overloads instead of using any.Source: Coding guidelines
🤖 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/db/src/live-query-observer.ts`:
- Around line 112-115: The snapshot cache in `LiveQueryObserver.getSnapshot()`
can stay stale when `preload()` or a status-only transition makes the collection
ready without bumping `version`. Update the cache invalidation logic so
`preload()` and any readiness/status changes also force a cached snapshot
refresh, not just version changes. Use the existing `cachedVersion`, `version`,
and `getSnapshot()` flow, and make sure the invalidation also covers the related
status transition path referenced in the observer.
- Around line 199-204: The deferred initial notification flush in
LiveQueryObserver can still emit stale changes after a
subscribe/unsubscribe/resubscribe cycle. Update the deferInitialNotify path in
live-query-observer’s attach logic to associate the queued microtask with the
specific attach instance (for example via an incrementing attach token or
generation) and skip flushing if a newer attach has superseded it or the
observer was detached. Ensure the guard is checked before emitting deferred
changes in the queueMicrotask callback.
In `@packages/db/tests/live-query-observer.test.ts`:
- Around line 120-155: Add regression coverage for the remaining lifecycle races
in createLiveQueryObserver: extend the existing observer tests to reproduce a
deferInitialNotify flow where a subscription is unsubscribed and then
resubscribed before the microtask flush, and verify only the latest subscription
receives the deferred notify. Also add a test around getSnapshot() and preload()
that calls getSnapshot() before preload(), then asserts a ready snapshot is
returned after preload() completes. Use the existing createLiveQueryObserver,
subscribe, getSnapshot, and preload behavior to keep the tests aligned with the
bug scenarios.
In `@packages/react-db/src/useLiveQuery.ts`:
- Around line 414-423: Move the observer teardown out of the render path in
useLiveQuery: the needsNewCollection branch should not call
observerRef.current?.dispose() during render, because that can tear down the
committed subscription before React finishes updating. Update useLiveQuery to
perform observer disposal/recreation in a commit-phase effect or cleanup, and
add separate state/flag tracking for initialization so disabled queries do not
recreate the observer on every render when collectionRef.current remains null.
In `@packages/solid-db/src/useLiveQuery.ts`:
- Around line 393-397: Clear the existing Solid state before wiring up the new
observer in useLiveQuery so stale rows from the previous collection are removed
when collection() changes. Add the reset immediately before
createLiveQueryObserver/currentCollection subscription setup, and keep the logic
localized around observer.subscribe so includeInitialState only repopulates the
new collection instead of leaving old keys behind.
---
Nitpick comments:
In `@packages/db/src/live-query-observer.ts`:
- Line 24: The observer typings are leaking `any` through the collection
generic, so tighten the `LiveQueryObserver` API to use `Collection<T, TKey>` (or
the existing default utility generic) instead of `Collection<T, TKey, any>`.
Update the `collection` field and the related observer methods/types in
`live-query-observer` that reference this shape, including the other affected
spots noted in the diff, so the generic stays precise without exposing `any`.
In `@packages/db/tests/live-query-observer.test.ts`:
- Line 45: The observer tests are masking the real type compatibility of
createLiveQueryObserver by casting sources to any; update the affected test
cases to pass properly typed collection/source values instead of any so the
public factory is exercised against its supported collection types. Use the
existing test helpers and types around createLiveQueryObserver, makeSource, and
the other affected observer assertions to infer the correct typings, and replace
any remaining any casts in the referenced test blocks with explicit, appropriate
types.
In `@packages/react-db/src/useLiveQuery.ts`:
- Around line 435-439: The return in useLiveQuery is erasing the hook’s type
contract with as any, which bypasses the overload guarantees. Update the
useSyncExternalStore result to preserve the concrete hook return type by casting
to the existing public snapshot/result type or introducing a typed adapter from
LiveQuerySnapshot, and keep the return aligned with the function’s declared
overloads instead of using any.
🪄 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: a2f170d5-edf2-466d-b219-bdc647b51052
📒 Files selected for processing (10)
.changeset/live-query-observer.mdpackages/angular-db/src/index.tspackages/angular-db/tests/conformance.test.tspackages/db/src/index.tspackages/db/src/live-query-observer.tspackages/db/tests/live-query-observer.test.tspackages/react-db/src/useLiveQuery.tspackages/solid-db/src/useLiveQuery.tspackages/svelte-db/src/useLiveQuery.svelte.tspackages/vue-db/src/useLiveQuery.ts
Comment on lines
+393
to
+397
|
|
||
| // The shared observer owns subscription, the ready-race, and status; Solid | ||
| // materializes into its keyed ReactiveMap (granular) + reconciled store. | ||
| const observer = createLiveQueryObserver(currentCollection) | ||
| const unsubscribe = observer.subscribe( |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Clear state before subscribing to a replacement collection.
includeInitialState only inserts current rows; it does not delete keys from the previous collection. Without a clear here, state can expose stale rows after collection() changes.
Proposed fix
// The shared observer owns subscription, the ready-race, and status; Solid
// materializes into its keyed ReactiveMap (granular) + reconciled store.
const observer = createLiveQueryObserver(currentCollection)
+ state.clear()
const unsubscribe = observer.subscribe(📝 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
| // The shared observer owns subscription, the ready-race, and status; Solid | |
| // materializes into its keyed ReactiveMap (granular) + reconciled store. | |
| const observer = createLiveQueryObserver(currentCollection) | |
| const unsubscribe = observer.subscribe( | |
| // The shared observer owns subscription, the ready-race, and status; Solid | |
| // materializes into its keyed ReactiveMap (granular) + reconciled store. | |
| const observer = createLiveQueryObserver(currentCollection) | |
| state.clear() | |
| const unsubscribe = observer.subscribe( |
🤖 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/solid-db/src/useLiveQuery.ts` around lines 393 - 397, Clear the
existing Solid state before wiring up the new observer in useLiveQuery so stale
rows from the previous collection are removed when collection() changes. Add the
reset immediately before createLiveQueryObserver/currentCollection subscription
setup, and keep the logic localized around observer.subscribe so
includeInitialState only repopulates the new collection instead of leaving old
keys behind.
- observer: getSnapshot() rebuilds when collection.status changes without a version bump (status-only loading→ready / preload with no active subscription), so a cached snapshot can't go stale. - observer: guard the deferred initial-notify microtask with the attach generation + listener count, so a superseded attach can't flush a stale initial batch to a later listener. - react: don't dispose the previous observer during render (unsafe under concurrent rendering) — useSyncExternalStore detaches it when the subscribe changes; dispose the current observer in an unmount effect instead. - tests: regressions for the deferred-notify race and the status-only snapshot refresh (both verified red before the fixes). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
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/react-db/src/useLiveQuery.ts`:
- Around line 428-434: The unmount cleanup in useLiveQuery’s observer lifecycle
is disposing the shared observer too early, which can leave observerRef.current
pointing to a disposed instance during StrictMode/offscreen replay. Update the
useEffect cleanup to avoid disposing the observer there, or ensure the observer
is recreated before reuse when attach() is called on a disposed instance. Keep
the fix localized to useLiveQuery and the observerRef/useSyncExternalStore
subscription path so the next subscribe always gets a live observer.
🪄 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: a7b1d806-2a37-4fb2-b0ff-7b3a757303ed
📒 Files selected for processing (3)
packages/db/src/live-query-observer.tspackages/db/tests/live-query-observer.test.tspackages/react-db/src/useLiveQuery.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/db/src/live-query-observer.ts
…tMode) The unmount-effect dispose could run during StrictMode/offscreen effect replay (mount → cleanup → mount) without a re-render, leaving observerRef pointing at a disposed observer; the next subscribe hit attach()'s disposed guard and the store stopped resubscribing. Remove the explicit dispose — useSyncExternalStore already detaches the observer on unsubscribe/unmount, so the collection subscription is torn down and the observer is GC'd. Adds a StrictMode regression test (verified red before the fix). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
RFC #1623 step 3 — the shared observer that unifies the live-query lifecycle, now adopted by all five adapters. This is where the duplicated lifecycle across adapters actually collapses.
Base: stacked on
refactor/extract-adapter-helpers(#1641, step 2) → conformance suite (#1636). Re-target down the stack as each lands.The observer
createLiveQueryObserver(collection | null, { deferInitialNotify? })in@tanstack/db. Given a resolved live-query collection (ornullfor disabled), it owns everything the five adapters used to each re-implement:includeInitialState, so consumers get initial rows + deltas through one aligned channel)onFirstReady, de-duplicated)getSnapshot()— reference-stable foruseSyncExternalStore)ChangeMessage[]to subscribers for granular consumersThe last two are the item-6 decision made concrete: the observer carries both a snapshot and the change set, so Vue/Svelte/Solid keep fine-grained keyed-map updates while React/Angular consume the snapshot.
Input resolution stays in each adapter (query fn / config / collection / disabled) — it's framework-reactive and fixed separately (#1637/#1638). The observer owns everything after the input resolves.
All five adapters migrated
Each keeps its native reactivity; the duplicated subscribe/
onFirstReady/status/ready-race plumbing is gone:useSyncExternalStore(observer.subscribe, observer.getSnapshot)— opts intodeferInitialNotifyChangeMessage[]deltas to itsreactivemapSvelteMap(runes)ReactiveMap; keepscreateResource/Suspense +reconcileTwo contract refinements the migrations forced (both interesting)
includeInitialStateis required, not optional. Seeding a granular adapter's map fromgetSnapshot()and subscribing without initial state desyncs the collection's per-subscriber change stream — deletes arrived as empty batches. The observer must subscribe with initial state so initial rows + deltas flow through one channel.deferInitialNotifyis per-consumer. React'suseSyncExternalStoremust not get a synchronous notify duringsubscribe, so React opts into deferring the initial notify to a microtask. Effect/watcher adapters (Svelte reads synchronously afterflushSync) want it synchronous, which is the default.Bonus
Angular's
config-object-inputconformance gap closes here as a side effect — the observer starts sync on the resolved collection regardless of the config path. (PR #1638 still fixes the source path directly.)Verification
All green:
@tanstack/db2461, react 119, vue 54, svelte 54, solid 62, angular 50 (+1 todo) — including the cross-adapter conformance suite that guards every migration. Observer unit tests cover both wholesale and granular paths, disabled, deferred-notify, and dispose. Minor changeset for db, patch for all five adapters.🤖 Generated with Claude Code
Summary by CodeRabbit
createLiveQueryObserver) exposed from the main DB package.