From 422b31f90d44432a023f0ccabb0151da01e19e2a Mon Sep 17 00:00:00 2001 From: Sam Willis Date: Thu, 4 Dec 2025 18:59:39 +0000 Subject: [PATCH 1/3] test(react-db): add tests for deletion from partial page in infinite query Add tests for deleting items from a page with fewer rows than pageSize in useLiveInfiniteQuery. Tests both ascending and descending order to verify the behavior difference. --- .../tests/useLiveInfiniteQuery.test.tsx | 131 ++++++++++++++++++ 1 file changed, 131 insertions(+) diff --git a/packages/react-db/tests/useLiveInfiniteQuery.test.tsx b/packages/react-db/tests/useLiveInfiniteQuery.test.tsx index 5dbb69c1b..b59753b34 100644 --- a/packages/react-db/tests/useLiveInfiniteQuery.test.tsx +++ b/packages/react-db/tests/useLiveInfiniteQuery.test.tsx @@ -368,6 +368,137 @@ describe(`useLiveInfiniteQuery`, () => { expect(result.current.pages[1]).toHaveLength(10) }) + it(`should handle deletion from partial page with descending order`, async () => { + // Create only 5 items - fewer than the pageSize of 20 + const posts = createMockPosts(5) + const collection = createCollection( + mockSyncCollectionOptions({ + id: `partial-page-deletion-desc-test`, + getKey: (post: Post) => post.id, + initialData: posts, + }) + ) + + const { result } = renderHook(() => { + return useLiveInfiniteQuery( + (q) => + q + .from({ posts: collection }) + .orderBy(({ posts: p }) => p.createdAt, `desc`), + { + pageSize: 20, + getNextPageParam: (lastPage) => + lastPage.length === 20 ? lastPage.length : undefined, + } + ) + }) + + await waitFor(() => { + expect(result.current.isReady).toBe(true) + }) + + // Should have all 5 items on one page (partial page) + expect(result.current.pages).toHaveLength(1) + expect(result.current.data).toHaveLength(5) + expect(result.current.hasNextPage).toBe(false) + + // Verify the first item (most recent by createdAt descending) + const firstItemId = result.current.data[0]!.id + expect(firstItemId).toBe(`1`) // Post 1 has the highest createdAt + + // Delete the first item (the one that appears first in descending order) + act(() => { + collection.utils.begin() + collection.utils.write({ + type: `delete`, + value: posts[0]!, // Post 1 + }) + collection.utils.commit() + }) + + // The deleted item should disappear from the result + await waitFor(() => { + expect(result.current.data).toHaveLength(4) + }) + + // Verify the deleted item is no longer in the data + expect( + result.current.data.find((p) => p.id === firstItemId) + ).toBeUndefined() + + // Verify the new first item is Post 2 + expect(result.current.data[0]!.id).toBe(`2`) + + // Still should have 1 page with 4 items + expect(result.current.pages).toHaveLength(1) + expect(result.current.pages[0]).toHaveLength(4) + expect(result.current.hasNextPage).toBe(false) + }) + + it(`should handle deletion from partial page with ascending order`, async () => { + // Create only 5 items - fewer than the pageSize of 20 + const posts = createMockPosts(5) + const collection = createCollection( + mockSyncCollectionOptions({ + id: `partial-page-deletion-asc-test`, + getKey: (post: Post) => post.id, + initialData: posts, + }) + ) + + const { result } = renderHook(() => { + return useLiveInfiniteQuery( + (q) => + q + .from({ posts: collection }) + .orderBy(({ posts: p }) => p.createdAt, `asc`), // ascending order + { + pageSize: 20, + getNextPageParam: (lastPage) => + lastPage.length === 20 ? lastPage.length : undefined, + } + ) + }) + + await waitFor(() => { + expect(result.current.isReady).toBe(true) + }) + + // Should have all 5 items on one page (partial page) + expect(result.current.pages).toHaveLength(1) + expect(result.current.data).toHaveLength(5) + expect(result.current.hasNextPage).toBe(false) + + // In ascending order, Post 5 has the lowest createdAt and appears first + const firstItemId = result.current.data[0]!.id + expect(firstItemId).toBe(`5`) // Post 5 has the lowest createdAt + + // Delete the first item (the one that appears first in ascending order) + act(() => { + collection.utils.begin() + collection.utils.write({ + type: `delete`, + value: posts[4]!, // Post 5 (index 4 in array) + }) + collection.utils.commit() + }) + + // The deleted item should disappear from the result + await waitFor(() => { + expect(result.current.data).toHaveLength(4) + }) + + // Verify the deleted item is no longer in the data + expect( + result.current.data.find((p) => p.id === firstItemId) + ).toBeUndefined() + + // Still should have 1 page with 4 items + expect(result.current.pages).toHaveLength(1) + expect(result.current.pages[0]).toHaveLength(4) + expect(result.current.hasNextPage).toBe(false) + }) + it(`should work with where clauses`, async () => { const posts = createMockPosts(50) const collection = createCollection( From 79a2e269db7ddb0bff8609e2c8bea648ac6bd27a Mon Sep 17 00:00:00 2001 From: Sam Willis Date: Thu, 4 Dec 2025 19:47:51 +0000 Subject: [PATCH 2/3] fix: correctly extract indexed value in requestLimitedSnapshot The `biggestObservedValue` variable was incorrectly set to the full row object instead of the indexed value (e.g., salary). This caused the BTree comparison in `index.take()` to fail, resulting in the same data being loaded multiple times. Each item would be inserted with a multiplicity > 1, and when deleted, the multiplicity would decrement but not reach 0, so the item would remain visible in the TopK. This fix creates a value extractor from the orderBy expression and uses it to extract the actual indexed value from each row before tracking it as the "biggest observed value". --- packages/db/src/collection/subscription.ts | 14 ++++++++-- packages/db/tests/query/order-by.test.ts | 32 ++++++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/packages/db/src/collection/subscription.ts b/packages/db/src/collection/subscription.ts index 1c23d7b04..9cc3700d1 100644 --- a/packages/db/src/collection/subscription.ts +++ b/packages/db/src/collection/subscription.ts @@ -1,7 +1,8 @@ import { ensureIndexForExpression } from "../indexes/auto-index.js" import { and, eq, gt, gte, lt } from "../query/builder/functions.js" -import { Value } from "../query/ir.js" +import { PropRef, Value } from "../query/ir.js" import { EventEmitter } from "../event-emitter.js" +import { compileExpression } from "../query/compiler/evaluators.js" import { createFilterFunctionFromExpression, createFilteredCallback, @@ -314,6 +315,13 @@ export class CollectionSubscription const valuesNeeded = () => Math.max(limit - changes.length, 0) const collectionExhausted = () => keys.length === 0 + // Create a value extractor for the orderBy field to properly track the biggest indexed value + const orderByExpression = orderBy[0]!.expression + const valueExtractor = + orderByExpression.type === `ref` + ? compileExpression(new PropRef(orderByExpression.path), true) + : null + while (valuesNeeded() > 0 && !collectionExhausted()) { const insertedKeys = new Set() // Track keys we add to `changes` in this iteration @@ -324,7 +332,9 @@ export class CollectionSubscription key, value, }) - biggestObservedValue = value + // Extract the indexed value (e.g., salary) from the row, not the full row + // This is needed for index.take() to work correctly with the BTree comparator + biggestObservedValue = valueExtractor ? valueExtractor(value) : value insertedKeys.add(key) // Track this key } diff --git a/packages/db/tests/query/order-by.test.ts b/packages/db/tests/query/order-by.test.ts index d7c11b848..82afec689 100644 --- a/packages/db/tests/query/order-by.test.ts +++ b/packages/db/tests/query/order-by.test.ts @@ -733,6 +733,38 @@ function createOrderByTests(autoIndex: `off` | `eager`): void { [5, 52_000], ]) }) + + it(`handles deletion from partial page with limit larger than data`, async () => { + const collection = createLiveQueryCollection((q) => + q + .from({ employees: employeesCollection }) + .orderBy(({ employees }) => employees.salary, `desc`) + .limit(20) // Limit larger than number of employees (5) + .select(({ employees }) => ({ + id: employees.id, + name: employees.name, + salary: employees.salary, + })) + ) + await collection.preload() + + const results = Array.from(collection.values()) + expect(results).toHaveLength(5) + expect(results[0]!.name).toBe(`Diana`) + + // Delete Diana (the highest paid employee, first in DESC order) + const dianaData = employeeData.find((e) => e.id === 4)! + employeesCollection.utils.begin() + employeesCollection.utils.write({ + type: `delete`, + value: dianaData, + }) + employeesCollection.utils.commit() + + const newResults = Array.from(collection.values()) + expect(newResults).toHaveLength(4) + expect(newResults[0]!.name).toBe(`Bob`) + }) }) describe(`OrderBy with Joins`, () => { From 7890567fcb7c47865f8be639613a072992adb81b Mon Sep 17 00:00:00 2001 From: Sam Willis Date: Thu, 4 Dec 2025 19:48:45 +0000 Subject: [PATCH 3/3] chore: add changeset for DESC deletion fix --- .changeset/fix-desc-deletion-partial-page.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .changeset/fix-desc-deletion-partial-page.md diff --git a/.changeset/fix-desc-deletion-partial-page.md b/.changeset/fix-desc-deletion-partial-page.md new file mode 100644 index 000000000..1c599b6fe --- /dev/null +++ b/.changeset/fix-desc-deletion-partial-page.md @@ -0,0 +1,9 @@ +--- +"@tanstack/db": patch +--- + +Fix useLiveInfiniteQuery not updating when deleting an item from a partial page with DESC order. + +The bug occurred when using `useLiveInfiniteQuery` with `orderBy(..., 'desc')` and having fewer items than the `pageSize`. Deleting an item would not update the live result - the deleted item would remain visible until another change occurred. + +The root cause was in `requestLimitedSnapshot` where `biggestObservedValue` was incorrectly set to the full row object instead of the indexed value (e.g., the salary field used for ordering). This caused the BTree comparison to fail, resulting in the same data being loaded multiple times with each item having a multiplicity > 1. When an item was deleted, its multiplicity would decrement but not reach 0, so it remained visible.