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. 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`, () => { 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(