feat(db): shared live-query window controller (RFC #1623 phase 5) - #1675
Conversation
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>
- 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>
…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>
Expose the keyed `state` map in the shared conformance harness (added to ConformanceResult and read by all five adapter drivers) and add a steady-state `recompile-drops-stale-keys` scenario asserting the map stays in sync with `data` across a narrowing recompile. Also add a solid-db regression (in useLiveQuery.test.tsx) that inspects `state` synchronously in the window after a recompile, where solid-db leaks the previous collection's keys until its async resource reconciles. This test fails until the follow-up fix (state.clear() before re-subscribing). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
When the query recompiles to a different collection, the observer re-seeds via `includeInitialState`, which only inserts current rows and never deletes keys from the previous collection. Without clearing first, the dropped keys lingered in `state` until the async resource reconciled — a transient window where `state` exposed stale rows (though `data`, rebuilt wholesale, stayed correct). Clear synchronously before re-subscribing, matching vue-db and svelte-db. Fixes the solid-db stale-keys regression added in the previous commit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…1623 phase 4) An `orderBy` live query that reorders its rows without changing any projected row value (an "order-only move") was swallowed by the collection's value-diff: `.values()`/`.entries()` re-sorted, but no change event fired, so subscribers kept the stale order. This is the last universal expected-fail in the cross-adapter conformance suite (issue #1601). Phase 4 of the live-query platform RFC calls for an explicit layout-revision contract rather than a forged row `update`. This does that: - The live-query flush captures the retracted side of each change and, after commit, detects an order-only move (value deep-equal, `orderByIndex` moved) and publishes a first-class empty layout-change notification via a new `CollectionChangesManager.emitLayoutChangeEvent()`. - The shared observer snapshot gains `layoutRevision`, which increments on any visible membership, ordering, or order-only-move change. All five adapters pick this up through their existing wholesale re-read, so the `order-only-move` conformance scenario is removed from UNIVERSAL_EXPECTED_FAIL and now passes on React, Vue, Svelte, Solid, and Angular. Distinct from PR #1601 (v-anton), which fixes the same bug via a forced row `update`; this uses the RFC's layout-revision approach instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses independent review of the layoutRevision contract: - The join-with-separator signature could collide: a key value equal to the concatenation of neighboring keys around the separator produces the same string as two separate keys, so a real layout change (a membership change whose combined key spans the separator) was missed. Compare the ordered key sequence directly instead - collision-free, and it avoids materializing a large string on every snapshot rebuild (a new key array is only allocated when the layout actually moved). Adds a regression test. - Correct the layoutRevision doc comment: it is NOT in lockstep with snapshot identity (a value-only update yields a new snapshot but the same layoutRevision). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two gaps in the order-only-move handling, reproduced as failing tests (to be fixed in a follow-up commit): 1. A commit containing both an ordinary value update and an order-only move publishes twice (commit's row batch + the separate empty layout event), where exactly one publication is expected. 2. Ordered child collections produced by `includes` don't consume the insertion-side order metadata or publish a layout-only move, so an ordered child stays in its old order after a child order-only move. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…dren
Addresses Kyle's review of the order-only-move handling:
1. A commit containing both an ordinary value update and an order-only move
published twice: commit() emitted the row batch and then the separate layout
event fired redundantly. Replace hasOrderOnlyMove with
needsLayoutOnlyPublication, which fires the layout event only when the commit
published nothing else (any real insert/delete/value-changed update already
notifies subscribers, who re-read the re-sorted collection).
2. Ordered child collections produced by includes did not reorder on an
order-only child move:
- The child accumulate replaced value on the insert side but left the
retracted orderByIndex, so the child collection re-sorted against a stale
index. Update orderByIndex on insert and capture the retract side (both the
single-level and nested-includes accumulate blocks).
- The child flush committed without a layout-only publication when the
projected child value was unchanged. Publish one through the same
mechanism (emitLayoutChange) when the child commit published nothing else.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The includes flush is recursive, so the order-only-move handling must hold beyond one level. Adds a two-level ordered-includes regression (org -> teams -> members): moving a grandchild whose projected value is unchanged must re-sort its collection and publish exactly once. Verified red when the child-flush layout publication is removed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extracts the forward-pagination state machine out of react-db's useLiveInfiniteQuery into a framework-agnostic createLiveQueryWindowController in @tanstack/db, composing the shared live-query observer. The controller owns loadedPageCount, the peek-ahead window (via collection.utils.setWindow), page slicing, and hasNextPage/isFetchingNextPage, and exposes a reactivity-free getSnapshot/subscribe/fetchNextPage/reset/dispose surface mirroring the observer. react-db's useLiveInfiniteQuery is now a thin binding over it with no public API change; its existing suite stays green. Vue/Svelte/Solid/Angular can build infinite queries on the same controller instead of re-porting React's logic. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
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:
📝 WalkthroughWalkthroughThe change adds a shared live-query window controller for pagination, leases, snapshots, retries, and lifecycle handling. ChangesLive query pagination
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant useLiveInfiniteQuery
participant LiveQueryWindowController
participant LiveQueryCollection
participant React
useLiveInfiniteQuery->>LiveQueryWindowController: configure pagination
LiveQueryWindowController->>LiveQueryCollection: apply leased page window
LiveQueryCollection-->>LiveQueryWindowController: publish rows and observer state
LiveQueryWindowController-->>React: notify snapshot subscription
React-->>useLiveInfiniteQuery: render paginated data and status
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
UseLiveInfiniteQueryReturn references ReturnType<typeof useLiveQuery>, but the import was dropped in the controller rewrite. vitest's typecheck missed it; the package build (strict tsc) caught it (TS2304). Re-add as a type-only import. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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: |
|
Size Change: 0 B Total Size: 132 kB ℹ️ View Unchanged
|
|
Size Change: 0 B Total Size: 3.79 kB ℹ️ View Unchanged
|
…match warn Addresses review of the window-controller extraction: - pageSize/initialPageParam are now part of the controller-recreation check, so changing them at runtime re-windows and re-slices (the old hook had them in its effect/memo deps; the first controller draft baked them in at creation). - Restore the one-time console.warn when a pre-created collection's existing window differs from the first page the hook enforces (dropped in the rewrite). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A listener that synchronously mutates the collection used to trigger a nested, reentrant dispatch: later subscribers could observe the nested event (e.g. a delete) before the outer one (the insert) it reacted to. Publications are now queued and dispatched FIFO. Each publication is delivered over a snapshot of subscription records taken when it is dispatched: a subscription removed mid-delivery still receives the in-flight publication, one added mid-delivery does not. Records — not raw callbacks — identify subscriptions, so subscribing the same function twice no longer collapses into one Set entry whose first unsubscribe tore down both. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…l replay subscribeChanges delivers the initial state synchronously, so a listener could dispose the observer before the subscription handle was stored — detach() then had nothing to release and the collection subscription leaked past disposal. The release hook is now registered before the subscription is created, making attachment transactional: if detach() fired mid-replay, the subscription is undone as soon as subscribeChanges returns. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The initial-state replay only happened on the first attach, so a second concurrent subscriber started with no rows and could never converge — its keyed map silently stayed empty. A subscriber arriving while the observer is already attached is now seeded with the collection's current rows as inserts, delivered to that subscription alone without advancing the observer revision. subscribe() after dispose() used to register a listener that could never fire; it now throws LiveQueryObserverDisposedError. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The observer counted every delivery — including per-attach bootstrap replays and empty ready flushes — as a semantic revision. One readiness transition published three times ([], undefined, []), a plain unsubscribe/resubscribe manufactured a new snapshot identity with unchanged data, and rows committed while nothing was attached left the cached snapshot stale. The semantic clock now lives on the collection: emitEvents advances a monotonic stateRevision once per committed batch, whether or not anyone is subscribed. getSnapshot keys its cache on (stateRevision, status), so detached snapshots stay fresh and attachment replay can no longer advance the clock. Empty change batches are dropped from publication — only real deltas and the synthetic ready notify go out — so a readiness transition publishes exactly once. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…contract
The hand-rolled mock notified subscribers with empty change batches as a
wake-up signal — something real collections never do — and lacked the
state revision and status event channel the observer relies on. It now
advances _stateRevision on committed changes, emits real delete/insert
deltas from __replaceAll, and publishes status transitions through
on('status:change') instead of an empty notify.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The observer consumed row changes and onFirstReady but not the
collection's status events: a mounted consumer could sit on a stale
loading/ready status after an error or cleaned-up transition until an
unrelated row event happened to arrive. Status changes now publish a
synthetic notify through the same canonical path as data changes.
This also retires the onFirstReady registration, whose callbacks could
not be unsubscribed and accumulated across attach/detach cycles while
loading — collection.on('status:change') returns a real unsubscribe that
detach releases.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # packages/db/src/collection/changes.ts # packages/db/src/collection/index.ts # packages/db/src/live-query-observer.ts # packages/db/tests/live-query-observer.test.ts
|
I reproduced the reported failures against head 1. Render-time activation and callback generation
This reproduction fails with const never = new Promise<void>(() => {})
function AbandonedQuery(): ReactNode {
useLiveInfiniteQuery(
(q) => q.from({ row: source }).orderBy(({ row }) => row.n, `asc`),
{ pageSize: 3 },
)
throw never
}
render(
<Suspense fallback={null}>
<AbandonedQuery />
</Suspense>,
)
await new Promise((resolve) => setTimeout(resolve, 0))
expect(source.subscriberCount).toBe(0)The stable pagination callback has a related generation problem: const fetchNextPage = useCallback(() => {
controllerRef.current?.fetchNextPage()
}, [])A callback returned for controller A follows the mutable ref after a later render replaces it with controller B. Retaining A's callback, replacing the collection, and then invoking A's callback advances B to two pages. Please make collection construction inert and let the first committed const fetchNextPage = useCallback(() => {
void controller.fetchNextPage()
}, [controller])One clarification: a normal Strict Mode mount/unmount sequence did return subscriber counts to baseline. The verified lifecycle failure is an abandoned render that never commits. 2. Dependency identity collisions
This test remains on group const { result, rerender } = renderHook(
({ filter }: { filter: Map<string, string> }) =>
useLiveInfiniteQuery(
(q) =>
q
.from({ row: source })
.where(({ row }) => eq(row.group, filter.get(`group`)))
.orderBy(({ row }) => row.n, `asc`),
{ pageSize: 3 },
[filter],
),
{ initialProps: { filter: new Map([[`group`, `a`]]) } },
)
await waitFor(() =>
expect(result.current.data.every((row) => row.group === `a`)).toBe(true),
)
rerender({ filter: new Map([[`group`, `b`]]) })
await waitFor(() =>
expect(result.current.data.every((row) => row.group === `b`)).toBe(true),
)Functions, Please match 3. Changing page shape discards loaded pagesRecreating the controller when I loaded three 3-row pages, changed await waitFor(() => expect(result.current.isReady).toBe(true))
act(() => result.current.fetchNextPage())
await waitFor(() => expect(result.current.pages).toHaveLength(2))
act(() => result.current.fetchNextPage())
await waitFor(() => expect(result.current.pages).toHaveLength(3))
rerender({ pageSize: 5 })
await waitFor(() => expect(result.current.pages).toHaveLength(3))
expect(result.current.data).toHaveLength(15)If this is meant to preserve public behavior, carry the committed page count into the replacement controller or add a reconfiguration operation. If reset semantics are intentional, please document the behavior change and test it after multiple pages have loaded. The current runtime page-size test changes the size while only one page exists, so it cannot distinguish these semantics. 4. Failed window updates commit pagination and poison retries
A synchronous failure poisons the next attempt: const setWindow = vi
.spyOn(collection.utils, `setWindow`)
.mockImplementationOnce(() => {
throw new Error(`window failed`)
})
.mockReturnValue(true)
expect(() => controller.subscribe(() => {})).toThrow(`window failed`)
controller.subscribe(() => {})
expect(setWindow).toHaveBeenCalledTimes(2)Actual result: An asynchronous failure also commits the next page: expect(controller.getSnapshot().hasNextPage).toBe(true)
vi.spyOn(collection.utils, `setWindow`).mockRejectedValueOnce(
new Error(`load failed`),
)
await expect(controller.fetchNextPage()).rejects.toThrow(`load failed`)
expect(controller.getSnapshot().pages).toHaveLength(1)
expect(controller.getSnapshot().hasNextPage).toBe(true)On this head, the controller exposes two pages after rejection. It can also lose Please separate committed page count, requested page count, pending generation/limit, accepted limit, and pagination error. Keep slicing at the committed count until the current-generation window request succeeds. On failure, retain the old pages and 5. Async expansion publishes duplicate and incoherent snapshotsFor an asynchronous window expansion, This sequence test receives two identical immediate const snapshots: Array<{ pages: number; fetching: boolean }> = []
controller.subscribe(() => {
const snapshot = controller.getSnapshot()
snapshots.push({
pages: snapshot.pages.length,
fetching: snapshot.isFetchingNextPage,
})
})
let resolveWindow!: () => void
vi.spyOn(collection.utils, `setWindow`).mockReturnValueOnce(
new Promise<void>((resolve) => {
resolveWindow = resolve
}),
)
const fetch = controller.fetchNextPage()
expect(snapshots).toEqual([{ pages: 1, fetching: true }])
resolveWindow()
await fetch
expect(snapshots).toEqual([
{ pages: 1, fetching: true },
{ pages: 2, fetching: false },
])Please place observer delivery behind a publication barrier while requested window, fetching state, and committed page count change. Emit one coherent loading snapshot and one success or failure snapshot. Synchronous observer notifications raised by 6. Preload and pipeline restart lose the desired windowThe controller's Reproduction: const unsubscribe = controller.subscribe(() => {})
await collection.preload()
controller.fetchNextPage()
await flush()
expect(controller.getSnapshot().data).toHaveLength(4)
unsubscribe()
await collection.cleanup()
controller.subscribe(() => {})
await collection.preload()
await flush()
expect(controller.getSnapshot().data).toHaveLength(4)
expect(controller.getSnapshot().hasNextPage).toBe(true)With an original AST limit of 3 and a controller-expanded limit of 5, the restarted result contains only 3 rows. The controller skips reapplication because
await controller.preload()
expect(controller.getSnapshot().hasNextPage).toBe(true)
expect(collection.utils.getWindow()).toEqual({ offset: 0, limit: 3 })Please store the desired window as durable builder state and replay it whenever 7. Multiple controllers overwrite one shared collection windowEach controller stores a private page count, but const larger = createLiveQueryWindowController(collection, {
pageSize: 2,
mode: `wholesale`,
})
const smaller = createLiveQueryWindowController(collection, {
pageSize: 1,
mode: `wholesale`,
})
larger.subscribe(() => {})
smaller.subscribe(() => {})
await collection.preload()
await flush()
expect(larger.getSnapshot().data).toHaveLength(2)
expect(larger.getSnapshot().hasNextPage).toBe(true)The second assertion fails. The smaller controller applies limit 2 after the larger controller applies limit 3. The larger controller then loses its peek row and reports For the current forward-only, offset-zero model, a collection-owned lease coordinator can apply the maximum active requested limit while each controller slices its own snapshot. Updating or releasing a lease recomputes the physical window. This also gives pipeline recompilation one authoritative desired window to reinstall. If shared ownership is out of scope, please enforce exclusive ownership and throw when a second controller attaches; silent last-writer-wins behavior corrupts pagination state. 8. The default observer mode does not match the controller contractThe controller inherits the observer's default granular mode, but its listener is only With default options, this test fails because the listener fires synchronously inside await collection.preload()
const controller = createLiveQueryWindowController(collection, {
pageSize: 2,
})
let subscribing = true
let notifiedDuringSubscribe = false
controller.subscribe(() => {
if (subscribing) notifiedDuringSubscribe = true
})
subscribing = false
expect(notifiedDuringSubscribe).toBe(false)The existing no-synchronous-notify test passes Please make the window controller wholesale and non-reentrant by construction and remove API boundaryRFC #1623 describes this as a shared internal controller and explicitly requires promise/error semantics, non-reentrant subscriptions, inert rendering, one atomic revision per observable change, and single ownership of the active window. This PR root-exports the controller and advertises it in a changeset without an Please either complete that contract before presenting the controller as a new framework-neutral feature, or mark the factory, controller, options, and snapshot as internal/unstable for this phase and update the changeset accordingly. VerificationI implemented proof fixes locally and reran the focused regressions plus both full package suites:
I recommend keeping the extraction. The main design change is to treat requested pagination state, committed pagination state, and the collection-owned physical window as separate concepts. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/react-db/src/useLiveInfiniteQuery.ts (1)
158-167: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReplace
JSON.stringify(deps)with elementwise identity comparison.
JSON.stringifyis not injective over the values React users put in a deps array, so distinct deps produce an equaldepsKeyand the hook does not recreate the collection. The query keeps its stale captured values.Concrete collisions:
- Any two
MaporSetvalues serialize to{}.- Any two functions serialize to
null, and[fn]equals[undefined].- Objects that differ only in
undefined-valued or non-enumerable fields serialize identically.Elementwise
Object.iscomparison matches React's own deps semantics, is O(n) instead of a full serialization, and removes the circular-reference failure mode along with itsthrow.Note that removing the
throwchanges behavior for the circular-deps case. The test at Line 1964 ofpackages/react-db/tests/useLiveInfiniteQuery.test.tsxasserts that error and needs updating.🐛 Proposed fix using elementwise comparison
Replace the
depsKeyblock:- // Track deps for query functions (stringify for comparison) - let depsKey: string - try { - depsKey = JSON.stringify(deps) - } catch { - throw new Error( - `useLiveInfiniteQuery: dependency array contains values that cannot be serialized (e.g. circular references). ` + - `Ensure all dependency values are JSON-serializable.`, - ) - }Add a module-level helper:
/** React-style deps comparison: same length, elementwise `Object.is`. */ function depsChanged( previous: ReadonlyArray<unknown> | null, next: ReadonlyArray<unknown>, ): boolean { if (previous === null || previous.length !== next.length) return true return next.some((value, i) => !Object.is(value, previous[i])) }Then change the ref and the
needsNewterm:- const depsRef = useRef<string | null>(null) + const depsRef = useRef<ReadonlyArray<unknown> | null>(null)- (!isCollection && depsRef.current !== depsKey) + (!isCollection && depsChanged(depsRef.current, deps))- depsRef.current = depsKey + depsRef.current = [...deps]🤖 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/useLiveInfiniteQuery.ts` around lines 158 - 167, Replace the JSON.stringify-based depsKey logic in useLiveInfiniteQuery with module-level elementwise Object.is comparison that checks dependency length and identity, removing serialization and its circular-reference throw. Update the ref and needsNew logic to use this comparison so query collections recreate when any dependency identity changes, and update the circular-dependency test expectation accordingly.
🧹 Nitpick comments (8)
packages/react-db/src/useLiveInfiniteQuery.ts (2)
263-279: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe blanket cast can hide missing fields.
UseLiveInfiniteQueryReturn<TContext>is derived fromOmit<ReturnType<typeof useLiveQuery<TContext>>, 'data'>. Theascast at Line 279 suppresses any structural mismatch. IfuseLiveQuerygains a return field, this object omits it and the compiler stays silent, so consumers getundefinedat runtime for a field the type promises.Remove the assertion and let the object literal be checked against the declared return type. The per-field casts on
data,pages, andpageParamscan stay.🤖 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/useLiveInfiniteQuery.ts` around lines 263 - 279, Remove the blanket `as UseLiveInfiniteQueryReturn<TContext>` assertion from the object returned by the live infinite query hook, allowing structural type checking to catch omitted fields. Keep the existing per-field casts for `data`, `pages`, and `pageParams` unchanged.Source: Coding guidelines
22-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMake
hasSetWindowan actual type guard or correct the doc comment.The JSDoc calls this a type guard, but the return type is
boolean, so it narrows nothing. Callers still accesscollection.utils.getWindow?.()andcollection.utils.setWindowwithout type support.As per coding guidelines: "Always provide the most precise return type annotation".
♻️ Proposed predicate signature
+type WindowedCollection = Collection<any, any, any> & { + utils: { + setWindow: (w: { offset: number; limit: number }) => true | Promise<void> + getWindow?: () => { offset: number; limit: number } | undefined + } +} + /** Type guard: does this collection expose `setWindow` (i.e. has an orderBy)? */ -function hasSetWindow(collection: Collection<any, any, any>): boolean { +function hasSetWindow( + collection: Collection<any, any, any>, +): collection is WindowedCollection { return typeof collection.utils?.setWindow === `function` }🤖 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/useLiveInfiniteQuery.ts` around lines 22 - 25, Update hasSetWindow to use a type-predicate return annotation that narrows the collection to one whose utils exposes setWindow, matching the existing JSDoc and enabling type-safe access for callers. Preserve the current runtime function check and use the narrowed type in the predicate target.Source: Coding guidelines
packages/react-db/tests/useLiveInfiniteQuery.test.tsx (2)
702-739: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtend the test to cover a
pageSizechange after pages are loaded.The test changes
pageSizewhile only page 1 is loaded, so it cannot observe what happens to already loaded pages.The hook recreates the controller on a
pageSizechange, and a fresh controller starts atloadedPageCount = 1. If a user has paged to 3 pages andpageSizethen changes, the loaded pages are discarded. The PR objectives record this as an open concern about changing established behavior. No test pins the intended outcome.Add a case that calls
fetchNextPage()before the rerender, then asserts the intended row count. That documents the decision and catches an unintended change later.🤖 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/tests/useLiveInfiniteQuery.test.tsx` around lines 702 - 739, Extend the “re-windows and re-slices when pageSize changes at runtime” test to call result.current.fetchNextPage() and wait until a second page is loaded before rerendering with the new pageSize. After the rerender, assert the intended data and pages row counts, explicitly documenting whether previously loaded pages are preserved or discarded.
1801-1813: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAwait
setWindowand assert the window is actually adjusted.Two problems in this test.
Line 1804 discards the
setWindowreturn value.setWindowreturnstrue | Promise<void>, as declared at Lines 291-294 ofpackages/db/src/live-query-window-controller.ts. If it returns a promise, the window is not established whenrenderHookruns at Line 1808. The test still passes today because the query was built with.limit(5), sogetWindow()reports limit 5 either way. The precondition is therefore not deterministic.The test asserts only that the warning fires. The warning text promises "Adjusting window now.", and the test never checks that the window becomes
{offset: 0, limit: 11}. The observable outcome is the part that matters to users.As per coding guidelines: "Test corner cases including: empty arrays/sets, single-element collections, undefined vs null values, resolved promises, async race conditions, and limit/offset edge cases".
💚 Proposed test hardening
- liveQueryCollection.utils.setWindow({ offset: 0, limit: 5 }) + const applied = liveQueryCollection.utils.setWindow({ + offset: 0, + limit: 5, + }) + if (applied !== true) await applied const warn = vi.spyOn(console, `warn`).mockImplementation(() => {}) try { renderHook(() => useLiveInfiniteQuery(liveQueryCollection, { pageSize: 10 }), ) expect(warn).toHaveBeenCalledWith( expect.stringContaining(`Pre-created collection has window`), ) + // The warning promises the window is adjusted; verify it. + await waitFor(() => + expect(liveQueryCollection.utils.getWindow?.()).toEqual({ + offset: 0, + limit: 11, + }), + ) } finally {🤖 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/tests/useLiveInfiniteQuery.test.tsx` around lines 1801 - 1813, Make the test await the return value of liveQueryCollection.utils.setWindow before rendering the hook, handling both synchronous and promise results. After renderHook, retain the warning assertion and also assert that the collection window is adjusted to offset 0 and limit 11, using the collection’s existing window-inspection API.Source: Coding guidelines
packages/db/tests/live-query-window-controller.test.ts (3)
101-116: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for an empty source and for a falsy
pageSize.The suite covers a partial last page and a source smaller than one page. Two documented edges are untested.
- An empty source collection.
getSnapshot()reportsenabled === true, so the loop at Lines 167-170 still produces one page, andpagesbecomes[[]]withhasNextPage === false. Lines 162-163 state this is intended. A test locks it in.pageSize: 0. The constructor at Line 130 uses||, so the value falls back toDEFAULT_PAGE_SIZE. Without a test, a later change to??would silently maketotalRequestedzero and keephasNextPagepermanently true.As per coding guidelines: "Test corner cases including: empty arrays/sets, single-element collections, undefined vs null values, resolved promises, async race conditions, and limit/offset edge cases".
🤖 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-window-controller.test.ts` around lines 101 - 116, Add tests alongside the existing live-query window controller pagination tests for an empty source and for a controller created with pageSize 0. Assert the empty source yields an enabled snapshot with pages containing one empty page and hasNextPage false; assert pageSize 0 uses the default page size and does not leave hasNextPage permanently true. Use the existing helpers and cleanup pattern.Source: Coding guidelines
150-153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTighten the notification assertions.
Both tests assert
toBeGreaterThan(0). That passes for any notification count, so it cannot detect duplicate or redundant publications.The PR objectives record a known concern that async window expansion can emit duplicate snapshots and should be batched into one loading and one settled notification. With the current assertion, a regression in publication count stays invisible, and a future batching change produces no test signal either.
Assert a bounded count, for example
expect(notifications).toBeLessThanOrEqual(2)alongside the existing lower bound, so the publication shape is pinned.Also applies to: 189-194
🤖 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-window-controller.test.ts` around lines 150 - 153, In both notification assertions around controller.fetchNextPage(), retain the existing positive lower-bound check and add an upper-bound assertion limiting notifications to two. Apply this to both referenced tests so async window expansion is constrained to at most one loading and one settled publication.
252-274: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression test for unsubscribe during a publication.
This test covers
dispose()from inside a listener. It does not cover the adjacent case: one listener unsubscribes a different listener during the same publication.The dispatch loop in
live-query-window-controller.tsat Lines 336-342 iterates a captured target list and never readsrecord.active, so the unsubscribed listener still receives the call. A test that mirrors this one, but calls the second subscriber's unsubscribe function instead ofdispose(), reproduces it.As per coding guidelines: "Always add unit tests that reproduce a bug before fixing it to ensure the bug is fixed and prevent regression".
🤖 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-window-controller.test.ts` around lines 252 - 274, Add a regression test alongside the existing in-flight publication test that stores the second subscription’s unsubscribe function, has the first listener invoke it during publication, and asserts the second listener is not notified. Preserve the same setup and publication trigger used by the existing test, targeting the controller’s listener dispatch behavior rather than calling controller.dispose().Source: Coding guidelines
packages/db/src/index.ts (1)
15-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMark the controller surface internal or unstable before release.
Line 15 re-exports every symbol from
live-query-window-controller, socreateLiveQueryWindowController,LiveQueryWindowController,LiveQueryWindowSnapshot, andCreateLiveQueryWindowControllerOptionsbecome public API of@tanstack/db. The stack includes a changeset, so this ships as a supported surface.The PR still lists open design questions for window semantics, forward-only versus bidirectional pagination,
getNextPageParam, and window ownership for pre-created collections. Publishing the API now creates a compatibility obligation before those decisions are settled.Add an
@internal/unstable note to the module doc comment, matching howlive-query-observerwas documented. Based on the PR objectives, which state the reviewer asked either to complete the RFC contract or mark the API as internal/unstable.🤖 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/index.ts` at line 15, Mark the live query window controller module as internal or unstable in its module documentation, following the existing documentation pattern used by live-query-observer. Update the doc comment associated with the symbols re-exported by the index rather than changing the re-export itself, and clearly indicate the API is not yet a supported public contract.
🤖 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 @.changeset/live-query-window-controller.md:
- Around line 2-3: The changeset must not publish
createLiveQueryWindowController and LiveQueryWindowController as an unresolved
stable patch API. Either finalize and document their RFC contract, update
`@tanstack/db` to a minor release, or explicitly mark the exports
unstable/internal and state that status in the changeset.
In `@packages/db/src/live-query-window-controller.ts`:
- Around line 336-342: Update the publication dispatch loop in
packages/db/src/live-query-window-controller.ts lines 336-342 to skip records
whose SubscriptionRecord.active is false before invoking record.listener(). Add
a test in packages/db/tests/live-query-window-controller.test.ts lines 252-274
mirroring the dispose-during-publication case, but unsubscribe the second
subscriber and assert it receives no notification.
- Around line 280-318: Update applyWindow in
packages/db/src/live-query-window-controller.ts:280-318 to validate setWindow
before changing appliedLimit, commit the limit only after the call succeeds, and
restore the prior invalidated state when it throws synchronously or its promise
rejects. In the last-subscriber detach path at
packages/db/src/live-query-window-controller.ts:227-235, clear appliedLimit so
the next subscribe re-applies the window.
In `@packages/react-db/src/useLiveInfiniteQuery.ts`:
- Around line 188-243: Dispose the existing controller before replacing it in
the needsNew block, and add an unmount cleanup effect that disposes and clears
controllerRef.current. Update the React imports to include useEffect, ensuring
every controller created by createLiveQueryWindowController is released on
parameter changes and unmount.
---
Outside diff comments:
In `@packages/react-db/src/useLiveInfiniteQuery.ts`:
- Around line 158-167: Replace the JSON.stringify-based depsKey logic in
useLiveInfiniteQuery with module-level elementwise Object.is comparison that
checks dependency length and identity, removing serialization and its
circular-reference throw. Update the ref and needsNew logic to use this
comparison so query collections recreate when any dependency identity changes,
and update the circular-dependency test expectation accordingly.
---
Nitpick comments:
In `@packages/db/src/index.ts`:
- Line 15: Mark the live query window controller module as internal or unstable
in its module documentation, following the existing documentation pattern used
by live-query-observer. Update the doc comment associated with the symbols
re-exported by the index rather than changing the re-export itself, and clearly
indicate the API is not yet a supported public contract.
In `@packages/db/tests/live-query-window-controller.test.ts`:
- Around line 101-116: Add tests alongside the existing live-query window
controller pagination tests for an empty source and for a controller created
with pageSize 0. Assert the empty source yields an enabled snapshot with pages
containing one empty page and hasNextPage false; assert pageSize 0 uses the
default page size and does not leave hasNextPage permanently true. Use the
existing helpers and cleanup pattern.
- Around line 150-153: In both notification assertions around
controller.fetchNextPage(), retain the existing positive lower-bound check and
add an upper-bound assertion limiting notifications to two. Apply this to both
referenced tests so async window expansion is constrained to at most one loading
and one settled publication.
- Around line 252-274: Add a regression test alongside the existing in-flight
publication test that stores the second subscription’s unsubscribe function, has
the first listener invoke it during publication, and asserts the second listener
is not notified. Preserve the same setup and publication trigger used by the
existing test, targeting the controller’s listener dispatch behavior rather than
calling controller.dispose().
In `@packages/react-db/src/useLiveInfiniteQuery.ts`:
- Around line 263-279: Remove the blanket `as
UseLiveInfiniteQueryReturn<TContext>` assertion from the object returned by the
live infinite query hook, allowing structural type checking to catch omitted
fields. Keep the existing per-field casts for `data`, `pages`, and `pageParams`
unchanged.
- Around line 22-25: Update hasSetWindow to use a type-predicate return
annotation that narrows the collection to one whose utils exposes setWindow,
matching the existing JSDoc and enabling type-safe access for callers. Preserve
the current runtime function check and use the narrowed type in the predicate
target.
In `@packages/react-db/tests/useLiveInfiniteQuery.test.tsx`:
- Around line 702-739: Extend the “re-windows and re-slices when pageSize
changes at runtime” test to call result.current.fetchNextPage() and wait until a
second page is loaded before rerendering with the new pageSize. After the
rerender, assert the intended data and pages row counts, explicitly documenting
whether previously loaded pages are preserved or discarded.
- Around line 1801-1813: Make the test await the return value of
liveQueryCollection.utils.setWindow before rendering the hook, handling both
synchronous and promise results. After renderHook, retain the warning assertion
and also assert that the collection window is adjusted to offset 0 and limit 11,
using the collection’s existing window-inspection API.
🪄 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: 7f080acc-7635-4a4b-828d-b2a6bf46a00e
📒 Files selected for processing (7)
.changeset/live-query-window-controller.mdpackages/db/src/errors.tspackages/db/src/index.tspackages/db/src/live-query-window-controller.tspackages/db/tests/live-query-window-controller.test.tspackages/react-db/src/useLiveInfiniteQuery.tspackages/react-db/tests/useLiveInfiniteQuery.test.tsx
| if (needsNew) { | ||
| pageSizeRef.current = pageSize | ||
| initialPageParamRef.current = initialPageParam | ||
| if (isCollection) { | ||
| // Reset if collection instance changed | ||
| if (collectionRef.current !== queryFnOrCollection) { | ||
| collectionRef.current = queryFnOrCollection | ||
| hasValidatedCollectionRef.current = false | ||
| shouldReset = true | ||
| } | ||
| } else { | ||
| // Reset if deps changed (for query functions) | ||
| if (prevDepsKeyRef.current !== depsKey) { | ||
| prevDepsKeyRef.current = depsKey | ||
| shouldReset = true | ||
| } | ||
| } | ||
|
|
||
| if (shouldReset) { | ||
| setLoadedPageCount(1) | ||
| } | ||
| }, [isCollection, queryFnOrCollection, depsKey]) | ||
|
|
||
| // Create a live query with initial limit and offset | ||
| // Either pass collection directly or wrap query function | ||
| // Use pageSize + 1 for peek-ahead detection (to know if there are more pages) | ||
| const queryResult = isCollection | ||
| ? useLiveQuery(queryFnOrCollection) | ||
| : useLiveQuery( | ||
| (q) => | ||
| queryFnOrCollection(q) | ||
| .limit(pageSize + 1) | ||
| .offset(0), | ||
| deps, | ||
| ) | ||
|
|
||
| // Adjust window when pagination changes | ||
| useEffect(() => { | ||
| const utils = queryResult.collection.utils | ||
| const expectedOffset = 0 | ||
| const expectedLimit = loadedPageCount * pageSize + 1 // +1 for peek ahead | ||
|
|
||
| // Check if collection has orderBy (required for setWindow) | ||
| if (!isLiveQueryCollectionUtils(utils)) { | ||
| // For pre-created collections, throw an error if no orderBy | ||
| if (isCollection) { | ||
| const collection = queryFnOrCollection as Collection<any, any, any> | ||
| if (!hasSetWindow(collection)) { | ||
| throw new Error( | ||
| `useLiveInfiniteQuery: Pre-created live query collection must have an orderBy clause for infinite pagination to work. ` + | ||
| `Please add .orderBy() to your createLiveQueryCollection query.`, | ||
| ) | ||
| } | ||
| return | ||
| } | ||
|
|
||
| // For pre-created collections, validate window on first check | ||
| if (isCollection && !hasValidatedCollectionRef.current) { | ||
| const currentWindow = utils.getWindow() | ||
| if ( | ||
| currentWindow && | ||
| (currentWindow.offset !== expectedOffset || | ||
| currentWindow.limit !== expectedLimit) | ||
| ) { | ||
| console.warn( | ||
| `useLiveInfiniteQuery: Pre-created collection has window {offset: ${currentWindow.offset}, limit: ${currentWindow.limit}} ` + | ||
| `but hook expects {offset: ${expectedOffset}, limit: ${expectedLimit}}. Adjusting window now.`, | ||
| ) | ||
| // Warn once per collection instance if its current window doesn't match | ||
| // the first page the hook is about to enforce. | ||
| if (validatedCollectionRef.current !== collection) { | ||
| validatedCollectionRef.current = collection | ||
| const currentWindow = collection.utils.getWindow?.() | ||
| if ( | ||
| currentWindow && | ||
| (currentWindow.offset !== 0 || currentWindow.limit !== pageSize + 1) | ||
| ) { | ||
| console.warn( | ||
| `useLiveInfiniteQuery: Pre-created collection has window {offset: ${currentWindow.offset}, limit: ${currentWindow.limit}} ` + | ||
| `but the hook expects {offset: 0, limit: ${pageSize + 1}}. Adjusting window now.`, | ||
| ) | ||
| } | ||
| } | ||
| hasValidatedCollectionRef.current = true | ||
| } | ||
|
|
||
| // For query functions, wait until collection is ready | ||
| if (!isCollection && !queryResult.isReady) return | ||
|
|
||
| // Adjust the window | ||
| let cancelled = false | ||
| const result = utils.setWindow({ | ||
| offset: expectedOffset, | ||
| limit: expectedLimit, | ||
| }) | ||
|
|
||
| if (result !== true) { | ||
| setIsFetchingNextPage(true) | ||
| result | ||
| .catch((error: unknown) => { | ||
| if (!cancelled) | ||
| console.error(`useLiveInfiniteQuery: setWindow failed:`, error) | ||
| }) | ||
| .finally(() => { | ||
| if (!cancelled) setIsFetchingNextPage(false) | ||
| }) | ||
| collection.startSyncImmediate() | ||
| collectionRef.current = collection | ||
| configRef.current = queryFnOrCollection | ||
| } else { | ||
| setIsFetchingNextPage(false) | ||
| } | ||
|
|
||
| return () => { | ||
| cancelled = true | ||
| } | ||
| }, [ | ||
| isCollection, | ||
| queryResult.collection, | ||
| queryResult.isReady, | ||
| loadedPageCount, | ||
| pageSize, | ||
| ]) | ||
|
|
||
| // Split the data array into pages and determine if there's a next page | ||
| const { pages, pageParams, hasNextPage, flatData } = useMemo(() => { | ||
| const dataArray = ( | ||
| Array.isArray(queryResult.data) ? queryResult.data : [] | ||
| ) as InferResultType<TContext> | ||
| const totalItemsRequested = loadedPageCount * pageSize | ||
|
|
||
| // Check if we have more data than requested (the peek ahead item) | ||
| const hasMore = dataArray.length > totalItemsRequested | ||
|
|
||
| // Build pages array (without the peek ahead item) | ||
| const pagesResult: Array<Array<InferResultType<TContext>[number]>> = [] | ||
| const pageParamsResult: Array<number> = [] | ||
|
|
||
| for (let i = 0; i < loadedPageCount; i++) { | ||
| const pageData = dataArray.slice(i * pageSize, (i + 1) * pageSize) | ||
| pagesResult.push(pageData) | ||
| pageParamsResult.push(initialPageParam + i) | ||
| // Wrap the query with the first page's peek-ahead window; the controller | ||
| // grows the limit from here via setWindow. | ||
| collectionRef.current = createLiveQueryCollection({ | ||
| query: (q: InitialQueryBuilder) => | ||
| queryFnOrCollection(q) | ||
| .limit(pageSize + 1) | ||
| .offset(0), | ||
| startSync: true, | ||
| gcTime: DEFAULT_GC_TIME_MS, | ||
| }) | ||
| depsRef.current = depsKey | ||
| } | ||
| controllerRef.current = createLiveQueryWindowController( | ||
| collectionRef.current, | ||
| { | ||
| pageSize, | ||
| initialPageParam, | ||
| // Wholesale mode provides useSyncExternalStore's no-sync-notify contract. | ||
| mode: 'wholesale', | ||
| // A query-function collection already carries page 1's window in its | ||
| // query, so defer the (redundant) first apply until it is ready; a | ||
| // pre-created collection needs its window established up front. | ||
| waitForReady: !isCollection, | ||
| }, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Dispose the previous controller and the collection it owns.
Line 230 overwrites controllerRef.current without calling dispose() on the controller it replaces. The hook never disposes a controller at all — there is no effect in this file that runs teardown on unmount.
Two consequences:
- Every
pageSizeorinitialPageParamchange leaks a controller and its observer. The new test at Line 702 ofpackages/react-db/tests/useLiveInfiniteQuery.test.tsxdrives this path directly. - In the query-function branch, Line 220 creates a live-query collection with
startSync: true. The replaced collection is abandoned while syncing.gcTime: 1may reclaim the collection, but the controller that wraps it is never told to release the observer.
A related risk sits in the same block. Line 214 calls startSyncImmediate(), and Line 220 starts sync, both during render. React discards renders in Strict Mode, under Suspense, and on concurrent interruption. Each discarded render activates synchronization that nothing tears down.
Add disposal for the replaced controller, and add an unmount effect that disposes the final one. Moving activation into an effect is the fuller fix and is worth planning before this ships.
🐛 Minimal fix: dispose the replaced controller and dispose on unmount
if (needsNew) {
+ // Release the controller (and its observer subscription) being replaced.
+ controllerRef.current?.dispose()
pageSizeRef.current = pageSize
initialPageParamRef.current = initialPageParamAdd an unmount-only effect after the controller is resolved:
// Dispose whichever controller is current when the hook unmounts.
useEffect(() => {
return () => {
controllerRef.current?.dispose()
controllerRef.current = null
}
}, [])useEffect must be added to the react import at Line 1.
📝 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.
| if (needsNew) { | |
| pageSizeRef.current = pageSize | |
| initialPageParamRef.current = initialPageParam | |
| if (isCollection) { | |
| // Reset if collection instance changed | |
| if (collectionRef.current !== queryFnOrCollection) { | |
| collectionRef.current = queryFnOrCollection | |
| hasValidatedCollectionRef.current = false | |
| shouldReset = true | |
| } | |
| } else { | |
| // Reset if deps changed (for query functions) | |
| if (prevDepsKeyRef.current !== depsKey) { | |
| prevDepsKeyRef.current = depsKey | |
| shouldReset = true | |
| } | |
| } | |
| if (shouldReset) { | |
| setLoadedPageCount(1) | |
| } | |
| }, [isCollection, queryFnOrCollection, depsKey]) | |
| // Create a live query with initial limit and offset | |
| // Either pass collection directly or wrap query function | |
| // Use pageSize + 1 for peek-ahead detection (to know if there are more pages) | |
| const queryResult = isCollection | |
| ? useLiveQuery(queryFnOrCollection) | |
| : useLiveQuery( | |
| (q) => | |
| queryFnOrCollection(q) | |
| .limit(pageSize + 1) | |
| .offset(0), | |
| deps, | |
| ) | |
| // Adjust window when pagination changes | |
| useEffect(() => { | |
| const utils = queryResult.collection.utils | |
| const expectedOffset = 0 | |
| const expectedLimit = loadedPageCount * pageSize + 1 // +1 for peek ahead | |
| // Check if collection has orderBy (required for setWindow) | |
| if (!isLiveQueryCollectionUtils(utils)) { | |
| // For pre-created collections, throw an error if no orderBy | |
| if (isCollection) { | |
| const collection = queryFnOrCollection as Collection<any, any, any> | |
| if (!hasSetWindow(collection)) { | |
| throw new Error( | |
| `useLiveInfiniteQuery: Pre-created live query collection must have an orderBy clause for infinite pagination to work. ` + | |
| `Please add .orderBy() to your createLiveQueryCollection query.`, | |
| ) | |
| } | |
| return | |
| } | |
| // For pre-created collections, validate window on first check | |
| if (isCollection && !hasValidatedCollectionRef.current) { | |
| const currentWindow = utils.getWindow() | |
| if ( | |
| currentWindow && | |
| (currentWindow.offset !== expectedOffset || | |
| currentWindow.limit !== expectedLimit) | |
| ) { | |
| console.warn( | |
| `useLiveInfiniteQuery: Pre-created collection has window {offset: ${currentWindow.offset}, limit: ${currentWindow.limit}} ` + | |
| `but hook expects {offset: ${expectedOffset}, limit: ${expectedLimit}}. Adjusting window now.`, | |
| ) | |
| // Warn once per collection instance if its current window doesn't match | |
| // the first page the hook is about to enforce. | |
| if (validatedCollectionRef.current !== collection) { | |
| validatedCollectionRef.current = collection | |
| const currentWindow = collection.utils.getWindow?.() | |
| if ( | |
| currentWindow && | |
| (currentWindow.offset !== 0 || currentWindow.limit !== pageSize + 1) | |
| ) { | |
| console.warn( | |
| `useLiveInfiniteQuery: Pre-created collection has window {offset: ${currentWindow.offset}, limit: ${currentWindow.limit}} ` + | |
| `but the hook expects {offset: 0, limit: ${pageSize + 1}}. Adjusting window now.`, | |
| ) | |
| } | |
| } | |
| hasValidatedCollectionRef.current = true | |
| } | |
| // For query functions, wait until collection is ready | |
| if (!isCollection && !queryResult.isReady) return | |
| // Adjust the window | |
| let cancelled = false | |
| const result = utils.setWindow({ | |
| offset: expectedOffset, | |
| limit: expectedLimit, | |
| }) | |
| if (result !== true) { | |
| setIsFetchingNextPage(true) | |
| result | |
| .catch((error: unknown) => { | |
| if (!cancelled) | |
| console.error(`useLiveInfiniteQuery: setWindow failed:`, error) | |
| }) | |
| .finally(() => { | |
| if (!cancelled) setIsFetchingNextPage(false) | |
| }) | |
| collection.startSyncImmediate() | |
| collectionRef.current = collection | |
| configRef.current = queryFnOrCollection | |
| } else { | |
| setIsFetchingNextPage(false) | |
| } | |
| return () => { | |
| cancelled = true | |
| } | |
| }, [ | |
| isCollection, | |
| queryResult.collection, | |
| queryResult.isReady, | |
| loadedPageCount, | |
| pageSize, | |
| ]) | |
| // Split the data array into pages and determine if there's a next page | |
| const { pages, pageParams, hasNextPage, flatData } = useMemo(() => { | |
| const dataArray = ( | |
| Array.isArray(queryResult.data) ? queryResult.data : [] | |
| ) as InferResultType<TContext> | |
| const totalItemsRequested = loadedPageCount * pageSize | |
| // Check if we have more data than requested (the peek ahead item) | |
| const hasMore = dataArray.length > totalItemsRequested | |
| // Build pages array (without the peek ahead item) | |
| const pagesResult: Array<Array<InferResultType<TContext>[number]>> = [] | |
| const pageParamsResult: Array<number> = [] | |
| for (let i = 0; i < loadedPageCount; i++) { | |
| const pageData = dataArray.slice(i * pageSize, (i + 1) * pageSize) | |
| pagesResult.push(pageData) | |
| pageParamsResult.push(initialPageParam + i) | |
| // Wrap the query with the first page's peek-ahead window; the controller | |
| // grows the limit from here via setWindow. | |
| collectionRef.current = createLiveQueryCollection({ | |
| query: (q: InitialQueryBuilder) => | |
| queryFnOrCollection(q) | |
| .limit(pageSize + 1) | |
| .offset(0), | |
| startSync: true, | |
| gcTime: DEFAULT_GC_TIME_MS, | |
| }) | |
| depsRef.current = depsKey | |
| } | |
| controllerRef.current = createLiveQueryWindowController( | |
| collectionRef.current, | |
| { | |
| pageSize, | |
| initialPageParam, | |
| // Wholesale mode provides useSyncExternalStore's no-sync-notify contract. | |
| mode: 'wholesale', | |
| // A query-function collection already carries page 1's window in its | |
| // query, so defer the (redundant) first apply until it is ready; a | |
| // pre-created collection needs its window established up front. | |
| waitForReady: !isCollection, | |
| }, | |
| ) | |
| } | |
| if (needsNew) { | |
| // Release the controller (and its observer subscription) being replaced. | |
| controllerRef.current?.dispose() | |
| pageSizeRef.current = pageSize | |
| initialPageParamRef.current = initialPageParam | |
| if (isCollection) { | |
| const collection = queryFnOrCollection as Collection<any, any, any> | |
| if (!hasSetWindow(collection)) { | |
| throw new Error( | |
| `useLiveInfiniteQuery: Pre-created live query collection must have an orderBy clause for infinite pagination to work. ` + | |
| `Please add .orderBy() to your createLiveQueryCollection query.`, | |
| ) | |
| } | |
| // Warn once per collection instance if its current window doesn't match | |
| // the first page the hook is about to enforce. | |
| if (validatedCollectionRef.current !== collection) { | |
| validatedCollectionRef.current = collection | |
| const currentWindow = collection.utils.getWindow?.() | |
| if ( | |
| currentWindow && | |
| (currentWindow.offset !== 0 || currentWindow.limit !== pageSize + 1) | |
| ) { | |
| console.warn( | |
| `useLiveInfiniteQuery: Pre-created collection has window {offset: ${currentWindow.offset}, limit: ${currentWindow.limit}} ` + | |
| `but the hook expects {offset: 0, limit: ${pageSize + 1}}. Adjusting window now.`, | |
| ) | |
| } | |
| } | |
| collection.startSyncImmediate() | |
| collectionRef.current = collection | |
| configRef.current = queryFnOrCollection | |
| } else { | |
| // Wrap the query with the first page's peek-ahead window; the controller | |
| // grows the limit from here via setWindow. | |
| collectionRef.current = createLiveQueryCollection({ | |
| query: (q: InitialQueryBuilder) => | |
| queryFnOrCollection(q) | |
| .limit(pageSize + 1) | |
| .offset(0), | |
| startSync: true, | |
| gcTime: DEFAULT_GC_TIME_MS, | |
| }) | |
| depsRef.current = depsKey | |
| } | |
| controllerRef.current = createLiveQueryWindowController( | |
| collectionRef.current, | |
| { | |
| pageSize, | |
| initialPageParam, | |
| // Wholesale mode provides useSyncExternalStore's no-sync-notify contract. | |
| mode: 'wholesale', | |
| // A query-function collection already carries page 1's window in its | |
| // query, so defer the (redundant) first apply until it is ready; a | |
| // pre-created collection needs its window established up front. | |
| waitForReady: !isCollection, | |
| }, | |
| ) | |
| } | |
| // Dispose whichever controller is current when the hook unmounts. | |
| useEffect(() => { | |
| return () => { | |
| controllerRef.current?.dispose() | |
| controllerRef.current = null | |
| } | |
| }, []) |
🤖 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/useLiveInfiniteQuery.ts` around lines 188 - 243,
Dispose the existing controller before replacing it in the needsNew block, and
add an unmount cleanup effect that disposes and clears controllerRef.current.
Update the React imports to include useEffect, ensuring every controller created
by createLiveQueryWindowController is released on parameter changes and unmount.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/db/src/live-query-window-controller.ts (1)
343-364: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winA rejected window promise stays unhandled when observer activation throws.
activateLeaseat Line 348 can return a promise. Ifthis.observer.subscribeat Line 350 throws, control moves to the catch block, and no handler is ever attached to that promise. A later rejection fromsetWindowthen becomes an unhandled rejection.Attach a no-op rejection handler before rethrowing.
🛡️ Proposed fix
} catch (error) { + if (windowResult !== undefined && windowResult !== true) { + // The lease is being released; the caller has no channel for this error. + void windowResult.catch(() => {}) + } observerUnsub?.() this.observerUnsub = null this.deactivateLease()Declare
let windowResult: WindowResult | undefinedoutside thetryblock and assign it inside.🤖 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-window-controller.ts` around lines 343 - 364, Update the activation error path in the surrounding lease-subscription flow so any promise returned by activateLease is given a no-op rejection handler before the caught error is rethrown. Declare windowResult outside the try block, assign it from activateLease inside, and in catch attach the handler when the result is promise-like while preserving existing cleanup.
🧹 Nitpick comments (4)
packages/react-db/tests/useLiveInfiniteQuery.test.tsx (1)
139-139: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the render-flush sleep into a shared helper.
Both abandoned-render tests use the same inline
await new Promise((resolve) => setTimeout(resolve, 0)). Extract one helper, for exampleflushMicrotasks(), and call it from both tests.As per coding guidelines: "Extract common logic into utility functions when identical or near-identical code blocks appear in multiple places".
🤖 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/tests/useLiveInfiniteQuery.test.tsx` at line 139, Extract the duplicated zero-delay render-flush promise into a shared helper such as flushMicrotasks in the test module, then replace the inline sleeps in both abandoned-render tests with calls to that helper.Source: Coding guidelines
packages/db/tests/live-query-window-controller.test.ts (2)
329-349: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a larger
gcTimefor this test.
gcTime: 1allows garbage collection of the live-query collection one millisecond after the last subscriber releases it.controller.preload()releases the temporary lease in itsfinallyblock before the assertions run. Under a slow CI event loop, the collection can be cleaned up beforelq.utils.getWindow()is read, which makes the test flaky. Use a value that clearly exceeds test scheduling jitter.♻️ Proposed change
- gcTime: 1, + gcTime: 5000,🤖 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-window-controller.test.ts` around lines 329 - 349, Increase the gcTime configured in the createLiveQueryCollection call within the “establishes the desired window before preload” test to a value that comfortably exceeds normal test scheduling jitter, while preserving the existing preload and window assertions.
120-122: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffReplace the repeated
lq as anycasts with a typed helper.Every controller construction in this file casts the live-query collection with
as any. The coding guidelines require avoidingany. GivemakeOrderedLiveQueryan explicitCollection<Row, string, ...>return type, or introduce one small helper that performs the construction once, so the casts disappear from each test.As per coding guidelines: "Avoid using
anytypes; useunknowninstead when the type is truly unknown, and provide proper type annotations for return values".🤖 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-window-controller.test.ts` around lines 120 - 122, Remove the repeated lq as any casts from controller construction in live-query-window-controller.test.ts by giving makeOrderedLiveQuery an explicit Collection<Row, string, ...> return type or adding a typed construction helper. Update every createLiveQueryWindowController call to use the typed result without any while preserving the existing test behavior.Source: Coding guidelines
packages/db/src/live-query-window-controller.ts (1)
549-556: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the peek-row computation shared with
getSnapshot.
getComputedHasNextPagerepeats the logic at Lines 289-295: readisEnabled, verify the data is an array, then compare the row count againstcommittedPageCount * pageSize. Two copies of the peek-row rule can drift when the window arithmetic changes.Add one private helper that takes the observer snapshot and returns the computed value, and call it from both sites.
As per coding guidelines: "Extract common logic into utility functions when identical or near-identical code blocks appear in multiple places".
🤖 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-window-controller.ts` around lines 549 - 556, Extract the shared peek-row calculation into one private helper that accepts a LiveQuerySnapshot and returns whether the enabled array data exceeds committedPageCount * pageSize. Update both getSnapshot and getComputedHasNextPage to call this helper, removing their duplicated isEnabled, array, and row-count logic.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-window-controller.ts`:
- Around line 398-417: Update preload() to clear hasPaginationError and
paginationError at the start of each attempt, before activating the lease or
calling observer.preload(). After a successful preload that cleared a prior
error, call notify() so getSnapshot() reflects the recovered state; preserve the
existing catch, rethrow, and lease-cleanup behavior.
- Around line 503-512: The temporary lease is tracked per call, allowing
overlapping reset/fetch requests to release the shared lease prematurely. In the
request flow around releaseTemporaryLease, replace the boolean with an in-flight
temporary-lease holder count, increment when a request acquires the lease,
decrement in finally, and deactivate only when the count reaches zero and there
are no subscriptions. Add a regression test covering overlapping reset() and
fetchNextPage() calls.
In `@packages/db/src/query/live/collection-config-builder.ts`:
- Around line 278-288: The setWindow rollback in the currentWindow update flow
must restore the actual initial operator window, including when previousWindow
is undefined, then rerun maybeRunGraphFn after restoration to remove partial
results. Make recovery exception-safe by preserving and rethrowing the original
error even if windowFn or graph rerun fails during rollback.
In `@packages/react-db/tests/useLiveInfiniteQuery.test.tsx`:
- Around line 890-892: Strengthen the stale-callback test around fetchFromA by
awaiting the asynchronous work to settle before asserting the page count remains
unchanged. Then call result.current.fetchNextPage() explicitly, await its
completion, and assert the current controller’s page count increases as
expected, proving both stale-callback isolation and the new callback binding.
---
Outside diff comments:
In `@packages/db/src/live-query-window-controller.ts`:
- Around line 343-364: Update the activation error path in the surrounding
lease-subscription flow so any promise returned by activateLease is given a
no-op rejection handler before the caught error is rethrown. Declare
windowResult outside the try block, assign it from activateLease inside, and in
catch attach the handler when the result is promise-like while preserving
existing cleanup.
---
Nitpick comments:
In `@packages/db/src/live-query-window-controller.ts`:
- Around line 549-556: Extract the shared peek-row calculation into one private
helper that accepts a LiveQuerySnapshot and returns whether the enabled array
data exceeds committedPageCount * pageSize. Update both getSnapshot and
getComputedHasNextPage to call this helper, removing their duplicated isEnabled,
array, and row-count logic.
In `@packages/db/tests/live-query-window-controller.test.ts`:
- Around line 329-349: Increase the gcTime configured in the
createLiveQueryCollection call within the “establishes the desired window before
preload” test to a value that comfortably exceeds normal test scheduling jitter,
while preserving the existing preload and window assertions.
- Around line 120-122: Remove the repeated lq as any casts from controller
construction in live-query-window-controller.test.ts by giving
makeOrderedLiveQuery an explicit Collection<Row, string, ...> return type or
adding a typed construction helper. Update every createLiveQueryWindowController
call to use the typed result without any while preserving the existing test
behavior.
In `@packages/react-db/tests/useLiveInfiniteQuery.test.tsx`:
- Line 139: Extract the duplicated zero-delay render-flush promise into a shared
helper such as flushMicrotasks in the test module, then replace the inline
sleeps in both abandoned-render tests with calls to that helper.
🪄 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: ccfb4d0e-c05b-423a-9766-0983813c3122
📒 Files selected for processing (7)
.changeset/live-query-window-controller.mdpackages/db/src/index.tspackages/db/src/live-query-window-controller.tspackages/db/src/query/live/collection-config-builder.tspackages/db/tests/live-query-window-controller.test.tspackages/react-db/src/useLiveInfiniteQuery.tspackages/react-db/tests/useLiveInfiniteQuery.test.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/db/src/index.ts
- .changeset/live-query-window-controller.md
- packages/react-db/src/useLiveInfiniteQuery.ts
|
@KyleAMathews All eight reproduced blockers are fixed, the follow-up review pass is clean, and every check is green. Please re-review and merge when ready: #1675 |
|
I rechecked the follow-up changes on current head 1.
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/db/src/live-query-observer.ts (1)
412-422: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove
anyfrom the layout subscription cast.Line 413 introduces
Collection<T, TKey, any>. Use the default utility type instead.Proposed fix
- collection as Collection<T, TKey, any> & { + collection as Collection<T, TKey> & {As per coding guidelines,
**/*.{ts,tsx}: “Avoid usinganytypes; useunknowninstead when the type is truly unknown.”🤖 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` around lines 412 - 422, Update the layout subscription cast in subscribeLayoutChanges to use the Collection default utility type instead of any, preserving the optional _subscribeLayoutChanges signature and existing subscription behavior.Source: Coding guidelines
packages/db/tests/live-query-order-only-move.test.ts (1)
405-408: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated observer setup.
Lines 405-408 and 484-487 duplicate the observer creation, notification counter, subscription, and initial-notification reset. Extract a small test helper that accepts the collection and returns the observer with notification access. This keeps both tests consistent.
As per coding guidelines,
**/*.{ts,tsx,js}says: “Extract common logic into utility functions when identical or near-identical code blocks appear in multiple places.”Also applies to: 484-487
🤖 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-order-only-move.test.ts` around lines 405 - 408, Extract the duplicated observer setup from the tests around createLiveQueryObserver into a small helper that accepts a collection, subscribes to the observer, resets the initial notification count, and returns the observer plus notification access. Replace both setup blocks at the referenced test locations with this helper while preserving each test’s existing behavior.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.
Nitpick comments:
In `@packages/db/src/live-query-observer.ts`:
- Around line 412-422: Update the layout subscription cast in
subscribeLayoutChanges to use the Collection default utility type instead of
any, preserving the optional _subscribeLayoutChanges signature and existing
subscription behavior.
In `@packages/db/tests/live-query-order-only-move.test.ts`:
- Around line 405-408: Extract the duplicated observer setup from the tests
around createLiveQueryObserver into a small helper that accepts a collection,
subscribes to the observer, resets the initial notification count, and returns
the observer plus notification access. Replace both setup blocks at the
referenced test locations with this helper while preserving each test’s existing
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 59cf5c99-59f6-4872-8472-24339ece7ed7
📒 Files selected for processing (7)
packages/db/src/collection/changes.tspackages/db/src/collection/index.tspackages/db/src/collection/subscription.tspackages/db/src/live-query-observer.tspackages/db/tests/live-query-order-only-move.test.tspackages/react-db/src/useLiveInfiniteQuery.tspackages/react-db/tests/useLiveInfiniteQuery.test.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/react-db/tests/useLiveInfiniteQuery.test.tsx
- packages/react-db/src/useLiveInfiniteQuery.ts
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/tests/useLiveInfiniteQuery.test.tsx`:
- Around line 931-933: Strengthen the failed-window-update test around
fetchNextPage by capturing the loaded page’s row IDs before invoking
fetchNextPage, then asserting the post-failure page IDs match the captured IDs
in the same order. Keep the existing pages-length and hasNextPage assertions,
but replace the length-only preservation check with content comparison.
🪄 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: ecfb244f-17c1-443a-b543-877c04c5479e
📒 Files selected for processing (2)
packages/react-db/src/useLiveInfiniteQuery.tspackages/react-db/tests/useLiveInfiniteQuery.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/react-db/src/useLiveInfiniteQuery.ts
| expect(result.current.pages).toHaveLength(1) | ||
| expect(result.current.hasNextPage).toBe(true) | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the loaded page contents, not only its length.
The test claims that a failed window update preserves the loaded page. toHaveLength(1) also passes if the page becomes empty or contains different rows. Capture the page IDs before fetchNextPage() and compare them after the failure, including their order.
💚 Proposed test strengthening
await waitFor(() => {
expect(result.current.isReady).toBe(true)
expect(result.current.hasNextPage).toBe(true)
})
+ const pageIdsBefore = result.current.pages.map((page) =>
+ page.map((post) => post.id),
+ )
const failure = new Error(`window load failed`)
...
expect(result.current.pages).toHaveLength(1)
+ expect(
+ result.current.pages.map((page) => page.map((post) => post.id)),
+ ).toEqual(pageIdsBefore)
expect(result.current.hasNextPage).toBe(true)🤖 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/tests/useLiveInfiniteQuery.test.tsx` around lines 931 -
933, Strengthen the failed-window-update test around fetchNextPage by capturing
the loaded page’s row IDs before invoking fetchNextPage, then asserting the
post-failure page IDs match the captured IDs in the same order. Keep the
existing pages-length and hasNextPage assertions, but replace the length-only
preservation check with content comparison.
3 participants
This implements Phase 5 of the live-query platform RFC (#1623): an internal, framework-neutral window controller now owns infinite-query pagination. React's
useLiveInfiniteQueryis a thin binding over that controller, with no public API change.Users keep the same forward-pagination behavior, while concurrent consumers, failed subset loads, cleanup/restart cycles, runtime page-size changes, and order-only updates now produce coherent results.
Root cause
The React hook previously owned page state and mutated a live-query collection's physical window directly. That coupled pagination to React and left no shared owner when several consumers used the same collection.
The first extraction exposed several distinctions that the old hook did not need to model explicitly:
Without those distinctions, overlapping operations could shrink a pending page request, failures could leave an inflated shared lease, observer updates could disappear while a load was pending, and a stale window could return after restart.
Approach
createLiveQueryWindowControllerto@tanstack/db. It owns page slicing, peek-aheadhasNextPage, loading/error state, reset, preload, subscription, and disposal.setWindowreturn the real subset-load promise so page commits and errors follow the underlying load outcome.useLiveInfiniteQueryarounduseSyncExternalStore, preserving loaded pages across page-shape changes and structurally equal dependency values.Key invariants
statusand its boolean flags always agree.Non-goals
getNextPageParambehavior; it remains accepted but unused.Trade-offs
The coordinator adds generation and lease bookkeeping, but keeps arbitration in one collection-scoped place instead of spreading race handling across framework hooks. React still lets
useSyncExternalStoreunsubscribe replaced controllers rather than disposing them during render, which avoids breaking abandoned or concurrent renders.Verification
Focused results:
@tanstack/db: 47 tests passed@tanstack/react-db: 42 tests passedFiles changed
packages/db/src/live-query-window-controller.ts: shared controller, lease coordinator, transactional commits, and snapshot state.packages/db/src/query/live/collection-config-builder.ts: physical-window inspection, real subset-load completion, and restart cleanup.packages/db/src/collection/*andpackages/db/src/query/live/collection-subscriber.ts: layout publication and rejection-safe load tracking.packages/db/tests/live-query-window-controller.test.tsandlive-query-order-only-move.test.ts: controller races, failures, lifecycle, and public layout regressions.packages/react-db/src/useLiveInfiniteQuery.ts: thin React binding with stable pagination across compatible rerenders.packages/react-db/tests/useLiveInfiniteQuery.test.tsx: hook lifecycle, dependency, error, warning, and pagination coverage..changeset/live-query-window-controller.md: patch release note for@tanstack/dband@tanstack/react-db.Related work