From 1669e720fe169571d6e11748a5b96ae880662423 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Bramer=20Schmidt?= Date: Fri, 17 Jul 2026 21:33:36 +0700 Subject: [PATCH 1/2] fix: save staged edits for rows loaded via infinite scroll MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With infinite scroll enabled, the grid queries pageIndex 0 with a grown pageSize window (25-row batches), but the row mutation hooks resolved their TanStack DB collection from the paginated pageIndex/pageSize. The two query scopes diverge once more than one batch is loaded, so collection.update() targeted a collection missing rows beyond the first batch and threw UpdateKeyNotFoundError, making "Save n rows" fail silently (issue prisma/studio#1466). The mutation hooks (update, updateMany, delete, insert) now receive the view's exact query props and resolve their collection through the new useActiveTableQueryCollection hook, so mutations always target the same collection scope the grid displays — including the search term dimension that was previously omitted as well. Verified end-to-end in the ppg demo: editing and deleting rows beyond the first 25-row batch with infinite scroll enabled now persists and shows the success toast. Co-Authored-By: Claude Fable 5 --- .changeset/tidy-lions-tap.md | 5 + Architecture/db-state.md | 13 + FEATURES.md | 1 + ui/hooks/use-active-table-delete.ts | 21 +- ui/hooks/use-active-table-insert.ts | 20 +- ui/hooks/use-active-table-query.ts | 25 +- .../use-active-table-update-many.test.tsx | 335 ++++++++++++++++++ ui/hooks/use-active-table-update-many.ts | 20 +- ui/hooks/use-active-table-update.ts | 20 +- ui/hooks/use-selection.test.tsx | 14 +- ui/hooks/use-selection.ts | 8 +- ui/studio/views/table/ActiveTableView.tsx | 45 ++- 12 files changed, 451 insertions(+), 76 deletions(-) create mode 100644 .changeset/tidy-lions-tap.md create mode 100644 ui/hooks/use-active-table-update-many.test.tsx diff --git a/.changeset/tidy-lions-tap.md b/.changeset/tidy-lions-tap.md new file mode 100644 index 00000000..3a18c605 --- /dev/null +++ b/.changeset/tidy-lions-tap.md @@ -0,0 +1,5 @@ +--- +"@prisma/studio-core": patch +--- + +Fix saving staged cell edits silently failing for rows loaded beyond the first batch when infinite scroll is enabled. Row update, delete, and insert mutations now target the same rows-collection scope the grid displays instead of the paginated first page. diff --git a/Architecture/db-state.md b/Architecture/db-state.md index 0c936ba8..ccd91eb8 100644 --- a/Architecture/db-state.md +++ b/Architecture/db-state.md @@ -185,6 +185,19 @@ There is currently no `onInsert` optimistic handler in the rows collection. Do n - Views MUST use `useActiveTableDelete` for row deletion. - Views MUST NOT call adapter query/update/delete directly. +### Mutation scope alignment + +Row mutation hooks (`useActiveTableUpdate`, `useActiveTableUpdateMany`, +`useActiveTableDelete`, `useActiveTableInsert`) take the view's query props +(`UseActiveTableQueryProps`) and resolve their collection through +`useActiveTableQueryCollection`. The view MUST pass the exact same query props +it uses for display, so mutations target the `queryScopeKey` that actually +contains the visible rows. With infinite scroll enabled that scope is +`pageIndex: 0` with the grown batch-window `pageSize`, not the paginated page — +deriving mutation scope independently (for example from `usePagination`) makes +`collection.update`/`collection.delete` silently miss rows loaded beyond the +first batch. + ## Lifecycle Rules Studio context owns collection lifecycle: diff --git a/FEATURES.md b/FEATURES.md index 2acaa70e..60d5a37e 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -171,6 +171,7 @@ When the ledger has migrations but no contract snapshots to diff (a database wri Table data is shown in a grid with server-backed pagination, filtered-row counts, loading feedback, and explicit empty states. The footer keeps page navigation, a page jump field, a fixed rows-per-page dropdown, and infinite-scroll mode in one compact control group, so users can either jump directly to a page, switch page density from a known preset, or turn on lazy-loading without leaving the grid. Rows-per-page and infinite-scroll preferences persist across tables through local storage, while the known filtered-row count keeps the footer stable during page transitions for the same filtered result set. Infinite scroll preloads before the hard bottom edge, always appends in fixed 25-row chunks regardless of the paginated page-size setting, keeps filling tall viewports until the grid is actually scrollable, and appends new rows in place without snapping the grid back to the top. +Row editing, deletion, and insertion operate on the same loaded row window the grid displays, so with infinite scroll enabled staged cell edits save correctly even for rows loaded beyond the first 25-row batch. Rapid sort and filter changes keep the latest request authoritative, and superseded table reads are aborted so a slower older result cannot overwrite the visible grid. ## PostgreSQL Stored Temporal Values diff --git a/ui/hooks/use-active-table-delete.ts b/ui/hooks/use-active-table-delete.ts index 2f5871b3..82bd0e85 100644 --- a/ui/hooks/use-active-table-delete.ts +++ b/ui/hooks/use-active-table-delete.ts @@ -1,20 +1,13 @@ import { useMutation } from "@tanstack/react-query"; -import { useActiveTableRowsCollection } from "./use-active-table-rows-collection"; -import { useFiltering } from "./use-filtering"; -import { usePagination } from "./use-pagination"; -import { useSorting } from "./use-sorting"; +import { + useActiveTableQueryCollection, + type UseActiveTableQueryProps, +} from "./use-active-table-query"; -export function useActiveTableDelete() { - const { paginationState } = usePagination(); - const { sortingState } = useSorting(); - const { appliedFilter } = useFiltering(); - const { activeTable, collection, refetch } = useActiveTableRowsCollection({ - pageIndex: paginationState.pageIndex, - pageSize: paginationState.pageSize, - sortOrder: sortingState, - filter: appliedFilter, - }); +export function useActiveTableDelete(query: UseActiveTableQueryProps) { + const { activeTable, collection, refetch } = + useActiveTableQueryCollection(query); const { schema = null, name: table = null } = activeTable ?? {}; return useMutation({ diff --git a/ui/hooks/use-active-table-insert.ts b/ui/hooks/use-active-table-insert.ts index a992a771..abfa45cd 100644 --- a/ui/hooks/use-active-table-insert.ts +++ b/ui/hooks/use-active-table-insert.ts @@ -1,23 +1,15 @@ import { useMutation } from "@tanstack/react-query"; import { useStudio } from "../studio/context"; -import { useActiveTableRowsCollection } from "./use-active-table-rows-collection"; -import { useFiltering } from "./use-filtering"; -import { usePagination } from "./use-pagination"; -import { useSorting } from "./use-sorting"; +import { + useActiveTableQueryCollection, + type UseActiveTableQueryProps, +} from "./use-active-table-query"; import { addRowIdToResult } from "./utils/add-row-id-to-result"; -export function useActiveTableInsert() { +export function useActiveTableInsert(query: UseActiveTableQueryProps) { const { adapter, onEvent } = useStudio(); - const { paginationState } = usePagination(); - const { sortingState } = useSorting(); - const { appliedFilter } = useFiltering(); - const { activeTable, refetch } = useActiveTableRowsCollection({ - pageIndex: paginationState.pageIndex, - pageSize: paginationState.pageSize, - sortOrder: sortingState, - filter: appliedFilter, - }); + const { activeTable, refetch } = useActiveTableQueryCollection(query); const { schema = null, name: table = null } = activeTable ?? {}; return useMutation({ diff --git a/ui/hooks/use-active-table-query.ts b/ui/hooks/use-active-table-query.ts index 290e4aae..60a05479 100644 --- a/ui/hooks/use-active-table-query.ts +++ b/ui/hooks/use-active-table-query.ts @@ -4,7 +4,10 @@ import type { SortOrderItem, Table, } from "../../data/adapter"; -import { useActiveTableRowsCollection } from "./use-active-table-rows-collection"; +import { + type ActiveTableRowsCollectionState, + useActiveTableRowsCollection, +} from "./use-active-table-rows-collection"; import { useNavigation } from "./use-navigation"; export interface UseActiveTableQueryProps { @@ -27,9 +30,16 @@ export interface UseActiveTableQueryResult { refetch: () => Promise; } -export function useActiveTableQuery( +/** + * Resolves the rows-collection state for the exact query scope described by + * `props`. Row mutation hooks must resolve their collection through this hook + * with the same query props the view uses for display, so mutations target the + * collection that actually contains the visible rows (for example the grown + * `pageIndex: 0` window used by infinite scroll). + */ +export function useActiveTableQueryCollection( props: UseActiveTableQueryProps, -): UseActiveTableQueryResult { +): ActiveTableRowsCollectionState { const { filter, pageIndex, pageSize, sortOrder } = props; const { metadata: { activeTable }, @@ -39,13 +49,20 @@ export function useActiveTableQuery( searchScope: props.searchScope ?? "table", searchTerm: props.searchTerm ?? "", }); - const state = useActiveTableRowsCollection({ + + return useActiveTableRowsCollection({ filter, fullTableSearchTerm, pageIndex, pageSize, sortOrder, }); +} + +export function useActiveTableQuery( + props: UseActiveTableQueryProps, +): UseActiveTableQueryResult { + const state = useActiveTableQueryCollection(props); return { data: state.activeTable diff --git a/ui/hooks/use-active-table-update-many.test.tsx b/ui/hooks/use-active-table-update-many.test.tsx new file mode 100644 index 00000000..09a36708 --- /dev/null +++ b/ui/hooks/use-active-table-update-many.test.tsx @@ -0,0 +1,335 @@ +import { + createCollection, + localOnlyCollectionOptions, +} from "@tanstack/react-db"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { act } from "react"; +import { createRoot } from "react-dom/client"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import type { + Adapter, + AdapterQueryDetails, + Column, + Table, +} from "../../data/adapter"; +import type { TableQueryMetaState } from "../studio/context"; +import { useActiveTableQueryCollection } from "./use-active-table-query"; +import { useActiveTableUpdateMany } from "./use-active-table-update-many"; + +const useStudioMock = vi.fn(); +const useNavigationMock = vi.fn(); + +vi.mock("../studio/context", () => ({ + useStudio: () => useStudioMock(), +})); + +vi.mock("./use-navigation", () => ({ + useNavigation: () => useNavigationMock(), +})); + +( + globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean } +).IS_REACT_ACT_ENVIRONMENT = true; + +const TOTAL_ROW_COUNT = 60; + +function createColumn(params: { + name: string; + pkPosition: number | null; +}): Column { + const { name, pkPosition } = params; + + return { + datatype: { + group: "string", + isArray: false, + isNative: true, + name: "text", + options: [], + schema: "public", + }, + defaultValue: null, + fkColumn: null, + fkSchema: null, + fkTable: null, + isAutoincrement: false, + isComputed: false, + isRequired: pkPosition != null, + name, + nullable: pkPosition == null, + pkPosition, + schema: "public", + table: "users", + }; +} + +function createActiveTable(): Table { + return { + columns: { + id: createColumn({ name: "id", pkPosition: 1 }), + name: createColumn({ name: "name", pkPosition: null }), + }, + name: "users", + schema: "public", + }; +} + +function createAdapterMock(): Adapter { + return { + defaultSchema: "public", + query: vi.fn(async (details: AdapterQueryDetails) => { + const start = details.pageIndex * details.pageSize; + const end = Math.min(TOTAL_ROW_COUNT, start + details.pageSize); + const rows = Array.from({ length: Math.max(0, end - start) }, (_, i) => ({ + id: `u${start + i + 1}`, + name: `User ${start + i + 1}`, + })); + + return [ + null, + { + filteredRowCount: TOTAL_ROW_COUNT, + query: { + parameters: [], + sql: "query", + }, + rows, + }, + ]; + }), + update: vi.fn(async (details) => { + return [ + null, + { + query: { + parameters: [], + sql: "update", + }, + row: { + ...details.row, + ...details.changes, + }, + }, + ]; + }), + } as unknown as Adapter; +} + +function createRowsCollectionCache() { + const cache = new Map(); + + return { + getOrCreateRowsCollection(key: string, factory: () => T): T { + const existing = cache.get(key) as T | undefined; + + if (existing != null) { + return existing; + } + + const created = factory(); + cache.set(key, created); + + return created; + }, + }; +} + +function createTableQueryExecutionStateCache() { + const cache = new Map< + string, + { activeController: AbortController | null; latestRequestId: number } + >(); + + return { + getOrCreateTableQueryExecutionState(key: string) { + const existing = cache.get(key); + + if (existing != null) { + return existing; + } + + const created = { + activeController: null, + latestRequestId: 0, + }; + cache.set(key, created); + + return created; + }, + }; +} + +function createTableQueryMetaCollection() { + return createCollection( + localOnlyCollectionOptions({ + id: "test-update-many-table-query-meta", + getKey(item) { + return item.id; + }, + initialData: [], + }), + ); +} + +async function flush(): Promise { + await act(async () => { + await Promise.resolve(); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); +} + +async function waitFor(assertion: () => boolean): Promise { + const timeoutMs = 2000; + const start = Date.now(); + + while (Date.now() - start < timeoutMs) { + if (assertion()) { + return; + } + + await flush(); + } + + throw new Error("Timed out waiting for hook state"); +} + +const emptyFilter = { + after: "and" as const, + filters: [], + id: "root", + kind: "FilterGroup" as const, +}; + +function renderHookHarness(queryProps: { + pageIndex: number; + pageSize: number; +}) { + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + + const adapter = createAdapterMock(); + const activeTable = createActiveTable(); + const tableQueryMetaCollection = createTableQueryMetaCollection(); + const queryClient = new QueryClient(); + const { getOrCreateRowsCollection } = createRowsCollectionCache(); + const { getOrCreateTableQueryExecutionState } = + createTableQueryExecutionStateCache(); + + useStudioMock.mockReturnValue({ + adapter, + getOrCreateTableQueryExecutionState, + getOrCreateRowsCollection, + onEvent: vi.fn(), + queryClient, + tableQueryMetaCollection, + }); + useNavigationMock.mockReturnValue({ + metadata: { + activeTable, + }, + }); + + const fullQueryProps = { + filter: emptyFilter, + pageIndex: queryProps.pageIndex, + pageSize: queryProps.pageSize, + sortOrder: [], + }; + + let latestCollectionState: + | ReturnType + | undefined; + let latestUpdateMany: ReturnType | undefined; + + function Harness() { + // Same query props the view uses for display: the collection scope holding + // the visible rows. + latestCollectionState = useActiveTableQueryCollection(fullQueryProps); + latestUpdateMany = useActiveTableUpdateMany(fullQueryProps); + + return null; + } + + act(() => { + root.render( + + + , + ); + }); + + function cleanup() { + act(() => { + root.unmount(); + }); + queryClient.clear(); + container.remove(); + } + + return { + adapter, + cleanup, + getCollectionState() { + return latestCollectionState; + }, + getUpdateMany() { + return latestUpdateMany; + }, + }; +} + +afterEach(() => { + vi.clearAllMocks(); + document.body.innerHTML = ""; +}); + +describe("useActiveTableUpdateMany", () => { + it("persists edits to rows loaded beyond the first infinite-scroll batch", async () => { + // Infinite scroll queries pageIndex 0 with a grown pageSize window + // (2 batches of 25 here). Rows 26..50 are not part of the paginated + // first page, which previously made saving their edits fail silently. + const { adapter, cleanup, getCollectionState, getUpdateMany } = + renderHookHarness({ + pageIndex: 0, + pageSize: 50, + }); + + await waitFor(() => (getCollectionState()?.rows.length ?? 0) === 50); + + const targetRow = getCollectionState()?.rows[30]; + + if (!targetRow) { + throw new Error("Expected a row beyond the first 25-row batch"); + } + + const updateMany = getUpdateMany(); + + if (!updateMany) { + throw new Error("updateMany hook was not rendered"); + } + + await act(async () => { + await updateMany.mutateAsync({ + updates: [ + { + changes: { name: "Renamed via infinite scroll" }, + row: targetRow, + }, + ], + }); + }); + + expect(adapter.update).toHaveBeenCalledTimes(1); + expect(adapter.update).toHaveBeenCalledWith( + expect.objectContaining({ + changes: { name: "Renamed via infinite scroll" }, + row: expect.objectContaining({ id: targetRow.id }), + }), + {}, + ); + + cleanup(); + }); +}); diff --git a/ui/hooks/use-active-table-update-many.ts b/ui/hooks/use-active-table-update-many.ts index 5c79f69d..96b4a230 100644 --- a/ui/hooks/use-active-table-update-many.ts +++ b/ui/hooks/use-active-table-update-many.ts @@ -1,9 +1,9 @@ import { useMutation } from "@tanstack/react-query"; -import { useActiveTableRowsCollection } from "./use-active-table-rows-collection"; -import { useFiltering } from "./use-filtering"; -import { usePagination } from "./use-pagination"; -import { useSorting } from "./use-sorting"; +import { + useActiveTableQueryCollection, + type UseActiveTableQueryProps, +} from "./use-active-table-query"; export interface UseActiveTableUpdateManyParams { updates: Array<{ @@ -12,16 +12,8 @@ export interface UseActiveTableUpdateManyParams { }>; } -export function useActiveTableUpdateMany() { - const { paginationState } = usePagination(); - const { sortingState } = useSorting(); - const { appliedFilter } = useFiltering(); - const { activeTable, collection } = useActiveTableRowsCollection({ - pageIndex: paginationState.pageIndex, - pageSize: paginationState.pageSize, - sortOrder: sortingState, - filter: appliedFilter, - }); +export function useActiveTableUpdateMany(query: UseActiveTableQueryProps) { + const { activeTable, collection } = useActiveTableQueryCollection(query); const queryKeyPrefix = [ "schema", activeTable?.schema ?? null, diff --git a/ui/hooks/use-active-table-update.ts b/ui/hooks/use-active-table-update.ts index 3cb61eab..3c1f506c 100644 --- a/ui/hooks/use-active-table-update.ts +++ b/ui/hooks/use-active-table-update.ts @@ -4,26 +4,18 @@ import type { AdapterUpdateDetails, AdapterUpdateOptions, } from "../../data/adapter"; -import { useActiveTableRowsCollection } from "./use-active-table-rows-collection"; -import { useFiltering } from "./use-filtering"; -import { usePagination } from "./use-pagination"; -import { useSorting } from "./use-sorting"; +import { + useActiveTableQueryCollection, + type UseActiveTableQueryProps, +} from "./use-active-table-query"; export interface UseActiveTableUpdateParams { details: AdapterUpdateDetails; options: AdapterUpdateOptions; } -export function useActiveTableUpdate() { - const { paginationState } = usePagination(); - const { sortingState } = useSorting(); - const { appliedFilter } = useFiltering(); - const { activeTable, collection } = useActiveTableRowsCollection({ - pageIndex: paginationState.pageIndex, - pageSize: paginationState.pageSize, - sortOrder: sortingState, - filter: appliedFilter, - }); +export function useActiveTableUpdate(query: UseActiveTableQueryProps) { + const { activeTable, collection } = useActiveTableQueryCollection(query); const queryKeyPrefix = [ "schema", activeTable?.schema ?? null, diff --git a/ui/hooks/use-selection.test.tsx b/ui/hooks/use-selection.test.tsx index 742d6933..f01eafdd 100644 --- a/ui/hooks/use-selection.test.tsx +++ b/ui/hooks/use-selection.test.tsx @@ -57,6 +57,18 @@ vi.mock("./use-table-ui-state", () => ({ globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean } ).IS_REACT_ACT_ENVIRONMENT = true; +const testQueryProps = { + filter: { + after: "and" as const, + filters: [], + id: "root", + kind: "FilterGroup" as const, + }, + pageIndex: 0, + pageSize: 25, + sortOrder: [], +}; + function renderHarness(data: { rows: Record[] }) { const container = document.createElement("div"); document.body.appendChild(container); @@ -65,7 +77,7 @@ function renderHarness(data: { rows: Record[] }) { let latestState: ReturnType | undefined; function Harness() { - latestState = useSelection(data); + latestState = useSelection(data, testQueryProps); return null; } diff --git a/ui/hooks/use-selection.ts b/ui/hooks/use-selection.ts index 5188d57c..d3d18dc6 100644 --- a/ui/hooks/use-selection.ts +++ b/ui/hooks/use-selection.ts @@ -6,12 +6,16 @@ type QueryRows = { }; import { useActiveTableDelete } from "./use-active-table-delete"; +import type { UseActiveTableQueryProps } from "./use-active-table-query"; import { usePagination } from "./use-pagination"; import { useTableUiState } from "./use-table-ui-state"; -export function useSelection(data: QueryRows | undefined) { +export function useSelection( + data: QueryRows | undefined, + query: UseActiveTableQueryProps, +) { const { paginationState } = usePagination(); - const { mutate } = useActiveTableDelete(); + const { mutate } = useActiveTableDelete(query); const { scopeKey, tableUiState, updateTableUiState } = useTableUiState(); const rowSelectionState = useMemo( () => tableUiState?.rowSelectionState ?? {}, diff --git a/ui/studio/views/table/ActiveTableView.tsx b/ui/studio/views/table/ActiveTableView.tsx index cdf7f11b..25570e63 100644 --- a/ui/studio/views/table/ActiveTableView.tsx +++ b/ui/studio/views/table/ActiveTableView.tsx @@ -205,20 +205,39 @@ export function ActiveTableView(_props: ViewProps) { schemaVersion: sqlEditorSchema.version, }; }, [adapter, sqlEditorSchema.dialect, sqlEditorSchema.version]); + // Single source of truth for the active table query scope. The row mutation + // hooks (update/insert/delete) must receive these exact props so they target + // the same rows collection the grid displays — with infinite scroll enabled + // that is the grown `pageIndex: 0` window, not the paginated page. + const activeTableQueryProps = useMemo( + () => ({ + pageIndex: isInfiniteScrollEnabled ? 0 : paginationState.pageIndex, + pageSize: isInfiniteScrollEnabled + ? INFINITE_SCROLL_BATCH_SIZE * loadedInfinitePageCount + : paginationState.pageSize, + sortOrder: sortingState, + filter: appliedFilter, + searchScope: supportsFullTableSearch + ? ("row" as const) + : ("table" as const), + searchTerm: activeRowSearchTerm, + }), + [ + activeRowSearchTerm, + appliedFilter, + isInfiniteScrollEnabled, + loadedInfinitePageCount, + paginationState.pageIndex, + paginationState.pageSize, + sortingState, + supportsFullTableSearch, + ], + ); const { data, isFetching, refetch: refetchActiveTable, - } = useActiveTableQuery({ - pageIndex: isInfiniteScrollEnabled ? 0 : paginationState.pageIndex, - pageSize: isInfiniteScrollEnabled - ? INFINITE_SCROLL_BATCH_SIZE * loadedInfinitePageCount - : paginationState.pageSize, - sortOrder: sortingState, - filter: appliedFilter, - searchScope: supportsFullTableSearch ? "row" : "table", - searchTerm: activeRowSearchTerm, - }); + } = useActiveTableQuery(activeTableQueryProps); const [stableInfiniteData, setStableInfiniteData] = useState<{ data: NonNullable; key: string; @@ -284,7 +303,7 @@ export function ActiveTableView(_props: ViewProps) { isSelecting, rowSelectionState, setRowSelectionState, - } = useSelection(visibleData); + } = useSelection(visibleData, activeTableQueryProps); const { streams } = useStreams(); const { tableUiState, updateTableUiState } = useTableUiState({ editingFilter, @@ -543,8 +562,8 @@ export function ActiveTableView(_props: ViewProps) { (column) => column.pkPosition != null, ); const isInserting = useIsInserting(); - const insert = useActiveTableInsert(); - const updateMany = useActiveTableUpdateMany(); + const insert = useActiveTableInsert(activeTableQueryProps); + const updateMany = useActiveTableUpdateMany(activeTableQueryProps); const pageCount = getPageCount( visibleData?.filteredRowCount ?? Infinity, paginationState.pageSize, From c56ccc0c1e21a6696ed53102d7033a7a2140795e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Bramer=20Schmidt?= Date: Sat, 18 Jul 2026 15:13:49 +0700 Subject: [PATCH 2/2] fix: address PR #1545 review feedback - useActiveTableUpdate: drop the ignored AdapterUpdateOptions/table params from UseActiveTableUpdateParams. Persistence is delegated to the rows collection's onUpdate handler, so a per-call options channel could never reach the adapter; removing it makes that explicit instead of silently ignoring the values (API migration; the hook has no product callers). - ActiveTableView: keep row mutations targeting the visible window during infinite-scroll growth. While a grown window is fetching, the grid keeps showing the previous settled window; mutation query props now stay pinned to that settled window and swap to the grown scope atomically together with the rows (new resolveVisibleTableWindow helper + tests), closing the save/delete race during the transition. Co-Authored-By: Claude Fable 5 --- Architecture/db-state.md | 7 ++ ui/hooks/use-active-table-update.ts | 16 ++-- ui/studio/views/table/ActiveTableView.tsx | 66 ++++++++------ ui/studio/views/table/infinite-scroll.test.ts | 87 +++++++++++++++++++ ui/studio/views/table/infinite-scroll.ts | 62 +++++++++++++ 5 files changed, 202 insertions(+), 36 deletions(-) diff --git a/Architecture/db-state.md b/Architecture/db-state.md index ccd91eb8..d147a67b 100644 --- a/Architecture/db-state.md +++ b/Architecture/db-state.md @@ -198,6 +198,13 @@ deriving mutation scope independently (for example from `usePagination`) makes `collection.update`/`collection.delete` silently miss rows loaded beyond the first batch. +While a grown infinite-scroll window is still fetching, the grid keeps showing +the previous settled window. During that transition the view MUST keep the +mutation query props pinned to the settled window too: +`resolveVisibleTableWindow` pairs the visible rows with the query props of the +scope they were loaded from, and both swap to the grown window atomically once +its query finishes. + ## Lifecycle Rules Studio context owns collection lifecycle: diff --git a/ui/hooks/use-active-table-update.ts b/ui/hooks/use-active-table-update.ts index 3c1f506c..18398071 100644 --- a/ui/hooks/use-active-table-update.ts +++ b/ui/hooks/use-active-table-update.ts @@ -1,17 +1,17 @@ import { useMutation } from "@tanstack/react-query"; -import type { - AdapterUpdateDetails, - AdapterUpdateOptions, -} from "../../data/adapter"; import { useActiveTableQueryCollection, type UseActiveTableQueryProps, } from "./use-active-table-query"; +// Persistence is delegated to the rows collection's `onUpdate` handler, which +// owns the `adapter.update` call. There is intentionally no per-call +// `AdapterUpdateOptions` channel here: options passed at this level could not +// reach the adapter and would be silently ignored. export interface UseActiveTableUpdateParams { - details: AdapterUpdateDetails; - options: AdapterUpdateOptions; + changes: Record; + row: Record; } export function useActiveTableUpdate(query: UseActiveTableQueryProps) { @@ -25,14 +25,14 @@ export function useActiveTableUpdate(query: UseActiveTableQueryProps) { return useMutation({ mutationFn: async (params: UseActiveTableUpdateParams) => { - const rowId = String(params.details.row.__ps_rowid ?? ""); + const rowId = String(params.row.__ps_rowid ?? ""); if (!collection || !activeTable || !rowId) { throw new Error("Active table collection is not available"); } const transaction = collection.update(rowId, (draft) => { - Object.assign(draft, params.details.changes); + Object.assign(draft, params.changes); }); await transaction.isPersisted.promise; diff --git a/ui/studio/views/table/ActiveTableView.tsx b/ui/studio/views/table/ActiveTableView.tsx index 25570e63..62f6d3c5 100644 --- a/ui/studio/views/table/ActiveTableView.tsx +++ b/ui/studio/views/table/ActiveTableView.tsx @@ -106,6 +106,7 @@ import { import { getNextInfinitePageRowTarget, INFINITE_SCROLL_BATCH_SIZE, + resolveVisibleTableWindow, } from "./infinite-scroll"; import { InlineTableFilterAddButton, @@ -205,10 +206,11 @@ export function ActiveTableView(_props: ViewProps) { schemaVersion: sqlEditorSchema.version, }; }, [adapter, sqlEditorSchema.dialect, sqlEditorSchema.version]); - // Single source of truth for the active table query scope. The row mutation - // hooks (update/insert/delete) must receive these exact props so they target - // the same rows collection the grid displays — with infinite scroll enabled - // that is the grown `pageIndex: 0` window, not the paginated page. + // Single source of truth for the active table query scope — with infinite + // scroll enabled that is the grown `pageIndex: 0` window, not the paginated + // page. The row mutation hooks (update/insert/delete) receive the visible + // window's query props derived from this below, so they always target the + // rows collection the grid displays. const activeTableQueryProps = useMemo( () => ({ pageIndex: isInfiniteScrollEnabled ? 0 : paginationState.pageIndex, @@ -241,28 +243,34 @@ export function ActiveTableView(_props: ViewProps) { const [stableInfiniteData, setStableInfiniteData] = useState<{ data: NonNullable; key: string; + queryProps: typeof activeTableQueryProps; } | null>(null); - const visibleData = useMemo(() => { - if (!isInfiniteScrollEnabled) { - return data; - } - - if ( - isFetching && - (data == null || data.rows.length === 0) && - stableInfiniteData?.key === infiniteScrollResetKey - ) { - return stableInfiniteData.data; - } - - return data; - }, [ - data, - infiniteScrollResetKey, - isFetching, - isInfiniteScrollEnabled, - stableInfiniteData, - ]); + // The visible window pairs the displayed rows with the query props of the + // scope they were loaded from. While a grown infinite-scroll window is + // fetching, both stay pinned to the previous settled window and swap over + // atomically once the grown query finishes, so saving or deleting during + // the transition still targets the collection holding the visible rows. + const visibleWindow = useMemo( + () => + resolveVisibleTableWindow({ + activeData: data, + activeQueryProps: activeTableQueryProps, + isFetching, + isInfiniteScrollEnabled, + resetKey: infiniteScrollResetKey, + stableWindow: stableInfiniteData, + }), + [ + activeTableQueryProps, + data, + infiniteScrollResetKey, + isFetching, + isInfiniteScrollEnabled, + stableInfiniteData, + ], + ); + const visibleData = visibleWindow.data; + const mutationQueryProps = visibleWindow.queryProps; useEffect(() => { if (!isInfiniteScrollEnabled || !data) { @@ -288,9 +296,11 @@ export function ActiveTableView(_props: ViewProps) { return { data, key: infiniteScrollResetKey, + queryProps: activeTableQueryProps, }; }); }, [ + activeTableQueryProps, data, infiniteScrollResetKey, isFetching, @@ -303,7 +313,7 @@ export function ActiveTableView(_props: ViewProps) { isSelecting, rowSelectionState, setRowSelectionState, - } = useSelection(visibleData, activeTableQueryProps); + } = useSelection(visibleData, mutationQueryProps); const { streams } = useStreams(); const { tableUiState, updateTableUiState } = useTableUiState({ editingFilter, @@ -562,8 +572,8 @@ export function ActiveTableView(_props: ViewProps) { (column) => column.pkPosition != null, ); const isInserting = useIsInserting(); - const insert = useActiveTableInsert(activeTableQueryProps); - const updateMany = useActiveTableUpdateMany(activeTableQueryProps); + const insert = useActiveTableInsert(mutationQueryProps); + const updateMany = useActiveTableUpdateMany(mutationQueryProps); const pageCount = getPageCount( visibleData?.filteredRowCount ?? Infinity, paginationState.pageSize, diff --git a/ui/studio/views/table/infinite-scroll.test.ts b/ui/studio/views/table/infinite-scroll.test.ts index 56538bdc..1465da45 100644 --- a/ui/studio/views/table/infinite-scroll.test.ts +++ b/ui/studio/views/table/infinite-scroll.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from "vitest"; import { getNextInfinitePageRowTarget, INFINITE_SCROLL_BATCH_SIZE, + resolveVisibleTableWindow, } from "./infinite-scroll"; describe("getNextInfinitePageRowTarget", () => { @@ -51,3 +52,89 @@ describe("getNextInfinitePageRowTarget", () => { ).toBeNull(); }); }); + +describe("resolveVisibleTableWindow", () => { + const previousWindowProps = { pageIndex: 0, pageSize: 25 }; + const grownWindowProps = { pageIndex: 0, pageSize: 50 }; + const previousWindowData = { + rows: Array.from({ length: 25 }, (_, i) => ({ id: i + 1 })), + }; + const grownWindowData = { + rows: Array.from({ length: 50 }, (_, i) => ({ id: i + 1 })), + }; + const resetKey = "public.users::25"; + + it("keeps rows and mutation scope pinned to the settled window while the grown window is fetching", () => { + // Saving or deleting mid-transition must target the collection that + // still holds the visible rows, not the empty grown collection. + const window = resolveVisibleTableWindow({ + activeData: { rows: [] }, + activeQueryProps: grownWindowProps, + isFetching: true, + isInfiniteScrollEnabled: true, + resetKey, + stableWindow: { + data: previousWindowData, + key: resetKey, + queryProps: previousWindowProps, + }, + }); + + expect(window.data).toBe(previousWindowData); + expect(window.queryProps).toBe(previousWindowProps); + }); + + it("swaps rows and mutation scope to the grown window together once its data arrives", () => { + const window = resolveVisibleTableWindow({ + activeData: grownWindowData, + activeQueryProps: grownWindowProps, + isFetching: false, + isInfiniteScrollEnabled: true, + resetKey, + stableWindow: { + data: previousWindowData, + key: resetKey, + queryProps: previousWindowProps, + }, + }); + + expect(window.data).toBe(grownWindowData); + expect(window.queryProps).toBe(grownWindowProps); + }); + + it("ignores a stable window from a different reset key", () => { + const window = resolveVisibleTableWindow({ + activeData: { rows: [] }, + activeQueryProps: grownWindowProps, + isFetching: true, + isInfiniteScrollEnabled: true, + resetKey, + stableWindow: { + data: previousWindowData, + key: "public.users::sorted", + queryProps: previousWindowProps, + }, + }); + + expect(window.data).toEqual({ rows: [] }); + expect(window.queryProps).toBe(grownWindowProps); + }); + + it("always uses the active window when infinite scroll is disabled", () => { + const window = resolveVisibleTableWindow({ + activeData: previousWindowData, + activeQueryProps: previousWindowProps, + isFetching: true, + isInfiniteScrollEnabled: false, + resetKey, + stableWindow: { + data: grownWindowData, + key: resetKey, + queryProps: grownWindowProps, + }, + }); + + expect(window.data).toBe(previousWindowData); + expect(window.queryProps).toBe(previousWindowProps); + }); +}); diff --git a/ui/studio/views/table/infinite-scroll.ts b/ui/studio/views/table/infinite-scroll.ts index f8e3d11f..c3f6c840 100644 --- a/ui/studio/views/table/infinite-scroll.ts +++ b/ui/studio/views/table/infinite-scroll.ts @@ -1,5 +1,67 @@ export const INFINITE_SCROLL_BATCH_SIZE = 25; +export interface VisibleTableWindow { + data: TData | undefined; + queryProps: TQueryProps; +} + +/** + * Resolves which table window is actually visible: the active query scope, or + * the previously settled infinite-scroll window that stays on screen while a + * grown window is still fetching. + * + * The returned `data` and `queryProps` always belong to the same query scope. + * Row mutations must use the returned `queryProps`, so that saving or deleting + * during an infinite-scroll window transition still targets the collection + * that contains the rows the user is looking at, instead of the grown + * collection that has not finished loading yet. + */ +export function resolveVisibleTableWindow< + TData extends { rows: unknown[] }, + TQueryProps, +>(args: { + activeData: TData | undefined; + activeQueryProps: TQueryProps; + isFetching: boolean; + isInfiniteScrollEnabled: boolean; + resetKey: string; + stableWindow: { + data: TData; + key: string; + queryProps: TQueryProps; + } | null; +}): VisibleTableWindow { + const { + activeData, + activeQueryProps, + isFetching, + isInfiniteScrollEnabled, + resetKey, + stableWindow, + } = args; + const activeWindow = { + data: activeData, + queryProps: activeQueryProps, + }; + + if (!isInfiniteScrollEnabled) { + return activeWindow; + } + + if ( + isFetching && + (activeData == null || activeData.rows.length === 0) && + stableWindow?.key === resetKey + ) { + return { + data: stableWindow.data, + queryProps: stableWindow.queryProps, + }; + } + + return activeWindow; +} + export function getNextInfinitePageRowTarget(args: { hasMoreInfiniteRows: boolean; isInfiniteScrollEnabled: boolean;