Uh oh!
There was an error while loading. Please reload this page.
fix(table-core): skip onRowSelectionChange on a no-op toggleAllRowsSelected - #6542
fix(table-core): skip onRowSelectionChange on a no-op toggleAllRowsSelected#6542lazerg wants to merge 1 commit into
Conversation
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthrough
ChangesRow selection no-op behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
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 |
🎯 Changes
table.toggleAllRowsSelected(true)rebuilds the whole selection map and callsonRowSelectionChangeeven when every selectable row is already selected, andtoggleAllRowsSelected(false)does the same when nothing is selected. Consumers that mirrorrowSelectioninto a URL or onto a server get a redundant write on every no-op toggle.toggleAllRowsExpandedgrew the equivalent guards in #6501, and #6532 kept those O(1) checks on purpose while leaving the generalsetRowSelectionunguarded. This adds the matching pair: return early when selection is requested andgetIsAllRowsSelected()is already true, and when deselection is requested and nothing is selected. Both read memoized getters, so the row-scaled map comparison thatsetRowSelectiondeliberately avoids stays out of the picture.Three adapter tests drove their second
onRowSelectionChangethroughtoggleAllRowsSelected(false)with a mock handler that never writes state back, leaving the selection empty, so that call is now a no-op. They toggle selection on instead.Fixes#6538
✅ Checklist
pnpm run test:pr.Green across all 409 projects except
@tanstack/ember-table:test:lib, whose testem browser will not start on my machine. It fails the same way on an unmodifiedmain.🚀 Release Impact
Summary by CodeRabbit
Bug Fixes
Tests