fix: Cache measured column widths to avoid getBoundingClientRect during render#4144
Conversation
| }; | ||
| } | ||
| if (sticky) { | ||
| // Use cached measured width to avoid getBoundingClientRect() during render. |
There was a problem hiding this comment.
What is the cost of calling getBoundingClientRect() during render exactly?
There was a problem hiding this comment.
Calling getBoundingClientRect() forces the browser to recompute layout. Calling it multiple times in a row is fine, since (if nothing in the DOM has changed) the layout computations are cached. But calling it in a React render is unpredictable: React may or may not batch different component renders together, so it's possible for the DOM state to change in between different getBoundingClientRect() calls, causing layout thrashing. It's also considered generally bad practice to perform expensive operations that block React updates.
The key change here is that all of the getBoundingClientRect() calls for the table are batched together to minimize potential overhead, and performed outside of render so they don't block React.
Note: This branch includes #4142 and #4143. Those PRs should be merged first.
Description
I noticed that
getBoundingClientRectwas being called directly inuseColumnWidths, which adds significant overhead to table renders. With this change,getBoundingClientRectis only called duringupdateColumnWidths, which is called outside of the React render flow. This should make table updates smoother.How has this been tested?
Since this is a render performance issue, it's difficult to test directly. I'd welcome suggestions.
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.