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/changes.ts b/packages/db/src/collection/changes.ts index dc07cd3f1..dc79a0443 100644 --- a/packages/db/src/collection/changes.ts +++ b/packages/db/src/collection/changes.ts @@ -60,6 +60,19 @@ export class CollectionChangesManager< } } + /** + * Notify subscribers that the visible layout (row order) changed without any + * row value changing — e.g. an order-only move in an `orderBy` live query. + * Emits an empty batch directly (bypassing the empty-array check) so ordered + * consumers re-read the now re-sorted collection. This is a first-class layout + * signal, deliberately not a forged row `update`. + */ + public emitLayoutChangeEvent(): void { + for (const subscription of this.changeSubscriptions) { + subscription.emitEvents([]) + } + } + /** * Enriches a change message with virtual properties ($synced, $origin, $key, $collectionId). * Uses the "add-if-missing" pattern to preserve virtual properties from upstream collections. diff --git a/packages/db/src/live-query-observer.ts b/packages/db/src/live-query-observer.ts index 8387b3838..cd742a7d5 100644 --- a/packages/db/src/live-query-observer.ts +++ b/packages/db/src/live-query-observer.ts @@ -22,6 +22,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 @@ -70,6 +81,7 @@ const DISABLED_SNAPSHOT: LiveQuerySnapshot = { state: undefined, data: undefined, collection: undefined, + layoutRevision: 0, status: `disabled`, isLoading: false, isReady: true, @@ -89,6 +101,8 @@ class LiveQueryObserverImpl< private cachedVersion = -1 private cachedStatus: CollectionStatus | undefined private cachedSnapshot: LiveQuerySnapshot = DISABLED_SNAPSHOT + private layoutRevision = 0 + private lastLayoutKeys: Array | undefined private readonly listeners = new Set>() private collectionUnsub: (() => void) | null = null // Bumped on each attach. `onFirstReady` can't be unsubscribed, so a callback @@ -124,6 +138,29 @@ class LiveQueryObserverImpl< let stateCache: Map | null = null let dataCache: Array | null = null + // 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 = { get state() { if (!stateCache) stateCache = new Map(entries) @@ -134,6 +171,7 @@ class LiveQueryObserverImpl< return singleResult ? dataCache[0] : dataCache }, collection, + layoutRevision: this.layoutRevision, status: collection.status, ...getLiveQueryStatusFlags(collection.status), isEnabled: true, diff --git a/packages/db/src/query/live/collection-config-builder.ts b/packages/db/src/query/live/collection-config-builder.ts index a6a51b478..0db2687de 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 }) } @@ -811,6 +817,16 @@ export class CollectionConfigBuilder< begin() changesToApply.forEach(this.applyChanges.bind(this, config)) commit() + // An order-only move (the row's projected value is unchanged but its + // `orderByIndex` moved) is swallowed by the collection's value-diff, so + // `commit()` emits nothing even though `.values()`/`.entries()` are now + // re-sorted. Publish an explicit layout-change notification so ordered + // consumers re-read — a first-class signal, not a forged row `update`. + // Only when the commit published nothing else: any real row change + // already notifies subscribers, who re-read the re-sorted collection. + if (needsLayoutOnlyPublication(changesToApply)) { + emitLayoutChange(config.collection) + } } pendingChanges = new Map() @@ -893,9 +909,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 +1341,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) @@ -1985,6 +2011,13 @@ function flushIncludesState( } } entry.syncMethods.commit() + // Same order-only-move handling as the parent flush: a child row that + // moved without a value change is swallowed by the value-diff, so + // publish a layout-only notification when the child commit published + // nothing else. + if (needsLayoutOnlyPublication(childChanges)) { + emitLayoutChange(entry.syncMethods.collection) + } } // Update routing index for nested includes @@ -2315,6 +2348,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 +2363,48 @@ function accumulateChanges( acc.set(key, changes) return acc } + +/** + * Decide whether a flush needs a standalone layout-change publication. + * + * 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. But that notification is only + * needed when the same `commit()` published nothing else: any real change + * (insert, delete, or a value-changed update) already notifies subscribers, who + * re-read the re-sorted collection, so a second layout event would be redundant. + * + * Returns true iff there is at least one order-only move AND no change that the + * commit itself publishes. + */ +function needsLayoutOnlyPublication( + changesToApply: Map>, +): boolean { + let layoutMoved = false + let commitPublishes = false + for (const changes of changesToApply.values()) { + const isUpdate = changes.inserts > 0 && changes.deletes > 0 + if (!isUpdate) { + // A net insert or delete always publishes a row change. + commitPublishes = true + } else if ( + changes.previousValue === undefined || + !deepEquals(changes.previousValue, changes.value) + ) { + // In-place update whose value changed — publishes a normal `update`. + commitPublishes = true + } else if (changes.orderByIndex !== changes.previousOrderByIndex) { + // Value unchanged, position moved — swallowed by the value-diff. + layoutMoved = true + } + } + return layoutMoved && !commitPublishes +} + +/** Emit a layout-only change notification on a live-query collection. */ +function emitLayoutChange(collection: Collection): void { + const changesManager = (collection as any)._changes as { + emitLayoutChangeEvent: () => void + } + changesManager.emitLayoutChangeEvent() +} 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 ba4c53360..4f612f4eb 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 1035a11ef..eb07e0c3d 100644 --- a/packages/db/tests/live-query-observer.test.ts +++ b/packages/db/tests/live-query-observer.test.ts @@ -189,4 +189,30 @@ describe(`createLiveQueryObserver`, () => { expect(observer.getSnapshot().status).toBe(`ready`) 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() + }) }) 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..a4fb2700f --- /dev/null +++ b/packages/db/tests/live-query-order-only-move.test.ts @@ -0,0 +1,322 @@ +import { describe, expect, it } from 'vitest' +import { createCollection } from '../src/collection/index.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(`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) + observer.subscribe(() => {}) + + 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) + 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) + observer.dispose() + }) + + // 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() + }) +})