fix(table-core): include defaultColumn in memoDeps for column, header, and sizing computations - #6587
fix(table-core): include defaultColumn in memoDeps for column, header, and sizing computations#6587lx3133584 wants to merge 1 commit into
Conversation
…, and sizing computations Co-Authored-By: Claude Code <noreply@anthropic.com> Signed-off-by: Liang Xu <lx3133584@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (8)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe change adds ChangesDefault column memoization
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Dynamic defaultColumn updates now refresh derived column, header, visibility, sizing, and offset values, including rendered sizes. Regression coverage verifies the updated metadata and sizing behavior, with no remaining merge-blocking risk identified. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This has been proposed a few times lately, but I think it would be too large of a breaking change and hurt performance for most tables, especially in react. This means that it would be expected for everyone to give their defaultColumn option a stable reference with useMemo, otherwise, all the calculations would be constantly re-firing |
Fixes #5275
🎯 Changes
When a table is constructed,
constructColumnmergesdefaultColumninto each column'scolumnDef. However, the memoized APIs across core features:table_getAllColumnstable_getAllFlatColumnstable_getAllFlatColumnsByIdtable_getAllLeafColumnscolumn_getFlatColumnscolumn_getLeafColumnstable_getHeaderGroupsheader_getLeafHeadersheader_getContextcolumn_getSizeheader_getSizeheader_getStarttable_getColumnOffsetscolumn_getIsVisibletable_getVisibleFlatColumnstable_getVisibleLeafColumnsonly included
table.options.columnsin theirmemoDeps, omittingtable.options.defaultColumn.As a result, when
defaultColumnwas updated dynamically viatable.setOptions(...)(such as in an effect or upon state changes), the column tree, headers, and sizing were not invalidated and continued to return stale definitions and sizes.This PR adds
defaultColumntomemoDepsacrosscoreColumnsFeature,coreHeadersFeature,columnSizingFeature, andcolumnVisibilityFeature, and adds unit tests coveringdefaultColumnupdates viasetOptions.✅ Checklist
pnpm testandpnpm test:e2e, or these tests do not apply to this pull request.🚀 Release Impact
Summary by CodeRabbit
Bug Fixes
Tests