fix(bench): AG Grid comparator publishes post-filter row count#157
Merged
Conversation
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) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Vercel preview readyPreview: https://pretable-h8tcgpt1f-cacheplane.vercel.app Updated automatically by the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Completes B2 follow-up #5 residual #3. #156 fixed MUI + TanStack; this fixes the last comparator, AG Grid. All four adapters now report identical, correct post-filter counts.
Verification (Chromium, S2/hypothesis, n=2)
sortfilter-metadatafilter-textRoot cause (this one was subtle)
AG Grid initially reported 3000 even though it filters correctly. The investigation:
aria-rowcount(501 for filter-text, 751 for filter-metadata) and zero console warnings. The earlier "maybe it doesn't filter" hypothesis was wrong.onFilterChangedfires with the right count (500/750), andsetStatedid update thedata-bench-result-row-countattribute — but only after the bench's settle loop had already recorded the metric.result_row_countthe instant the filtered rows paint — but a normal ReactsetStatere-render lands a few frames later, after settle. MUI and TanStack avoid this because they're React-rendered, so their visible rows and the count update in the same commit.Fix
onFilterChangedviaflushSync, forcing the attribute update synchronously with the event so it lands inside the settle window.gridReadyflag so a plan present at mount is applied once the grid API exists (also makes the behavior unit-testable).Gates
typecheck/lintcleantest— 62 passed (incl. new AG Grid filter-count test)prettier --checkcleanresult_row_countis informational comparator evidence🤖 Generated with Claude Code