Skip to content

Commit c9ec751

Browse files
kevin-dpclaude
andcommitted
fix(db): coalesce layout publications and cover ordered includes children
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>
1 parent 43f1a7d commit c9ec751

1 file changed

Lines changed: 56 additions & 19 deletions

File tree

packages/db/src/query/live/collection-config-builder.ts

Lines changed: 56 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -822,11 +822,10 @@ export class CollectionConfigBuilder<
822822
// `commit()` emits nothing even though `.values()`/`.entries()` are now
823823
// re-sorted. Publish an explicit layout-change notification so ordered
824824
// consumers re-read — a first-class signal, not a forged row `update`.
825-
if (hasOrderOnlyMove(changesToApply)) {
826-
const changesManager = (config.collection as any)._changes as {
827-
emitLayoutChangeEvent: () => void
828-
}
829-
changesManager.emitLayoutChangeEvent()
825+
// Only when the commit published nothing else: any real row change
826+
// already notifies subscribers, who re-read the re-sorted collection.
827+
if (needsLayoutOnlyPublication(changesToApply)) {
828+
emitLayoutChange(config.collection)
830829
}
831830
}
832831
pendingChanges = new Map()
@@ -910,9 +909,14 @@ export class CollectionConfigBuilder<
910909

911910
if (multiplicity < 0) {
912911
existing.deletes += Math.abs(multiplicity)
912+
existing.previousValue = childResult
913+
existing.previousOrderByIndex = _orderByIndex
913914
} else if (multiplicity > 0) {
914915
existing.inserts += multiplicity
915916
existing.value = childResult
917+
if (_orderByIndex !== undefined) {
918+
existing.orderByIndex = _orderByIndex
919+
}
916920
}
917921

918922
byChild.set(childKey, existing)
@@ -1337,9 +1341,14 @@ function setupNestedPipelines(
13371341

13381342
if (multiplicity < 0) {
13391343
existing.deletes += Math.abs(multiplicity)
1344+
existing.previousValue = childResult
1345+
existing.previousOrderByIndex = _orderByIndex
13401346
} else if (multiplicity > 0) {
13411347
existing.inserts += multiplicity
13421348
existing.value = childResult
1349+
if (_orderByIndex !== undefined) {
1350+
existing.orderByIndex = _orderByIndex
1351+
}
13431352
}
13441353

13451354
byChild.set(childKey, existing)
@@ -2002,6 +2011,13 @@ function flushIncludesState(
20022011
}
20032012
}
20042013
entry.syncMethods.commit()
2014+
// Same order-only-move handling as the parent flush: a child row that
2015+
// moved without a value change is swallowed by the value-diff, so
2016+
// publish a layout-only notification when the child commit published
2017+
// nothing else.
2018+
if (needsLayoutOnlyPublication(childChanges)) {
2019+
emitLayoutChange(entry.syncMethods.collection)
2020+
}
20052021
}
20062022

20072023
// Update routing index for nested includes
@@ -2349,25 +2365,46 @@ function accumulateChanges<T>(
23492365
}
23502366

23512367
/**
2352-
* Detect whether any accumulated change is an "order-only move": a row that was
2353-
* updated in place (both retracted and re-inserted) whose `orderByIndex` moved
2354-
* but whose projected value is deep-equal to before. The collection's value-diff
2355-
* emits nothing for these, so the flush must publish a layout notification.
2356-
* Rows whose value actually changed are ignored — they emit a normal `update`.
2368+
* Decide whether a flush needs a standalone layout-change publication.
2369+
*
2370+
* An "order-only move" — a row updated in place whose `orderByIndex` moved but
2371+
* whose projected value is deep-equal to before — is swallowed by the value-diff
2372+
* and needs an explicit layout notification. But that notification is only
2373+
* needed when the same `commit()` published nothing else: any real change
2374+
* (insert, delete, or a value-changed update) already notifies subscribers, who
2375+
* re-read the re-sorted collection, so a second layout event would be redundant.
2376+
*
2377+
* Returns true iff there is at least one order-only move AND no change that the
2378+
* commit itself publishes.
23572379
*/
2358-
function hasOrderOnlyMove<T>(
2380+
function needsLayoutOnlyPublication<T>(
23592381
changesToApply: Map<unknown, Changes<T>>,
23602382
): boolean {
2383+
let layoutMoved = false
2384+
let commitPublishes = false
23612385
for (const changes of changesToApply.values()) {
2362-
if (
2363-
changes.inserts > 0 &&
2364-
changes.deletes > 0 &&
2365-
changes.orderByIndex !== changes.previousOrderByIndex &&
2366-
changes.previousValue !== undefined &&
2367-
deepEquals(changes.previousValue, changes.value)
2386+
const isUpdate = changes.inserts > 0 && changes.deletes > 0
2387+
if (!isUpdate) {
2388+
// A net insert or delete always publishes a row change.
2389+
commitPublishes = true
2390+
} else if (
2391+
changes.previousValue === undefined ||
2392+
!deepEquals(changes.previousValue, changes.value)
23682393
) {
2369-
return true
2394+
// In-place update whose value changed — publishes a normal `update`.
2395+
commitPublishes = true
2396+
} else if (changes.orderByIndex !== changes.previousOrderByIndex) {
2397+
// Value unchanged, position moved — swallowed by the value-diff.
2398+
layoutMoved = true
23702399
}
23712400
}
2372-
return false
2401+
return layoutMoved && !commitPublishes
2402+
}
2403+
2404+
/** Emit a layout-only change notification on a live-query collection. */
2405+
function emitLayoutChange(collection: Collection<any, any, any>): void {
2406+
const changesManager = (collection as any)._changes as {
2407+
emitLayoutChangeEvent: () => void
2408+
}
2409+
changesManager.emitLayoutChangeEvent()
23732410
}

0 commit comments

Comments
 (0)