Stabilize ListBoard row press callback to avoid full re-renders on open#651
Open
wyne wants to merge 1 commit into
Open
Stabilize ListBoard row press callback to avoid full re-renders on open#651wyne wants to merge 1 commit into
wyne wants to merge 1 commit into
Conversation
On first open the board layout settles in multiple onLayout passes. Each setBoardLayout recreated handleRowPress (boardLayout was in its dep array), which changed the onRowPress prop on every MemoizedPlayerRow and forced the entire row list to re-render. Profiling showed two full ListBoard subtree re-renders (~1205 fibers each, ~280ms combined) right after mount. Read the latest layout from a ref inside handleRowPress so the callback keeps a stable identity across layout passes; rows now bail out of React.memo. The boardLayout state is retained only for rendering DialOverlay's dimensions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
Coverage after merging claude/brave-mccarthy-5192d0 into main will be
Coverage Report |
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.
Problem
Profiling a first-open of the ListBoard (React DevTools) showed two full re-renders of the entire ListBoard subtree right after mount:
That's ~280ms of avoidable work (~50 player rows × ~24 elements each, re-rendered twice).
Root cause
handleRowPresswas auseCallbackwithboardLayoutin its dependency array. On first open the container's layout settles in multipleonLayoutpasses (initial mount, then again after the GameSheet bottom sheet mounts and changes available height). EachsetBoardLayout:handleRowPresswith a new identity,onRowPressprop on everyMemoizedPlayerRow,React.memoand forcing all rows to re-render — twice.The rows were already correctly memoized; the unstable callback was busting it.
Fix
Mirror
boardLayoutinto auseRefand readboardLayoutRef.currentinsidehandleRowPress, removingboardLayoutfrom its dependency array. The callback now keeps a stable identity across layout passes, so the rows bail out ofReact.memo. TheboardLayoutstate is retained only for renderingDialOverlay's dimensions (fresh at press time, so rotation still works).Each layout pass now re-renders only
ListBoarditself (a handful of fibers) instead of the full ~1205-fiber subtree.Testing
ListBoard.test.tsxtests pass (including the row-press → DialOverlay path this change touches).tsc --noEmitclean.🤖 Generated with Claude Code