From fb58baaa6a14f98322515d1b95c67ede16175ab2 Mon Sep 17 00:00:00 2001 From: Brian Love Date: Fri, 5 Jun 2026 16:02:51 -0700 Subject: [PATCH] fix(bench): AG Grid comparator publishes post-filter row count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes B2 follow-up #5 residual #3 (MUI + TanStack landed in #156). AG Grid now reports filter-metadata → 750, filter-text → 500 (matching pretable) and stays 3000 on sort, verified in Chromium. Root cause: AG Grid renders its rows imperatively (outside React), so the bench settle loop records result_row_count the instant the filtered rows paint — but a normal setState re-render of the data-bench-result-row-count attribute lands a few frames later, after settle, so the bench captured the stale pre-filter count. (aria-rowcount confirmed AG Grid *does* filter correctly: 501/751.) MUI and TanStack didn't hit this because they're React-rendered — visible rows and the count update in the same commit. Fix: publish the count via onFilterChanged using flushSync, forcing the attribute update synchronously with the event so it lands inside the settle window. Also gate the interaction effect on a gridReady flag so a plan present at mount is applied once the grid API exists (and to make the behavior unit-testable). Adds a jsdom regression test for the published post-filter count. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/__tests__/ag-grid-adapter.test.tsx | 67 +++++++++++++++++++ apps/bench/src/ag-grid-adapter.tsx | 26 ++++++- 2 files changed, 90 insertions(+), 3 deletions(-) diff --git a/apps/bench/src/__tests__/ag-grid-adapter.test.tsx b/apps/bench/src/__tests__/ag-grid-adapter.test.tsx index 950a01e2..7591e91a 100644 --- a/apps/bench/src/__tests__/ag-grid-adapter.test.tsx +++ b/apps/bench/src/__tests__/ag-grid-adapter.test.tsx @@ -2,6 +2,7 @@ import { render, waitFor } from "@testing-library/react"; import { describe, expect, test } from "vitest"; import { AgGridAdapter } from "../ag-grid-adapter"; +import type { BenchInteractionPlan } from "../interaction-plan"; const dataset = { columns: [ @@ -14,6 +15,35 @@ const dataset = { ], }; +const statusDataset = { + columns: [ + { id: "id", header: "ID", wrap: false, widthPx: 80 }, + { id: "status", header: "Status", wrap: false, widthPx: 160 }, + ], + rows: [ + { id: "1", status: "running" }, + { id: "2", status: "stopped" }, + { id: "3", status: "running" }, + { id: "4", status: "idle" }, + ], +}; + +function filterPlan( + mode: "filter-metadata" | "filter-text", + filters: Record, +): BenchInteractionPlan { + return { + focusedRowId: null, + filters, + mode, + probeColumnId: Object.keys(filters)[0] ?? "", + resultRowCount: 0, + rows: [], + selectedRowId: null, + sort: null, + }; +} + describe("AgGridAdapter", () => { test("mounts and renders AG Grid public selectors", async () => { const { container } = render( @@ -28,4 +58,41 @@ describe("AgGridAdapter", () => { expect(container.querySelector(".ag-root-wrapper")).not.toBeNull(); }); }); + + test("publishes the post-filter row count, not the full dataset size", async () => { + // Mirror the bench: mount first, let the grid become ready, THEN apply the + // interaction plan. (The flushSync timing in the adapter is what makes the + // count land inside the bench's settle window in Chromium; this jsdom test + // guards the onFilterChanged wiring and that the count is published.) + const { container, rerender } = render( + , + ); + await waitFor(() => { + expect(container.querySelector(".ag-root-wrapper")).not.toBeNull(); + }); + + rerender( + , + ); + + // status === "running" matches 2 of 4 rows. Filtering is a pure + // client-side row-model operation in AG Grid (no layout required), so the + // displayed-row count must reflect the filter even in jsdom. + await waitFor(() => { + const section = container.querySelector( + '[data-benchmark-adapter="ag-grid"]', + ); + expect(section?.getAttribute("data-bench-result-row-count")).toBe("2"); + }); + }); }); diff --git a/apps/bench/src/ag-grid-adapter.tsx b/apps/bench/src/ag-grid-adapter.tsx index b90a753d..838547ec 100644 --- a/apps/bench/src/ag-grid-adapter.tsx +++ b/apps/bench/src/ag-grid-adapter.tsx @@ -1,10 +1,12 @@ -import { useEffect, useMemo, useRef } from "react"; +import { useEffect, useMemo, useRef, useState } from "react"; +import { flushSync } from "react-dom"; import { AgGridReact } from "ag-grid-react"; import { AllCommunityModule, ModuleRegistry, themeQuartz, type ColDef, + type FilterChangedEvent, type GridApi, type GridReadyEvent, } from "ag-grid-community"; @@ -78,6 +80,12 @@ export function AgGridAdapter({ const apiRef = useRef(null); const onUpdateApiReadyRef = useRef(onUpdateApiReady); const onAutosizeReadyRef = useRef(onAutosizeReady); + // Post-filter displayed-row count, published as data-bench-result-row-count. + const [resultRowCount, setResultRowCount] = useState(dataset.rows.length); + // `onGridReady` fires asynchronously after mount; gating the interaction + // effect on this flag re-applies a filter/sort plan that is already present + // at mount once the grid API exists, rather than silently skipping it. + const [gridReady, setGridReady] = useState(false); useEffect(() => { onUpdateApiReadyRef.current = onUpdateApiReady; @@ -94,6 +102,7 @@ export function AgGridAdapter({ const onGridReady = (event: GridReadyEvent) => { apiRef.current = event.api; + setGridReady(true); const apply: ApplyBenchUpdates = (patches) => { const updates = patches.map((p) => ({ ...p })); event.api.applyTransaction({ update: updates }); @@ -142,13 +151,13 @@ export function AgGridAdapter({ } api.setFilterModel(model); } - }, [interactionPlan, runKey]); + }, [interactionPlan, runKey, gridReady]); return (
@@ -164,6 +173,17 @@ export function AgGridAdapter({ columnDefs={columnDefs} rowHeight={ROW_HEIGHT} onGridReady={onGridReady} + onFilterChanged={(event: FilterChangedEvent) => { + // AG Grid renders rows imperatively, so the bench's settle loop + // records result_row_count the moment the filtered rows paint. + // A normal setState re-render lands a few frames later — after + // settle — so the bench would capture the stale pre-filter count. + // flushSync forces the attribute update synchronously with this + // event, landing it inside the settle window. + flushSync(() => + setResultRowCount(event.api.getDisplayedRowCount()), + ); + }} getRowId={(params) => String((params.data as { id: unknown }).id)} />