fix: Avoid redundant table column measurements on re-render#4609
Open
TrevorBurnham wants to merge 1 commit into
Open
fix: Avoid redundant table column measurements on re-render#4609TrevorBurnham wants to merge 1 commit into
TrevorBurnham wants to merge 1 commit into
Conversation
Memoize the visible column definitions and the width/id arrays derived from them in InternalTable. These were rebuilt as fresh arrays on every render, so the reference passed to ColumnWidthsProvider always changed. That re-triggered the provider's width-sync effect, which calls getBoundingClientRect() on every render even when columns are unchanged (e.g. typing in the filter box), contributing to interaction lag. With the columns memoized, the effect only re-runs when the columns actually change. Adds tests asserting that re-rendering with unchanged columns performs no DOM measurements, while changing columns still does.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4609 +/- ##
=======================================
Coverage 97.50% 97.50%
=======================================
Files 948 948
Lines 30275 30317 +42
Branches 11039 11046 +7
=======================================
+ Hits 29520 29562 +42
Misses 748 748
Partials 7 7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
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.
This PR reduces unnecessary DOM measurements when a table with "sticky" features (
stickyColumnsorstickyHeader) re-renders.ColumnWidthsProviderruns auseEffectthat depends on itsvisibleColumnsarray. That effect callsupdateColumnWidths(), which synchronizes sticky cells by reading layout from the DOM viagetBoundingClientRect()(thestickybranch ofgetColumnStyles). InInternalTable, the arrays passed to the provider —visibleColumnDefinitionsand the derivedvisibleColumnWidthsWithSelection/visibleColumnIdsWithSelection— are rebuilt as fresh arrays of fresh objects on every render, causing the provider's effect to re-fire. These unnecessary calculations contribute to input lag when users type in a sticky table's searchbox.This PR memoizes those derivations in
InternalTablewithuseMemo:visibleColumnDefinitions— keyed oncolumnDefinitions,columnDisplay, andvisibleColumns.visibleColumnWidthsWithSelection/visibleColumnIdsWithSelection— keyed onhasSelectionandvisibleColumnDefinitions.Benefits
Measured
getBoundingClientRectcalls per re-render with unchanged columns:stickyColumnsstickyHeaderThe remaining reads come from
StickyHeader's own per-render synchronization, a separate mechanism this PR does not change (a candidate for a follow-up).Non-sticky tables benefit in a smaller way from reduced array allocations.
How has this been tested?
src/table/__tests__/columns-width.test.tsx:does not measure column widths when re-rendering with unchanged columns— re-renders with newitemsbut identicalcolumnDefinitionsand asserts that nogetBoundingClientRectcalls occur. This test fails onmain(measurements still happen) and passes with this change.does measure column widths when the columns change— a negative control that re-renders with an added column and asserts measurement still occurs, confirming the first test isn't trivially passing.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.