diff --git a/packages/dashboard-ui-components/src/components/data-grid/use-data-source.test.tsx b/packages/dashboard-ui-components/src/components/data-grid/use-data-source.test.tsx index 245b61118..4567ec8f1 100644 --- a/packages/dashboard-ui-components/src/components/data-grid/use-data-source.test.tsx +++ b/packages/dashboard-ui-components/src/components/data-grid/use-data-source.test.tsx @@ -3,7 +3,7 @@ import { act, cleanup, fireEvent, render, waitFor } from "@testing-library/react"; import { afterEach, describe, expect, it } from "vitest"; import { useDataSource } from "./use-data-source"; -import type { DataGridColumnDef, DataGridDataSource } from "./types"; +import type { DataGridColumnDef, DataGridDataPaginationMode, DataGridDataSource } from "./types"; type Row = { id: string, @@ -19,7 +19,13 @@ const columns: DataGridColumnDef[] = [ }, ]; -function DataSourceHarness({ dataSource }: { dataSource: DataGridDataSource }) { +function DataSourceHarness({ + dataSource, + paginationMode = "infinite", +}: { + dataSource: DataGridDataSource, + paginationMode?: DataGridDataPaginationMode, +}) { const gridData = useDataSource({ dataSource, columns, @@ -27,13 +33,14 @@ function DataSourceHarness({ dataSource }: { dataSource: DataGridDataSource sorting: [], quickSearch: "", pagination: { pageIndex: 0, pageSize: 25 }, - paginationMode: "infinite", + paginationMode, }); return ( <> {gridData.rows.length} + {gridData.rows[0]?.name ?? ""} {gridData.error?.message ?? ""} ); @@ -133,6 +140,49 @@ describe("useDataSource infinite pagination", () => { expect(callsA).toEqual([{ cursor: undefined }, { cursor: "cursor-1" }]); }); + it("does not treat an aborted zero-yield reset as a completed fetch", async () => { + const dataSourceA: DataGridDataSource = async function* () { + yield { + rows: [{ id: "row-a", name: "A" }], + nextCursor: null, + hasMore: false, + }; + }; + let bCalls = 0; + let resolveFirstB: (value: void) => void = () => {}; + const dataSourceB: DataGridDataSource = async function* () { + bCalls++; + if (bCalls === 1) { + // Aborted before any yield: completing this must not mark B as a + // successful skip key, or a later render with B would keep A's rows. + await new Promise((resolve) => { + resolveFirstB = resolve; + }); + return; + } + yield { + rows: [{ id: "row-b", name: "B" }], + nextCursor: null, + hasMore: false, + }; + }; + + const { getByTestId, rerender } = render( + , + ); + await waitFor(() => expect(getByTestId("row-name").textContent).toBe("A")); + + rerender(); + rerender(); + await act(async () => { + resolveFirstB(); + }); + + rerender(); + await waitFor(() => expect(getByTestId("row-name").textContent).toBe("B")); + expect(bCalls).toBe(2); + }); + it("does not replay a deferred loadMore after the in-flight fetch errors", async () => { const calls: Array<{ cursor: unknown }> = []; let rejectInitial: (err: Error) => void = () => {}; @@ -162,6 +212,53 @@ describe("useDataSource infinite pagination", () => { expect(calls).toHaveLength(1); }); + it("discards a deferred loadMore when pagination mode leaves infinite", async () => { + const calls: Array<{ cursor: unknown }> = []; + let resolveInitial: (value: void) => void = () => {}; + const initialFetch = new Promise((resolve) => { + resolveInitial = resolve; + }); + const dataSource: DataGridDataSource = async function* (params) { + calls.push({ cursor: params.cursor }); + if (calls.length === 1) { + await initialFetch; + yield { + rows: [{ id: "row-1", name: "Row 1" }], + nextCursor: "cursor-1", + hasMore: true, + }; + return; + } + yield { + rows: [{ id: "row-2", name: "Row 2" }], + nextCursor: null, + hasMore: false, + }; + }; + + const { getByRole, rerender } = render( + , + ); + await waitFor(() => expect(calls).toHaveLength(1)); + + fireEvent.click(getByRole("button", { name: "Load more" })); + expect(calls).toHaveLength(1); + + rerender(); + + await act(async () => { + resolveInitial(); + }); + + // Mode left infinite while the deferred loadMore was queued — do not + // append against server pagination after the fetch settles. + await waitFor(() => expect(calls).toHaveLength(1)); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 20)); + }); + expect(calls).toHaveLength(1); + }); + it("uses the completed page cursor for the next loadMore fetch", async () => { const calls: Array<{ cursor: unknown }> = []; const dataSource: DataGridDataSource = async function* (params) { diff --git a/packages/dashboard-ui-components/src/components/data-grid/use-data-source.ts b/packages/dashboard-ui-components/src/components/data-grid/use-data-source.ts index 2865e3658..3939e239c 100644 --- a/packages/dashboard-ui-components/src/components/data-grid/use-data-source.ts +++ b/packages/dashboard-ui-components/src/components/data-grid/use-data-source.ts @@ -1,4 +1,5 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { runAsynchronously } from "@hexclave/shared/dist/utils/promises"; import type { DataGridColumnDef, DataGridDataSource, @@ -131,12 +132,13 @@ function useAsyncDataSource(opts: { // fetch settles. const pendingLoadMoreRef = useRef(false); const hasMoreRef = useRef(true); + const paginationModeRef = useRef(paginationMode); + paginationModeRef.current = paginationMode; const pageIndexRef = useRef(0); const hasDataRef = useRef(false); const hasMountedServerPaginationRef = useRef(false); - // Effects can be replayed when a descendant suspends. Only a completed - // fetch is safe to reuse; an aborted fetch may have reset the cursor without - // ever replacing the visible rows. + // Effects can be replayed when a descendant suspends. Only a fetch that + // actually delivered results is safe to reuse as a skip key. const lastCompletedNonAppendFetchKeyRef = useRef<{ dataSource: DataGridDataSource, sortingKey: string, @@ -203,6 +205,11 @@ function useAsyncDataSource(opts: { let cursor = append ? cursorRef.current : undefined; let pageIndex = append ? pageIndexRef.current : 0; let failed = false; + // The abort guard lives inside the for-await body, so a generator that + // completes with zero yields never hits it and would otherwise fall + // through to the post-loop completion-key write. Only mark a reset + // complete after it has actually delivered (and committed) a result. + let committedResult = false; try { const params: DataGridFetchParams = { @@ -246,10 +253,11 @@ function useAsyncDataSource(opts: { } hasDataRef.current = true; + committedResult = true; pageIndex++; pageIndexRef.current = pageIndex; } - if (!append) { + if (!append && committedResult && !controller.signal.aborted) { lastCompletedNonAppendFetchKeyRef.current = fetchKey; } } catch (err) { @@ -272,19 +280,21 @@ function useAsyncDataSource(opts: { setIsRefetching(false); setIsLoadingMore(false); // Replay a loadMore that was requested (and deferred) while this - // fetch was in flight — but only if this fetch succeeded. Skip on - // failure (the append would run against inconsistent state and its - // setError(null) would hide this fetch's error) and on - // unmount-cleanup aborts — `inFlightFetchRef.current === controller` - // with an aborted signal only happens then. + // fetch was in flight — but only if this fetch succeeded while we + // are still in infinite mode. Skip on failure (the append would run + // against inconsistent state and its setError(null) would hide this + // fetch's error), when pagination mode changed away from infinite, + // and on unmount-cleanup aborts — `inFlightFetchRef.current === + // controller` with an aborted signal only happens then. const shouldChainLoadMore = pendingLoadMoreRef.current && !failed && !controller.signal.aborted - && hasMoreRef.current; + && hasMoreRef.current + && paginationModeRef.current === "infinite"; pendingLoadMoreRef.current = false; if (shouldChainLoadMore) { - fetchPage(true).catch(() => {}); + runAsynchronously(fetchPage(true)); } } } @@ -292,6 +302,14 @@ function useAsyncDataSource(opts: { [], ); + useEffect(() => { + // Deferred loadMore is only meaningful for infinite scroll; drop it when + // the consumer leaves that mode so a later settle cannot append wrongly. + if (paginationMode !== "infinite") { + pendingLoadMoreRef.current = false; + } + }, [paginationMode]); + useEffect(() => { const lastCompletedKey = lastCompletedNonAppendFetchKeyRef.current; const isSameCompletedFetch = hasDataRef.current @@ -300,7 +318,7 @@ function useAsyncDataSource(opts: { && lastCompletedKey.quickSearchKey === quickSearchKey && lastCompletedKey.pageSize === pagination.pageSize; if (!isSameCompletedFetch) { - fetchPage(false).catch(() => {}); + runAsynchronously(fetchPage(false)); } // Always return the abort cleanup (even when the fetch is skipped) so an // in-flight append is cancelled on unmount. @@ -319,7 +337,7 @@ function useAsyncDataSource(opts: { hasMountedServerPaginationRef.current = true; return; } - fetchPage(false).catch(() => {}); + runAsynchronously(fetchPage(false)); }, [fetchPage, paginationMode, pagination.pageIndex]); const loadMore = useCallback(() => { @@ -333,11 +351,11 @@ function useAsyncDataSource(opts: { pendingLoadMoreRef.current = true; return; } - fetchPage(true).catch(() => {}); + runAsynchronously(fetchPage(true)); }, [hasMore, paginationMode, fetchPage]); const reload = useCallback(() => { - fetchPage(false).catch(() => {}); + runAsynchronously(fetchPage(false)); }, [fetchPage]); return {