diff --git a/.changeset/orderby-reorder-emission.md b/.changeset/orderby-reorder-emission.md new file mode 100644 index 000000000..5116473ec --- /dev/null +++ b/.changeset/orderby-reorder-emission.md @@ -0,0 +1,5 @@ +--- +"@tanstack/db": patch +--- + +Fix live queries with `orderBy` not emitting a change when a row's position changes but its projected value does not. Previously, a query like `q.from(...).orderBy((r) => r.value).select((r) => ({ id: r.id }))` would silently keep the stale order after a reorder (the collection's value-diff suppressed the move because the projected value was unchanged), so `useLiveQuery` rendered the old order. Order-only moves are now detected from the orderBy operator's retract/insert pair and emitted directly, with no effect on collections without `orderBy` and no double-emit when the sort field is part of the projection. diff --git a/packages/db/src/query/live/collection-config-builder.ts b/packages/db/src/query/live/collection-config-builder.ts index b40e6e431..6e8aec04e 100644 --- a/packages/db/src/query/live/collection-config-builder.ts +++ b/packages/db/src/query/live/collection-config-builder.ts @@ -5,6 +5,7 @@ import { compileQuery, } from '../compiler/index.js' import { createCollection } from '../../collection/index.js' +import { deepEquals } from '../../utils.js' import { MissingAliasInputsError, SetWindowRequiresOrderByError, @@ -797,6 +798,11 @@ export class CollectionConfigBuilder< existing.orderByIndex = changes.orderByIndex } } + // Keep the previous value/index from the retract side (the old row) + if (changes.deletes > 0 && changes.previousValue !== undefined) { + existing.previousValue = changes.previousValue + existing.previousOrderByIndex = changes.previousOrderByIndex + } } else { merged.set(customKey, { ...changes }) } @@ -809,6 +815,9 @@ export class CollectionConfigBuilder< begin() changesToApply.forEach(this.applyChanges.bind(this, config)) commit() + // Order-only moves (same value, new orderByIndex) are suppressed by the + // collection's value-diff; emit them directly so ordered consumers update. + this.emitOrderOnlyMoves(config, changesToApply) } pendingChanges = new Map() @@ -967,6 +976,61 @@ export class CollectionConfigBuilder< } } + /** + * Emit `update` events for rows whose position changed but whose value did + * not. The collection's synced commit diffs by value (deepEquals), so a pure + * reorder — same projected value, new orderByIndex — is silently swallowed + * even though the sorted output order changed. We detect those moves from the + * retract/insert pair the orderBy operator already streams and emit directly, + * mirroring the includes-materialization direct-emit pattern. + * + * The gate (index changed AND value unchanged) is mutually exclusive with the + * collection's own emit (which fires only when the value changed), so this can + * never double-emit, and it is a no-op for collections without orderBy. + */ + private emitOrderOnlyMoves( + config: SyncMethods, + changesToApply: Map>, + ): void { + const moves: Array> = [] + for (const changes of changesToApply.values()) { + const { + deletes, + inserts, + value, + orderByIndex, + previousValue, + previousOrderByIndex, + } = changes + if ( + inserts > 0 && + deletes > 0 && + orderByIndex !== undefined && + previousOrderByIndex !== undefined && + orderByIndex !== previousOrderByIndex && + previousValue !== undefined && + deepEquals(previousValue, value) + ) { + moves.push({ + type: `update`, + key: config.collection.getKeyFromItem(value), + value, + previousValue, + }) + } + } + + if (moves.length > 0) { + const changesManager = (config.collection as any)._changes as { + emitEvents: ( + changes: Array>, + forceEmit?: boolean, + ) => void + } + changesManager.emitEvents(moves, true) + } + } + /** * Handle status changes from source collections */ @@ -2009,6 +2073,10 @@ function accumulateChanges( } if (multiplicity < 0) { changes.deletes += Math.abs(multiplicity) + // Capture the retracted (old) value/index so an order-only move can be + // detected later, even when the retract and insert arrive in either order. + changes.previousValue = value + changes.previousOrderByIndex = orderByIndex } else if (multiplicity > 0) { changes.inserts += multiplicity // Update value to the latest version for this key diff --git a/packages/db/src/query/live/types.ts b/packages/db/src/query/live/types.ts index 118015bd6..19d43360f 100644 --- a/packages/db/src/query/live/types.ts +++ b/packages/db/src/query/live/types.ts @@ -16,6 +16,11 @@ export type Changes = { inserts: number value: T orderByIndex: string | undefined + // The retracted (old) side of a change, captured so the live query can detect + // an order-only move (same value, different orderByIndex) that the collection's + // value-diff would otherwise suppress. + previousValue?: T + previousOrderByIndex?: string | undefined } export type SyncState = { diff --git a/packages/db/tests/live-query-orderby-reorder.test.ts b/packages/db/tests/live-query-orderby-reorder.test.ts new file mode 100644 index 000000000..993ac52fc --- /dev/null +++ b/packages/db/tests/live-query-orderby-reorder.test.ts @@ -0,0 +1,206 @@ +import { describe, expect, it } from 'vitest' +import { createCollection, createLiveQueryCollection } from '../src/index' +import type { SyncConfig } from '../src/types' + +type Item = { id: string; name: string; value: number } + +type SyncFns = Pick< + Parameters[`sync`]>[0], + `begin` | `write` | `commit` | `markReady` +> + +// The live query processes synchronously on commit, so tests assert directly +// after each write — no timing/sleep needed. +const idsOf = (live: { values: () => IterableIterator }): Array => + Array.from(live.values(), (v) => (v as { id: string }).id) + +function manualCollection(id: string, initial: Array) { + let fns: SyncFns | undefined + const collection = createCollection({ + id, + getKey: (i) => i.id, + sync: { + sync: (params) => { + fns = params + params.begin() + for (const item of initial) params.write({ type: `insert`, value: item }) + params.commit() + params.markReady() + }, + }, + }) + collection.startSyncImmediate() + const write = (type: `insert` | `update` | `delete`, value: Item) => { + if (!fns) throw new Error(`sync functions not initialized`) + fns.begin() + fns.write({ type, value }) + fns.commit() + } + return { collection, write } +} + +const seed: Array = [ + { id: `a`, name: `Alice`, value: 2 }, + { id: `b`, name: `Bob`, value: 1 }, + { id: `c`, name: `Carol`, value: 3 }, +] + +describe(`live query orderBy: order-only reorder must emit a change`, () => { + it(`select(id) + orderBy(value): a reorder emits a change and updates order`, () => { + const { collection: source, write } = manualCollection(`reorder-emit-src`, [ + seed[0]!, + seed[1]!, + ]) + const live = createLiveQueryCollection({ + startSync: true, + query: (q) => + q + .from({ s: source }) + .orderBy(({ s }) => s.value) + .select(({ s }) => ({ id: s.id })), + }) + expect(idsOf(live)).toEqual([`b`, `a`]) + + let emitted = 0 + const sub = live.subscribeChanges(() => { + emitted++ + }) + + write(`update`, { id: `a`, name: `Alice`, value: 0 }) + + expect(emitted).toBeGreaterThan(0) + expect(idsOf(live)).toEqual([`a`, `b`]) + sub.unsubscribe() + }) + + it(`select(id, value) + orderBy(value): reorder emits EXACTLY one event for the moved key (no double-emit)`, () => { + const { collection: source, write } = manualCollection(`reorder-single`, [ + seed[0]!, + seed[1]!, + ]) + const live = createLiveQueryCollection({ + startSync: true, + query: (q) => + q + .from({ s: source }) + .orderBy(({ s }) => s.value) + .select(({ s }) => ({ id: s.id, value: s.value })), + }) + + let eventsForA = 0 + const sub = live.subscribeChanges((changes) => { + for (const c of changes) if (c.key === `a`) eventsForA++ + }) + + // reorder by changing the (projected) sort field + write(`update`, { id: `a`, name: `Alice`, value: 0 }) + + expect(eventsForA).toBe(1) // state.ts emits once; direct-emit is gated off + expect(idsOf(live)).toEqual([`a`, `b`]) + sub.unsubscribe() + }) + + it(`no reorder + unprojected field change: order-only path stays silent`, () => { + const { collection: source, write } = manualCollection(`no-move`, [ + seed[0]!, + seed[1]!, + ]) + const live = createLiveQueryCollection({ + startSync: true, + query: (q) => + q + .from({ s: source }) + .orderBy(({ s }) => s.value) + .select(({ s }) => ({ id: s.id })), + }) + + let emitted = 0 + const sub = live.subscribeChanges(() => { + emitted++ + }) + + // change a non-projected, non-sort field; position is unchanged + write(`update`, { id: `a`, name: `Alicia`, value: 2 }) + + expect(emitted).toBe(0) // nothing visible changed + expect(idsOf(live)).toEqual([`b`, `a`]) + sub.unsubscribe() + }) + + it(`select(id) + orderBy + limit: reorder within the window emits and reorders`, () => { + const { collection: source, write } = manualCollection(`reorder-limit`, seed) + const live = createLiveQueryCollection({ + startSync: true, + query: (q) => + q + .from({ s: source }) + .orderBy(({ s }) => s.value) + .limit(3) + .select(({ s }) => ({ id: s.id })), + }) + expect(idsOf(live)).toEqual([`b`, `a`, `c`]) + + let emitted = 0 + const sub = live.subscribeChanges(() => { + emitted++ + }) + + // move c to the front (3 -> 0) + write(`update`, { id: `c`, name: `Carol`, value: 0 }) + + expect(emitted).toBeGreaterThan(0) + expect(idsOf(live)).toEqual([`c`, `b`, `a`]) + sub.unsubscribe() + }) + + it(`limit(0): empty window stays empty and silent across a reorder`, () => { + const { collection: source, write } = manualCollection(`reorder-limit0`, seed) + const live = createLiveQueryCollection({ + startSync: true, + query: (q) => + q + .from({ s: source }) + .orderBy(({ s }) => s.value) + .limit(0) + .select(({ s }) => ({ id: s.id })), + }) + expect(idsOf(live)).toEqual([]) + + let emitted = 0 + const sub = live.subscribeChanges(() => { + emitted++ + }) + + write(`update`, { id: `a`, name: `Alice`, value: 0 }) + + expect(emitted).toBe(0) + expect(idsOf(live)).toEqual([]) + sub.unsubscribe() + }) + + it(`offset beyond data length: empty window stays empty and silent across a reorder`, () => { + const { collection: source, write } = manualCollection(`reorder-offset`, seed) + const live = createLiveQueryCollection({ + startSync: true, + query: (q) => + q + .from({ s: source }) + .orderBy(({ s }) => s.value) + .offset(10) + .limit(3) + .select(({ s }) => ({ id: s.id })), + }) + expect(idsOf(live)).toEqual([]) + + let emitted = 0 + const sub = live.subscribeChanges(() => { + emitted++ + }) + + write(`update`, { id: `a`, name: `Alice`, value: 0 }) + + expect(emitted).toBe(0) + expect(idsOf(live)).toEqual([]) + sub.unsubscribe() + }) +}) diff --git a/packages/react-db/tests/useLiveQuery-orderby-reorder.test.tsx b/packages/react-db/tests/useLiveQuery-orderby-reorder.test.tsx new file mode 100644 index 000000000..1af0d8264 --- /dev/null +++ b/packages/react-db/tests/useLiveQuery-orderby-reorder.test.tsx @@ -0,0 +1,47 @@ +import { describe, expect, it } from 'vitest' +import { act, renderHook, waitFor } from '@testing-library/react' +import { createCollection } from '@tanstack/db' +import { mockSyncCollectionOptions } from '../../db/tests/utils' +import { useLiveQuery } from '../src/useLiveQuery' + +type Item = { id: string; value: number } + +function make(id: string, initialData: Array) { + const collection = createCollection( + mockSyncCollectionOptions({ id, getKey: (i) => i.id, initialData }), + ) + const write = (type: `insert` | `update` | `delete`, value: Item) => { + collection.utils.begin() + collection.utils.write({ type, value }) + collection.utils.commit() + } + return { collection, write } +} + +describe(`useLiveQuery orderBy + select(id-only): reorder is reflected`, () => { + it(`a reorder updates the rendered id order even when value is not selected`, async () => { + const { collection, write } = make(`uq-reorder`, [ + { id: `a`, value: 2 }, + { id: `b`, value: 1 }, + ]) + const { result } = renderHook(() => + useLiveQuery((q) => + q + .from({ s: collection }) + .orderBy(({ s }) => s.value) + .select(({ s }) => ({ id: s.id })), + ), + ) + + await waitFor(() => expect(result.current.data.length).toBe(2)) + expect(result.current.data.map((r: any) => r.id)).toEqual([`b`, `a`]) + + act(() => { + write(`update`, { id: `a`, value: 0 }) + }) + + await waitFor(() => + expect(result.current.data.map((r: any) => r.id)).toEqual([`a`, `b`]), + ) + }) +})