From a633baaaf584dd5705bfbf548155b4114b9697c2 Mon Sep 17 00:00:00 2001 From: Jaime Resano Date: Sun, 19 Jul 2026 15:56:50 +0200 Subject: [PATCH] Fix #1685 --- .../query/live/collection-config-builder.ts | 27 +++- packages/db/tests/query/includes.test.ts | 117 ++++++++++++++++++ 2 files changed, 142 insertions(+), 2 deletions(-) diff --git a/packages/db/src/query/live/collection-config-builder.ts b/packages/db/src/query/live/collection-config-builder.ts index 4d80b3fe1..f70ee17ec 100644 --- a/packages/db/src/query/live/collection-config-builder.ts +++ b/packages/db/src/query/live/collection-config-builder.ts @@ -1864,6 +1864,9 @@ function createChildCollectionEntry( * 3. Drain nested buffers — route buffered grandchild changes to per-entry states * 4. Flush per-entry states — recursively flush nested includes on each entry * 5. Parent DELETEs — clean up child entries and routing index + * + * `routingCleanup` collects the rows whose internal routing stamp must be + * dropped once the whole recursive pass is done; the outermost call owns it. */ function flushIncludesState( includesState: Array, @@ -1871,7 +1874,13 @@ function flushIncludesState( parentId: string, parentChanges: Map> | null, parentSyncMethods: SyncMethods | null, + routingCleanup?: Set>, ): void { + // The outermost call owns the cleanup set; nested calls inherit it so the + // stamp survives until every parent group has been flushed. + const isOutermostCall = routingCleanup === undefined + const cleanup = routingCleanup ?? new Set>() + for (const state of includesState) { // Phase 1: Parent INSERTs — ensure a child Collection exists for every parent if (parentChanges) { @@ -2008,6 +2017,7 @@ function flushIncludesState( entry.collection.id, childChanges, entry.syncMethods, + cleanup, ) } } @@ -2022,6 +2032,7 @@ function flushIncludesState( entry.collection.id, null, entry.syncMethods, + cleanup, ) } } @@ -2044,6 +2055,7 @@ function flushIncludesState( entry.collection.id, null, entry.syncMethods, + cleanup, ) deepBufferDirty.add(correlationKey) } @@ -2128,10 +2140,21 @@ function flushIncludesState( } } - // Clean up the internal routing stamp from parent/child results + // Defer clearing the internal routing stamp: a single child result object can + // be routed to several parent groups (sibling parents that share a + // correlation key), and each of those groups reads the stamp when its own + // nested level is flushed. Stripping it here would drop the deeper levels for + // every group flushed after the first, so collect the rows and clear them + // once the whole recursive pass is done. if (parentChanges) { for (const [, changes] of parentChanges) { - delete changes.value[INCLUDES_ROUTING] + cleanup.add(changes.value) + } + } + + if (isOutermostCall) { + for (const value of cleanup) { + delete value[INCLUDES_ROUTING] } } } diff --git a/packages/db/tests/query/includes.test.ts b/packages/db/tests/query/includes.test.ts index 2eba6f122..48ac222b2 100644 --- a/packages/db/tests/query/includes.test.ts +++ b/packages/db/tests/query/includes.test.ts @@ -6759,4 +6759,121 @@ describe(`includes subqueries`, () => { ]) }) }) + + // Regression test for https://github.com/TanStack/db/issues/1685 + // + // Shape of the collections: + // + // root 1 ──> mid 1 ─┐ + // ├──> shared 1 ──> leaf 1 + // root 2 ──> mid 2 ─┘ + // + // Both root rows resolve (through distinct `mid` rows) to the *same* `shared` + // row, which in turn resolves to the same `leaf` row. The deepest level was + // being dropped on all but one of the parent rows. + describe(`nested subqueries with a shared middle level`, () => { + type Leaf = { id: number } + type Shared = { id: number; leafId: number } + type Mid = { id: number; sharedId: number } + type Root = { id: number; midId: number } + + function buildCollections() { + const leaves = createCollection( + mockSyncCollectionOptions({ + id: `leaves`, + getKey: (r) => r.id, + initialData: [{ id: 1 }], + }), + ) + + // The shared level: a single row that both `mid` rows point at. + const shareds = createCollection( + mockSyncCollectionOptions({ + id: `shareds`, + getKey: (r) => r.id, + initialData: [{ id: 1, leafId: 1 }], + }), + ) + + const mids = createCollection( + mockSyncCollectionOptions({ + id: `mids`, + getKey: (r) => r.id, + initialData: [ + { id: 1, sharedId: 1 }, + { id: 2, sharedId: 1 }, + ], + }), + ) + + const roots = createCollection( + mockSyncCollectionOptions({ + id: `roots`, + getKey: (r) => r.id, + initialData: [ + { id: 1, midId: 1 }, + { id: 2, midId: 2 }, + ], + }), + ) + + return { leaves, shareds, mids, roots } + } + + it(`keeps the deepest level on every parent row`, async () => { + const { leaves, shareds, mids, roots } = buildCollections() + + const live = createLiveQueryCollection((q) => + q.from({ root: roots }).select(({ root }) => ({ + ...root, + mid: toArray( + q + .from({ mid: mids }) + .where(({ mid }) => eq(mid.id, root.midId)) + .select(({ mid }) => ({ + ...mid, + shared: toArray( + q + .from({ shared: shareds }) + .where(({ shared }) => eq(shared.id, mid.sharedId)) + .select(({ shared }) => ({ + ...shared, + leaf: toArray( + q + .from({ leaf: leaves }) + .where(({ leaf }) => eq(leaf.id, shared.leafId)) + .findOne(), + ), + })) + .findOne(), + ), + })) + .findOne(), + ), + })), + ) + + await live.preload() + + const rows = live.toArray + expect(rows).toHaveLength(2) + + for (const root of rows) { + const mid = root.mid.at(0) + expect(mid, `root ${root.id} is missing its mid`).toBeDefined() + expect(mid!.id).toBe(root.midId) + + const shared = mid!.shared.at(0) + expect(shared, `root ${root.id} is missing its shared`).toBeDefined() + expect(shared!.id).toBe(1) + + // The regression: `leaf` was undefined on one of the two root rows, + // because `shared 1` is reached through two different `mid` rows. + expect( + stripVirtualProps(shared!.leaf.at(0)!), + `root ${root.id} dropped its leaf`, + ).toEqual({ id: 1 }) + } + }) + }) })