Open
Conversation
Identifies why enabling experimentalMemoization in arcadia QuotasTable3 yielded no measurable change: inline rowAttributes, custom context fanout in TreeExpandableCell, and MemoBaseCell exported but never wired into BaseRow. Plans a coordinated library + consumer fix plus dev-only warning hook and migration doc. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ation Add an internal `Cell` prop to `BaseRow` (defaults to `BaseCell`) and thread `MemoBaseCell` through `MemoBaseRow` so that cell-level memoization is active when `experimentalMemoization` is enabled. Adds a regression test that asserts cell renderers do not re-run when only the row className changes. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…perimentalMemoization
Spec for a one-line BaseCell change (useContext(RowStateContext)) that makes row.getIsExpanded() / row.getIsSelected() memo-safe inside cell render fns, removing the need for consumers to import useIsExpanded. Includes a currently-red regression test that proves the bug exists today and will turn green once the fix lands. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ession test" This reverts commit c592628.
Library change is doc-only: rewrite migration guide to lead with the TreeExpandableCell component (already exported, already encapsulates useIsExpanded internally). Consumer drops their local chevron component and uses the library's directly. useIsExpanded remains exported as the escape hatch for custom row-state-aware UI. Supersedes the rejected auto-subscribe approach (over-rendered cells of the toggled row). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rewrite anti-pattern #3 and the worked example to lead with the library's TreeExpandableCell component. useIsExpanded is demoted to an advanced 'Building your own analogue' sub-section for consumers who need custom chevron styling. row.getIsExpanded() inside a memoized cell remains an explicit anti-pattern.
Thread row state into areCellPropsEqual so MemoBaseCell re-renders when expansion or selection changes. Consumers can now call row.getIsExpanded() directly in any cell render function under experimentalMemoization without useIsExpanded.
Pass isExpanded/isSelected to all Cell call sites so MemoBaseCell's comparator can detect row state changes without a context subscription. Delete RowStateContext.ts and migrate all callers (BaseGroupHeader, TreeExpandableCell, stories) to row.getIsExpanded() directly.
Remove useIsExpanded from all guidance — it no longer exists. row.getIsExpanded() now works correctly in any cell render fn under experimentalMemoization. Remove the 'Building your own analogue' sub-section and update anti-pattern #2 Fix and the 'What memoization does not fix' bullet accordingly.
Replaces the closed-world isSelected/isExpanded lifting with a user-controlled getRowVersion(row) that returns a snapshot array. Cells become standard TanStack code; users control which row state slices participate in memoization. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Step-by-step plan derived from the design spec, with TDD scaffolding for the new arraysShallowEqual helper and behavioral tests for default and custom getRowVersion. Documents the atomic refactor across BaseRow, BaseCell, BaseDraggableRow comparators and the BaseTable wiring. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
… + _rowVersion The MemoBaseRow / MemoBaseDraggableRow / MemoBaseCell comparators now short-circuit on arraysShallowEqual(_rowVersion). BaseTable computes the version per row from getRowVersion (default: [getIsSelected, getIsExpanded]) and passes it down. The previously-explicit isSelected/isExpanded props are removed from BaseRowProps, BaseCellProps, MemoBaseRowProps, and MemoBaseDraggableRowProps; BaseRow reads row state inline.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Preview is ready. |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…table into fix/tree-rerender-v2
added 2 commits
April 29, 2026 18:46
beliarh
reviewed
Apr 30, 2026
| prev.attributes === next.attributes && | ||
| prev.cellAttributes === next.cellAttributes && | ||
| prev.rowVirtualizer === next.rowVirtualizer && | ||
| prev['aria-rowindex'] === next['aria-rowindex'] && |
Collaborator
There was a problem hiding this comment.
In MemoBaseRow aria-rowindex was removed from the comparator, but here it's still compared. Should the logic be the same in both components?
|
|
||
| const getRowId = (row: Item) => row.id; | ||
|
|
||
| describe('MemoBaseRow + MemoBaseCell wiring', () => { |
Collaborator
There was a problem hiding this comment.
Could you add tests for renderGroupHeaderRowContent and renderCustomRowContent with experimentalMemoization?
I'm not sure if memoization actually works inside these slots.
| | ((row: Row<TData>) => React.HTMLAttributes<HTMLTableRowElement>); | ||
| cellAttributes?: BaseCellProps<TData>['attributes']; | ||
| Cell?: React.FunctionComponent<BaseCellProps<TData>>; | ||
| _rowVersion?: readonly unknown[]; |
Collaborator
There was a problem hiding this comment.
Why this prop is not @internal? Ideally _rowVersion should be excluded from the public interfaces entirely.
| props: BaseRowProps<TData, TScrollElement> & {ref?: React.Ref<HTMLTableRowElement>}, | ||
| ) => React.ReactElement; | ||
|
|
||
| export const MemoBaseRow = React.memo(BaseRowWithMemoCell, areEqual) as < |
Collaborator
There was a problem hiding this comment.
Why don't the new memoized components have displayName?
| @@ -0,0 +1,151 @@ | |||
| // src/components/BaseRow/__tests__/BaseRow.memo.test.tsx | |||
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
3 participants
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.
No description provided.