diff --git a/.changeset/live-query-observer.md b/.changeset/live-query-observer.md index 95565b091..62e747700 100644 --- a/.changeset/live-query-observer.md +++ b/.changeset/live-query-observer.md @@ -9,8 +9,8 @@ Add an internal shared live-query observer and migrate all five framework adapters to it -Introduces `createLiveQueryObserver` in `@tanstack/db`: given a resolved live-query collection (or `null` for a disabled query) it owns the lifecycle every adapter used to re-implement — sync activation on first subscribe, change and status subscriptions, a snapshot with stable identity per state revision for wholesale consumers, and delivery of the raw `ChangeMessage[]` for granular consumers. React, Vue, Svelte, Solid, and Angular's live-query hooks now materialize from the observer instead of their own hand-rolled subscription/status/snapshot machinery, keeping each adapter's native reactivity and each adapter's data-loading policy (wholesale adapters subscribe without initial state; granular adapters seed from it). +Introduces `createLiveQueryObserver` in `@tanstack/db`: given a resolved live-query collection (or `null` for a disabled query) it owns the subscription lifecycle every adapter used to re-implement — change and status subscriptions, a snapshot with stable identity per state revision for wholesale consumers, and delivery of the raw `ChangeMessage[]` for granular consumers. React, Vue, Svelte, Solid, and Angular's live-query hooks now materialize from the observer instead of their own hand-rolled subscription/status/snapshot machinery, keeping each adapter's native reactivity and each adapter's data-loading policy (wholesale adapters subscribe without initial state; granular adapters seed from it). The observer is an **internal, unstable contract** for TanStack DB's official adapters — it is exported so the adapter packages can consume it, but it is not a public extension point yet and its API may change in any release. -The migration also fixes several live-query lifecycle defects: status-only transitions (`error`, `cleaned-up`) now reach mounted consumers; snapshot identity is stable across unsubscribe/resubscribe and stays fresh while detached; dispatch is FIFO and non-reentrant with subscriptions identified by record rather than callback; disposing during the synchronous initial replay no longer leaks the collection subscription; subscribing after dispose throws instead of registering a dead listener; Solid guards its async resource continuations against superseded collections; and constructing an observer no longer activates sync (activation belongs to the first committed subscription). +The migration also fixes several live-query lifecycle defects: status-only transitions (`error`, `cleaned-up`) now reach mounted consumers; snapshot identity is stable across unsubscribe/resubscribe and stays fresh while detached; dispatch is FIFO and non-reentrant with subscriptions identified by record rather than callback; disposing during the synchronous initial replay no longer leaks the collection subscription; subscribing after dispose throws instead of registering a dead listener; Solid guards its async resource continuations against superseded collections; and constructing an observer no longer activates sync. Observers activate on their first committed subscription unless an adapter has already started a pre-created collection supplied directly or returned from a callback. diff --git a/.changeset/live-query-order-only-move.md b/.changeset/live-query-order-only-move.md new file mode 100644 index 000000000..b391621da --- /dev/null +++ b/.changeset/live-query-order-only-move.md @@ -0,0 +1,13 @@ +--- +'@tanstack/db': patch +--- + +fix(db): republish ordered live queries on an order-only move + +An `orderBy` live query that reordered its rows without changing any projected +row value (an "order-only move") previously emitted nothing, so `useLiveQuery` +kept rendering the stale order. The live-query collection now publishes an +explicit layout-change notification when this happens, and the shared live-query +observer snapshot exposes a `layoutRevision` that increments on any visible +membership, ordering, or order-only-move change. All five framework adapters +pick this up via their existing wholesale re-read. diff --git a/packages/db/src/collection/change-events.ts b/packages/db/src/collection/change-events.ts index 3f4977b7b..e70e44903 100644 --- a/packages/db/src/collection/change-events.ts +++ b/packages/db/src/collection/change-events.ts @@ -253,7 +253,7 @@ export function createFilteredCallback< >( originalCallback: (changes: Array>) => void, options: SubscribeChangesOptions, -): (changes: Array>) => void { +): (changes: Array>) => boolean { const filterFn = createFilterFunctionFromExpression(options.whereExpression!) return (changes: Array>) => { @@ -303,7 +303,9 @@ export function createFilteredCallback< // if the original changes array was empty (which indicates a ready signal) if (filteredChanges.length > 0 || changes.length === 0) { originalCallback(filteredChanges) + return true } + return false } } diff --git a/packages/db/src/collection/changes.ts b/packages/db/src/collection/changes.ts index e5bdfb845..87d092f9b 100644 --- a/packages/db/src/collection/changes.ts +++ b/packages/db/src/collection/changes.ts @@ -39,6 +39,13 @@ export class CollectionChangesManager< */ public stateRevision = 0 + /** + * Monotonic revision advanced only for explicit layout-only publications. + * This distinguishes them from the legacy empty ready event, since both use + * an empty change batch at the public subscription boundary. + */ + public layoutRevision = 0 + /** * Creates a new CollectionChangesManager instance */ @@ -85,10 +92,12 @@ export class CollectionChangesManager< public emitEvents( changes: Array>, forceEmit = false, + layoutChanged = false, ): void { // The visible state was already committed by the caller, so the revision // advances even when the events below end up batched for later emission. if (changes.length > 0) this.stateRevision++ + if (layoutChanged) this.layoutRevision++ // Skip batching for user actions (forceEmit=true) to keep UI responsive if (this.shouldBatchEvents && !forceEmit) { @@ -111,7 +120,7 @@ export class CollectionChangesManager< this.shouldBatchEvents = false } - if (rawEvents.length === 0) { + if (rawEvents.length === 0 && !layoutChanged) { return } @@ -123,7 +132,7 @@ export class CollectionChangesManager< // Emit to all listeners for (const subscription of this.changeSubscriptions) { - subscription.emitEvents(enrichedEvents) + subscription.emitEvents(enrichedEvents, layoutChanged) } } diff --git a/packages/db/src/collection/index.ts b/packages/db/src/collection/index.ts index 13887a43d..c775e7ada 100644 --- a/packages/db/src/collection/index.ts +++ b/packages/db/src/collection/index.ts @@ -429,6 +429,19 @@ export class CollectionImpl< return this._changes.stateRevision } + /** + * Monotonic revision of explicit layout-only publications. + * Internal — used to distinguish them from empty ready events. + */ + public get _layoutRevision(): number { + return this._changes.layoutRevision + } + + /** Mark the active sync transaction as layout-changing. Internal. */ + public _markLayoutChange(): void { + this._sync.markLayoutChange() + } + /** * Register a callback to be executed when the collection first becomes ready * Useful for preloading collections diff --git a/packages/db/src/collection/state.ts b/packages/db/src/collection/state.ts index 0f7b3b868..1eeb92b2d 100644 --- a/packages/db/src/collection/state.ts +++ b/packages/db/src/collection/state.ts @@ -25,6 +25,7 @@ interface PendingSyncedTransaction< TKey extends string | number = string | number, > { committed: boolean + layoutChanged: boolean operations: Array> truncate?: boolean deletedKeys: Set @@ -836,10 +837,12 @@ export class CollectionStateManager< uncommittedSyncedTransactions, hasTruncateSync, hasImmediateSync, + layoutChanged, } = this.pendingSyncedTransactions.reduce( (acc, t) => { if (t.committed) { acc.committedSyncedTransactions.push(t) + acc.layoutChanged ||= t.layoutChanged if (t.truncate) { acc.hasTruncateSync = true } @@ -860,6 +863,7 @@ export class CollectionStateManager< >, hasTruncateSync: false, hasImmediateSync: false, + layoutChanged: false, }, ) @@ -1331,7 +1335,7 @@ export class CollectionStateManager< } // End batching and emit all events (combines any batched events with sync events) - this.changes.emitEvents(events, true) + this.changes.emitEvents(events, true, layoutChanged) this.pendingSyncedTransactions = uncommittedSyncedTransactions diff --git a/packages/db/src/collection/subscription.ts b/packages/db/src/collection/subscription.ts index 2d48add4b..0f603ee71 100644 --- a/packages/db/src/collection/subscription.ts +++ b/packages/db/src/collection/subscription.ts @@ -83,7 +83,7 @@ export class CollectionSubscription // Track the last key sent via requestLimitedSnapshot for cursor-based pagination private lastSentKey: string | number | undefined - private filteredCallback: (changes: Array>) => void + private filteredCallback: (changes: Array>) => boolean private orderByIndex: IndexInterface | undefined @@ -99,6 +99,7 @@ export class CollectionSubscription // This prevents a flash of missing content between deletes and new inserts private isBufferingForTruncate = false private truncateBuffer: Array>> = [] + private truncateBufferHasLayoutChange = false private pendingTruncateRefetches: Set> = new Set() public get status(): SubscriptionStatus { @@ -132,7 +133,10 @@ export class CollectionSubscription // Create a filtered callback if where clause is provided this.filteredCallback = options.whereExpression ? createFilteredCallback(this.callback, options) - : this.callback + : (changes) => { + this.callback(changes) + return true + } // Listen for truncate events to re-request data after must-refetch // When a truncate happens (e.g., from a 409 must-refetch), all collection data is cleared. @@ -245,11 +249,14 @@ export class CollectionSubscription // Flatten all buffered changes into a single array for atomic emission // This ensures consumers see all truncate changes (deletes + inserts) in one callback const merged = this.truncateBuffer.flat() - if (merged.length > 0) { - this.filteredCallback(merged) + const layoutChanged = this.truncateBufferHasLayoutChange + if (merged.length > 0 || layoutChanged) { + const delivered = this.filteredCallback(merged) + if (layoutChanged && !delivered) this.filteredCallback([]) } this.truncateBuffer = [] + this.truncateBufferHasLayoutChange = false } setOrderByIndex(index: IndexInterface) { @@ -318,7 +325,10 @@ export class CollectionSubscription return this.snapshotSent } - emitEvents(changes: Array>) { + emitEvents( + changes: Array>, + layoutChanged = false, + ): boolean { const newChanges = this.filterAndFlipChanges(changes) if (this.isBufferingForTruncate) { @@ -327,8 +337,12 @@ export class CollectionSubscription if (newChanges.length > 0) { this.truncateBuffer.push(newChanges) } + if (layoutChanged) this.truncateBufferHasLayoutChange = true + return false } else { - this.filteredCallback(newChanges) + const delivered = this.filteredCallback(newChanges) + if (layoutChanged && !delivered) return this.filteredCallback([]) + return delivered } } @@ -728,6 +742,7 @@ export class CollectionSubscription // Clean up truncate buffer state this.isBufferingForTruncate = false this.truncateBuffer = [] + this.truncateBufferHasLayoutChange = false this.pendingTruncateRefetches.clear() // Unload all subsets that this subscription loaded diff --git a/packages/db/src/collection/sync.ts b/packages/db/src/collection/sync.ts index af89ed2cf..8fa49b163 100644 --- a/packages/db/src/collection/sync.ts +++ b/packages/db/src/collection/sync.ts @@ -71,6 +71,11 @@ export class CollectionSyncManager< this._events = deps.events } + /** Mark the active sync transaction as changing collection layout. */ + public markLayoutChange(): void { + this.getActivePendingSyncTransaction().layoutChanged = true + } + /** * Start the sync process for this collection * This is called when the collection is first accessed or preloaded @@ -92,6 +97,7 @@ export class CollectionSyncManager< begin: (options?: { immediate?: boolean }) => { this.state.pendingSyncedTransactions.push({ committed: false, + layoutChanged: false, operations: [], deletedKeys: new Set(), rowMetadataWrites: new Map(), diff --git a/packages/db/src/live-query-observer.ts b/packages/db/src/live-query-observer.ts index 40028fefb..3bb35c818 100644 --- a/packages/db/src/live-query-observer.ts +++ b/packages/db/src/live-query-observer.ts @@ -24,6 +24,17 @@ export interface LiveQuerySnapshot< data: T | ReadonlyArray | undefined /** The underlying collection, or `undefined` when disabled. */ collection: Collection | undefined + /** + * Monotonic counter bumped whenever the visible layout (the ordered key + * sequence) changes — membership, ordering, or an order-only move. Lets + * consumers detect a reorder that changed no row value (which `data`/`state` + * identity alone can't express once row values are structurally shared). + * + * It is NOT in lockstep with snapshot identity: a value-only update produces a + * new snapshot while `layoutRevision` stays put. A `layoutRevision` change + * always accompanies a new snapshot, but not vice versa. + */ + layoutRevision: number status: CollectionStatus | `disabled` isLoading: boolean isReady: boolean @@ -88,12 +99,15 @@ interface Publication { entries?: Array<[TKey, T]> status: CollectionStatus collectionRevision?: number + collectionLayoutRevision?: number + layoutChanged: boolean } const DISABLED_SNAPSHOT: LiveQuerySnapshot = { state: undefined, data: undefined, collection: undefined, + layoutRevision: 0, status: `disabled`, isLoading: false, isReady: true, @@ -112,8 +126,12 @@ class LiveQueryObserverImpl< private visibleStatus: CollectionStatus | undefined private cachedEntries: Array<[TKey, T]> | undefined private cachedCollectionRevision: number | undefined + private cachedCollectionLayoutRevision: number | undefined private snapshotDirty = true private cachedSnapshot: LiveQuerySnapshot = DISABLED_SNAPSHOT + private layoutRevision = 0 + private lastLayoutKeys: Array | undefined + private deliveredLayoutRevision: number | undefined private readonly subscriptions = new Set>() // Publications are dispatched FIFO: an emit that happens while another // publication is being delivered (a listener mutating the collection @@ -147,10 +165,34 @@ class LiveQueryObserverImpl< const singleResult = isSingleResultCollection(collection) const status = this.visibleStatus ?? collection.status + // Bump the layout revision when the ordered key sequence changes + // (membership, ordering, or an order-only move). Compare the key sequence + // directly rather than via a serialized signature: a joined-with-separator + // signature can collide when a key value equals the concatenation of + // neighboring keys around the separator. Comparing keys also avoids + // materializing a large string on every rebuild; a new key array is only + // allocated when the layout actually moved. + const prevKeys = this.lastLayoutKeys + let layoutChanged = + prevKeys === undefined || prevKeys.length !== entries.length + if (!layoutChanged) { + for (let i = 0; i < entries.length; i++) { + if (prevKeys![i] !== entries[i]![0]) { + layoutChanged = true + break + } + } + } + if (layoutChanged) { + this.lastLayoutKeys = entries.map(([key]) => key) + this.layoutRevision++ + } + this.cachedSnapshot = { state, data: singleResult ? data[0] : data, collection, + layoutRevision: this.layoutRevision, status, ...getLiveQueryStatusFlags(status), isEnabled: true, @@ -167,6 +209,14 @@ class LiveQueryObserverImpl< return typeof revision === `number` ? revision : undefined } + private getCollectionLayoutRevision( + collection: Collection, + ): number | undefined { + const revision = (collection as { _layoutRevision?: unknown }) + ._layoutRevision + return typeof revision === `number` ? revision : undefined + } + private readEntries(collection: Collection): { entries: Array<[TKey, T]> revision?: number @@ -219,13 +269,16 @@ class LiveQueryObserverImpl< private refreshDetachedState(collection: Collection): void { const status = collection.status const revision = this.getCollectionRevision(collection) + const layoutRevision = this.getCollectionLayoutRevision(collection) if (revision !== undefined) { if ( this.cachedEntries === undefined || - revision !== this.cachedCollectionRevision + revision !== this.cachedCollectionRevision || + layoutRevision !== this.cachedCollectionLayoutRevision ) { this.captureEntries(collection) + this.cachedCollectionLayoutRevision = layoutRevision this.snapshotDirty = true } } else { @@ -282,8 +335,10 @@ class LiveQueryObserverImpl< private attach(): void { const collection = this.collection if (!collection || this.disposed) return + this.refreshDetachedState(collection) this.attached = true this.visibleStatus ??= collection.status + this.deliveredLayoutRevision = this.getCollectionLayoutRevision(collection) this.blockDelivery = this.wholesale // Sync activation happens inside subscribeChanges (addSubscriber starts @@ -304,10 +359,23 @@ class LiveQueryObserverImpl< status: CollectionStatus = collection.status, ) => { if (this.disposed || this.subscriptions.size === 0) return - // An empty batch carries no semantic change (e.g. the collection's - // empty-ready flush); only real deltas and the synthetic ready notify - // (undefined) are published. - if (changes !== undefined && changes.length === 0) return + const layoutRevision = this.getCollectionLayoutRevision(collection) + let layoutChanged = false + if (changes !== undefined && changes.length === 0) { + // Empty ready events predate the explicit layout signal and share its + // empty-array payload. Only forward an empty batch when the collection + // confirms that a new layout-only publication occurred. + if ( + layoutRevision === undefined || + layoutRevision === this.deliveredLayoutRevision + ) { + return + } + layoutChanged = true + } + if (changes !== undefined && layoutRevision !== undefined) { + this.deliveredLayoutRevision = layoutRevision + } const captured = changes !== undefined ? this.readEntries(collection) @@ -320,6 +388,8 @@ class LiveQueryObserverImpl< captured?.entries, status, captured?.revision, + layoutRevision, + layoutChanged, ) } @@ -377,6 +447,8 @@ class LiveQueryObserverImpl< entries?: Array<[TKey, T]>, status = this.collection?.status ?? `cleaned-up`, collectionRevision?: number, + collectionLayoutRevision?: number, + layoutChanged = false, ): void { this.publicationQueue.push({ changes, @@ -384,6 +456,8 @@ class LiveQueryObserverImpl< entries, status, collectionRevision, + collectionLayoutRevision, + layoutChanged, }) if (this.dispatching || this.blockDelivery) return @@ -404,6 +478,13 @@ class LiveQueryObserverImpl< publication.collectionRevision, ) } + if (publication.collectionLayoutRevision !== undefined) { + this.cachedCollectionLayoutRevision = + publication.collectionLayoutRevision + } + if (publication.layoutChanged) { + this.snapshotDirty = true + } if (this.visibleStatus !== publication.status) { this.visibleStatus = publication.status this.snapshotDirty = true diff --git a/packages/db/src/query/live/collection-config-builder.ts b/packages/db/src/query/live/collection-config-builder.ts index 4d80b3fe1..e324119c6 100644 --- a/packages/db/src/query/live/collection-config-builder.ts +++ b/packages/db/src/query/live/collection-config-builder.ts @@ -11,6 +11,7 @@ import { } from '../../errors.js' import { transactionScopedScheduler } from '../../scheduler.js' import { getActiveTransaction } from '../../transactions.js' +import { deepEquals } from '../../utils.js' import { CollectionSubscriber } from './collection-subscriber.js' import { getCollectionBuilder } from './collection-registry.js' import { LIVE_QUERY_INTERNAL } from './internal.js' @@ -799,6 +800,11 @@ export class CollectionConfigBuilder< existing.orderByIndex = changes.orderByIndex } } + // Keep the retracted (old) side for order-only-move detection. + if (changes.deletes > 0) { + existing.previousValue = changes.previousValue + existing.previousOrderByIndex = changes.previousOrderByIndex + } } else { merged.set(customKey, { ...changes }) } @@ -810,6 +816,9 @@ export class CollectionConfigBuilder< if (hasParentChanges) { begin() changesToApply.forEach(this.applyChanges.bind(this, config)) + if (hasOrderOnlyMove(changesToApply)) { + markLayoutChange(config.collection) + } commit() } pendingChanges = new Map() @@ -893,9 +902,14 @@ export class CollectionConfigBuilder< if (multiplicity < 0) { existing.deletes += Math.abs(multiplicity) + existing.previousValue = childResult + existing.previousOrderByIndex = _orderByIndex } else if (multiplicity > 0) { existing.inserts += multiplicity existing.value = childResult + if (_orderByIndex !== undefined) { + existing.orderByIndex = _orderByIndex + } } byChild.set(childKey, existing) @@ -1320,9 +1334,14 @@ function setupNestedPipelines( if (multiplicity < 0) { existing.deletes += Math.abs(multiplicity) + existing.previousValue = childResult + existing.previousOrderByIndex = _orderByIndex } else if (multiplicity > 0) { existing.inserts += multiplicity existing.value = childResult + if (_orderByIndex !== undefined) { + existing.orderByIndex = _orderByIndex + } } byChild.set(childKey, existing) @@ -1984,6 +2003,9 @@ function flushIncludesState( entry.syncMethods.write({ value: change.value, type: `delete` }) } } + if (hasOrderOnlyMove(childChanges)) { + markLayoutChange(entry.syncMethods.collection) + } entry.syncMethods.commit() } @@ -2315,6 +2337,10 @@ function accumulateChanges( } if (multiplicity < 0) { changes.deletes += Math.abs(multiplicity) + // Remember the retracted (old) value + position so the flush can tell an + // order-only move apart from a real value change. + changes.previousValue = value + changes.previousOrderByIndex = orderByIndex } else if (multiplicity > 0) { changes.inserts += multiplicity // Update value to the latest version for this key @@ -2326,3 +2352,33 @@ function accumulateChanges( acc.set(key, changes) return acc } + +/** + * Decide whether a flush contains an order-only move. + * + * An "order-only move" — a row updated in place whose `orderByIndex` moved but + * whose projected value is deep-equal to before — is swallowed by the value-diff + * and needs an explicit layout notification. The collection coalesces that + * signal with any ordinary row publication per subscriber. + */ +function hasOrderOnlyMove( + changesToApply: Map>, +): boolean { + for (const changes of changesToApply.values()) { + const isUpdate = changes.inserts > 0 && changes.deletes > 0 + if ( + isUpdate && + changes.previousValue !== undefined && + deepEquals(changes.previousValue, changes.value) && + changes.orderByIndex !== changes.previousOrderByIndex + ) { + return true + } + } + return false +} + +/** Mark the collection's next commit as layout-changing. */ +function markLayoutChange(collection: { _markLayoutChange: () => void }): void { + collection._markLayoutChange() +} diff --git a/packages/db/src/query/live/types.ts b/packages/db/src/query/live/types.ts index 118015bd6..307397698 100644 --- a/packages/db/src/query/live/types.ts +++ b/packages/db/src/query/live/types.ts @@ -16,6 +16,12 @@ export type Changes = { inserts: number value: T orderByIndex: string | undefined + // Captured from the retract side of a change so the flush can detect an + // "order-only move": a row whose projected value is unchanged but whose + // `orderByIndex` moved. Such a move is swallowed by the collection's + // value-diff, so it needs an explicit layout notification. + previousValue?: T + previousOrderByIndex?: string | undefined } export type SyncState = { diff --git a/packages/db/tests/conformance/suite.ts b/packages/db/tests/conformance/suite.ts index 1a2d21c70..39759017b 100644 --- a/packages/db/tests/conformance/suite.ts +++ b/packages/db/tests/conformance/suite.ts @@ -39,7 +39,7 @@ const ISSUES: Array = [ ] /** Keys that are expected to fail on ALL adapters (core gaps, not adapter drift). */ -const UNIVERSAL_EXPECTED_FAIL = new Set([`order-only-move`]) +const UNIVERSAL_EXPECTED_FAIL = new Set([]) export function runSuite(rawDriver: LiveQueryDriver) { const { ops } = rawDriver diff --git a/packages/db/tests/live-query-observer.test.ts b/packages/db/tests/live-query-observer.test.ts index d73682c2c..bb0ac8ff5 100644 --- a/packages/db/tests/live-query-observer.test.ts +++ b/packages/db/tests/live-query-observer.test.ts @@ -590,6 +590,32 @@ describe(`createLiveQueryObserver`, () => { observer.dispose() }) + it(`bumps layoutRevision on a membership change that a joined key signature would collide on`, () => { + // Two keys "a","b" vs a single key "a\u0000b" join to the same string under + // any separator that can appear in a key. The revision compares the key + // sequence directly, so it must still register the membership change. + const source = makeSource([ + { id: `a`, name: `A` }, + { id: `b`, name: `B` }, + ]) + const observer = createLiveQueryObserver(source as any) + observer.subscribe(() => {}) + + const revBefore = observer.getSnapshot().layoutRevision + + source.utils.begin() + source.utils.write({ type: `delete`, value: { id: `a`, name: `A` } }) + source.utils.write({ type: `delete`, value: { id: `b`, name: `B` } }) + source.utils.write({ + type: `insert`, + value: { id: `a\u0000b`, name: `AB` }, + }) + source.utils.commit() + + expect(observer.getSnapshot().layoutRevision).not.toBe(revBefore) + observer.dispose() + }) + it(`keeps an unread snapshot pinned to the revision when it was created`, () => { const source = makeSource() const observer = createLiveQueryObserver(source as any, { diff --git a/packages/db/tests/live-query-order-only-move.test.ts b/packages/db/tests/live-query-order-only-move.test.ts new file mode 100644 index 000000000..2637bf8a3 --- /dev/null +++ b/packages/db/tests/live-query-order-only-move.test.ts @@ -0,0 +1,485 @@ +import { describe, expect, it } from 'vitest' +import { createCollection } from '../src/collection/index.js' +import { createDeferred } from '../src/deferred.js' +import { createLiveQueryCollection } from '../src/query/live-query-collection.js' +import { createLiveQueryObserver } from '../src/live-query-observer.js' +import { eq } from '../src/query/builder/functions.js' +import { mockSyncCollectionOptions } from './utils.js' + +interface Person { + id: string + name: string + age: number +} + +const SEED: Array = [ + { id: `1`, name: `Alice`, age: 30 }, + { id: `2`, name: `Bob`, age: 20 }, + { id: `3`, name: `Carol`, age: 40 }, +] + +let seq = 0 +function makeSource(data: Array = SEED) { + return createCollection( + mockSyncCollectionOptions({ + id: `order-only-move-${seq++}`, + getKey: (p) => p.id, + initialData: data, + }), + ) +} + +/** Live query ordered by `age` (NOT projected), selecting only `{ id, name }`. */ +async function makeOrderedByAge(source: ReturnType) { + const lq = createLiveQueryCollection((q) => + q + .from({ p: source }) + .orderBy(({ p }) => p.age, `asc`) + .select(({ p }) => ({ id: p.id, name: p.name })), + ) + await lq.preload() + return lq +} + +const flush = () => new Promise((r) => setTimeout(r, 0)) + +describe(`order-only move (RFC #1623 phase 4)`, () => { + it(`republishes the ordered result when a row moves but its value is unchanged`, async () => { + const source = makeSource() + const lq = await makeOrderedByAge(source) + const observer = createLiveQueryObserver< + { id: string; name: string }, + string + >(lq as any) + + let notifications = 0 + observer.subscribe(() => { + notifications++ + }) + + const before = observer.getSnapshot() + expect((before.data as Array).map((r) => r.id)).toEqual([ + `2`, + `1`, + `3`, + ]) + const revBefore = before.layoutRevision + + // Move Bob (age 20 -> 99) to the end. The projected `{ id, name }` is + // identical, so the collection's value-diff emits no row change — only the + // layout notification should republish the new order. + source.utils.begin() + source.utils.write({ + type: `update`, + value: { id: `2`, name: `Bob`, age: 99 }, + }) + source.utils.commit() + await flush() + + const after = observer.getSnapshot() + expect((after.data as Array).map((r) => r.id)).toEqual([`1`, `3`, `2`]) + expect(after.layoutRevision).toBeGreaterThan(revBefore) + expect(notifications).toBeGreaterThan(0) + observer.dispose() + }) + + it(`refreshes a detached observer after an order-only move`, async () => { + const source = makeSource() + const lq = await makeOrderedByAge(source) + const observer = createLiveQueryObserver< + { id: string; name: string }, + string + >(lq as any) + + const before = observer.getSnapshot() + expect((before.data as Array).map((row) => row.id)).toEqual([ + `2`, + `1`, + `3`, + ]) + + source.utils.begin() + source.utils.write({ + type: `update`, + value: { id: `2`, name: `Bob`, age: 99 }, + }) + source.utils.commit() + await flush() + + const after = observer.getSnapshot() + expect(after).not.toBe(before) + expect((after.data as Array).map((row) => row.id)).toEqual([ + `1`, + `3`, + `2`, + ]) + expect(after.layoutRevision).toBeGreaterThan(before.layoutRevision) + observer.dispose() + }) + + it(`refreshes a detached observer when an order-only sync is parked`, async () => { + const source = makeSource() + const persist = createDeferred() + const lq = createLiveQueryCollection({ + getKey: (row) => row.id, + query: (q) => + q + .from({ p: source }) + .orderBy(({ p }) => p.age, `asc`) + .select(({ p }) => ({ id: p.id, name: p.name })), + onUpdate: () => persist.promise, + }) + await lq.preload() + const observer = createLiveQueryObserver< + { id: string; name: string }, + string + >(lq as any) + + const before = observer.getSnapshot() + const collectionLayoutRevisionBefore = lq._layoutRevision + expect((before.data as Array).map((row) => row.id)).toEqual([ + `2`, + `1`, + `3`, + ]) + + const mutation = lq.update( + `1`, + { optimistic: false }, + (draft) => void (draft.name = `Pending`), + ) + expect(mutation.state).toBe(`persisting`) + + source.utils.begin() + source.utils.write({ + type: `update`, + value: { id: `2`, name: `Bob`, age: 99 }, + }) + source.utils.commit() + await flush() + + const parked = observer.getSnapshot() + expect((parked.data as Array).map((row) => row.id)).toEqual([ + `2`, + `1`, + `3`, + ]) + expect(lq._layoutRevision).toBe(collectionLayoutRevisionBefore) + + persist.resolve() + await mutation.isPersisted.promise + await flush() + + const after = observer.getSnapshot() + expect((after.data as Array).map((row) => row.id)).toEqual([ + `1`, + `3`, + `2`, + ]) + expect(lq._layoutRevision).toBeGreaterThan(collectionLayoutRevisionBefore) + observer.dispose() + }) + + it(`does not bump the layout revision when nothing about the layout changes`, async () => { + const source = makeSource() + const lq = await makeOrderedByAge(source) + const observer = createLiveQueryObserver< + { id: string; name: string }, + string + >(lq as any) + let notifications = 0 + observer.subscribe(() => notifications++) + notifications = 0 + + const revBefore = observer.getSnapshot().layoutRevision + + // Update a row's `age` in a way that keeps its sort position (20 -> 21, + // still the youngest) and does not change the projected value. + source.utils.begin() + source.utils.write({ + type: `update`, + value: { id: `2`, name: `Bob`, age: 21 }, + }) + source.utils.commit() + await flush() + + // Order is unchanged (`2` still first), so the layout revision is stable. + const after = observer.getSnapshot() + expect((after.data as Array).map((r) => r.id)).toEqual([`2`, `1`, `3`]) + expect(after.layoutRevision).toBe(revBefore) + expect(notifications).toBe(0) + observer.dispose() + }) + + it(`does not publish when multiple moves cancel within one transaction`, async () => { + const source = makeSource() + const lq = await makeOrderedByAge(source) + const observer = createLiveQueryObserver< + { id: string; name: string }, + string + >(lq as any) + let notifications = 0 + observer.subscribe(() => notifications++) + notifications = 0 + + const before = observer.getSnapshot() + source.utils.begin() + source.utils.write({ + type: `update`, + value: { id: `2`, name: `Bob`, age: 99 }, + }) + source.utils.write({ + type: `update`, + value: { id: `2`, name: `Bob`, age: 20 }, + }) + source.utils.commit() + await flush() + + expect(observer.getSnapshot()).toBe(before) + expect(notifications).toBe(0) + observer.dispose() + }) + + it(`bumps the layout revision on membership changes too`, async () => { + const source = makeSource() + const lq = await makeOrderedByAge(source) + const observer = createLiveQueryObserver< + { id: string; name: string }, + string + >(lq as any) + observer.subscribe(() => {}) + + const revBefore = observer.getSnapshot().layoutRevision + + source.utils.begin() + source.utils.write({ + type: `insert`, + value: { id: `4`, name: `Dan`, age: 10 }, + }) + source.utils.commit() + await flush() + + const after = observer.getSnapshot() + expect((after.data as Array).map((r) => r.id)).toEqual([ + `4`, + `2`, + `1`, + `3`, + ]) + expect(after.layoutRevision).toBeGreaterThan(revBefore) + observer.dispose() + }) + + // Kyle's review issue 1: a commit containing both an ordinary value update + // and an order-only move must publish exactly once — the ordinary publication + // already carries the final values and ordering, so the separate layout event + // is redundant. + it(`publishes a mixed value update and order-only move exactly once`, async () => { + const source = makeSource() + const lq = await makeOrderedByAge(source) + const observer = createLiveQueryObserver< + { id: string; name: string }, + string + >(lq as any) + + let notifications = 0 + observer.subscribe(() => notifications++) + notifications = 0 // exclude subscribeChanges' initial-state publication + + source.utils.begin() + source.utils.write({ + type: `update`, + value: { id: `1`, name: `Alicia`, age: 30 }, + }) + source.utils.write({ + type: `update`, + value: { id: `2`, name: `Bob`, age: 99 }, + }) + source.utils.commit() + + const after = observer.getSnapshot() + expect( + (after.data as Array).map(({ id, name }) => [id, name]), + ).toEqual([ + [`1`, `Alicia`], + [`3`, `Carol`], + [`2`, `Bob`], + ]) + expect(notifications).toBe(1) + + // The layout clock from this mixed publication must be consumed even + // though it arrived with row changes. A later legacy empty-ready event is + // not a second layout publication. + ;(lq as any)._changes.emitEmptyReadyEvent() + expect(notifications).toBe(1) + observer.dispose() + }) + + it(`publishes a mixed batch to a subscriber that filters out the row update`, async () => { + const source = makeSource() + const lq = await makeOrderedByAge(source) + const publications: Array> = [] + const subscription = lq.subscribeChanges( + (changes) => publications.push(changes), + { + includeInitialState: false, + where: (row) => eq(row.name, `Bob`), + }, + ) + + source.utils.begin() + source.utils.write({ + type: `update`, + value: { id: `1`, name: `Alicia`, age: 30 }, + }) + source.utils.write({ + type: `update`, + value: { id: `2`, name: `Bob`, age: 99 }, + }) + source.utils.commit() + + expect(publications).toEqual([[]]) + subscription.unsubscribe() + }) + + // Kyle's review issue 2: an ordered child collection produced by `includes` + // must consume the insertion-side order metadata and publish its move when the + // projected child value is unchanged. + it(`publishes an ordered included child move exactly once`, async () => { + const parents = createCollection( + mockSyncCollectionOptions<{ id: string }>({ + id: `order-only-parents-${seq++}`, + getKey: ({ id }) => id, + initialData: [{ id: `p1` }], + }), + ) + const children = createCollection( + mockSyncCollectionOptions<{ + id: string + parentId: string + name: string + position: number + }>({ + id: `order-only-children-${seq++}`, + getKey: ({ id }) => id, + initialData: [ + { id: `c1`, parentId: `p1`, name: `One`, position: 1 }, + { id: `c2`, parentId: `p1`, name: `Two`, position: 2 }, + ], + }), + ) + const lq = createLiveQueryCollection((q) => + q.from({ parent: parents }).select(({ parent }) => ({ + id: parent.id, + children: q + .from({ child: children }) + .where(({ child }) => eq(child.parentId, parent.id)) + .orderBy(({ child }) => child.position) + .select(({ child }) => ({ id: child.id, name: child.name })), + })), + ) + await lq.preload() + + const childCollection = (lq.get(`p1`) as any).children + let notifications = 0 + const subscription = childCollection.subscribeChanges( + () => notifications++, + { includeInitialState: false }, + ) + + children.utils.begin() + children.utils.write({ + type: `update`, + value: { id: `c1`, parentId: `p1`, name: `One`, position: 3 }, + }) + children.utils.commit() + + expect([...childCollection.values()].map(({ id }: any) => id)).toEqual([ + `c2`, + `c1`, + ]) + expect(notifications).toBe(1) + subscription.unsubscribe() + }) + + // The includes flush is recursive, so the order-only-move handling must hold + // at depth, not just one level. Two levels of ordered includes + // (org -> teams -> members); move a grandchild whose projected value is + // unchanged and assert its collection re-sorts and publishes exactly once. + it(`publishes an ordered move in a deeply-nested included child exactly once`, async () => { + const orgs = createCollection( + mockSyncCollectionOptions<{ id: string }>({ + id: `order-only-orgs-${seq++}`, + getKey: ({ id }) => id, + initialData: [{ id: `o1` }], + }), + ) + const teams = createCollection( + mockSyncCollectionOptions<{ + id: string + orgId: string + position: number + }>({ + id: `order-only-teams-${seq++}`, + getKey: ({ id }) => id, + initialData: [{ id: `t1`, orgId: `o1`, position: 1 }], + }), + ) + const members = createCollection( + mockSyncCollectionOptions<{ + id: string + teamId: string + name: string + position: number + }>({ + id: `order-only-members-${seq++}`, + getKey: ({ id }) => id, + initialData: [ + { id: `m1`, teamId: `t1`, name: `One`, position: 1 }, + { id: `m2`, teamId: `t1`, name: `Two`, position: 2 }, + ], + }), + ) + const lq = createLiveQueryCollection((q) => + q.from({ org: orgs }).select(({ org }) => ({ + id: org.id, + teams: q + .from({ team: teams }) + .where(({ team }) => eq(team.orgId, org.id)) + .orderBy(({ team }) => team.position) + .select(({ team }) => ({ + id: team.id, + members: q + .from({ member: members }) + .where(({ member }) => eq(member.teamId, team.id)) + .orderBy(({ member }) => member.position) + .select(({ member }) => ({ id: member.id, name: member.name })), + })), + })), + ) + await lq.preload() + + const teamCollection = (lq.get(`o1`) as any).teams + const memberCollection = teamCollection.get(`t1`).members + let notifications = 0 + const subscription = memberCollection.subscribeChanges( + () => notifications++, + { includeInitialState: false }, + ) + + // Move m1 behind m2 (position 1 -> 3); projected { id, name } unchanged. + members.utils.begin() + members.utils.write({ + type: `update`, + value: { id: `m1`, teamId: `t1`, name: `One`, position: 3 }, + }) + members.utils.commit() + + expect([...memberCollection.values()].map(({ id }: any) => id)).toEqual([ + `m2`, + `m1`, + ]) + expect(notifications).toBe(1) + subscription.unsubscribe() + }) +}) diff --git a/packages/db/tests/query/includes-oracle.property.test.ts b/packages/db/tests/query/includes-oracle.property.test.ts index 0239bd3df..a69fde2da 100644 --- a/packages/db/tests/query/includes-oracle.property.test.ts +++ b/packages/db/tests/query/includes-oracle.property.test.ts @@ -1198,16 +1198,16 @@ describe(`includes recompute oracle`, () => { }, ) - // These known failures must reject with the oracle's assertion mismatch. - // A fixed bug or an unrelated runtime error makes the matching test fail. fcTest.prop([fc.constant(confirmedChildReorderSeed)], { numRuns: 1, seed: 2051245230, })( - `discovered seed: confirmed child reorder matches recomputation`, - expectAssertionFailure(expectScenarioMatches), + `regression seed: confirmed child reorder matches recomputation`, + expectScenarioMatches, ) + // These known failures must reject with the oracle's assertion mismatch. + // A fixed bug or an unrelated runtime error makes the matching test fail. fcTest.prop([fc.constant(sharedMaterializeSeed)], { numRuns: 1, seed: 1685, @@ -1348,8 +1348,8 @@ describe(`includes recompute oracle`, () => { ) fcTest.prop([fc.constant(`#1444`)], { numRuns: 1, seed: 1444 })( - `known seed: optimistic child reorder matches recomputation`, - expectAssertionFailure(async () => { + `regression seed: optimistic child reorder matches recomputation`, + async () => { const roots = createControlledCollection(`order-seed-roots`, [ { id: 1, group: 1, value: 0, position: 0 }, ]) @@ -1411,6 +1411,6 @@ describe(`includes recompute oracle`, () => { children.collection.cleanup(), ]) } - }), + }, ) })