fix(table-core): flatten sorted parent rows ahead of their sub-rows - #6529
Conversation
The sorted row model recursed into a row's sub-rows before pushing the row itself, so getSortedRowModel().flatRows came out in post-order: every descendant preceded its own parent. The core row model, the paginated row model and the depth-truncation helper in the filtered row model all flatten a parent first, and getSortedRowModel().rows is already in that order, so flatRows disagreed with its own rows. Reserve the row's slot before descending, and overwrite it when the branch is cloned so flatRows keeps the same row instance that rows holds.
|
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)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe sorted row model now keeps parent rows before descendants in ChangesSorted flat-row ordering
Estimated code review effort: 3 (Moderate) | ~25 minutes 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 |
|
View your CI Pipeline Execution ↗ for commit 726f21b
☁️ Nx Cloud last updated this comment at |
🎯 Changes
_createSortedRowModelrecurses into a row's sub-rows before pushing the row itself, sogetSortedRowModel().flatRowscomes out in post-order — every descendant precedes its own parent.With two parent rows that each have sub-rows, sorted ascending:
That disagrees with the model's own
rows, which are already in render order, and with how the rest of the pipeline flattens:createCoreRowModelpushes a row, then recurses into its sub-rows.createPaginatedRowModel'shandleRowdoes the same.addSubRowsToFlatArrays(added for themaxLeafRowFilterDepthcase in fix(column-filtering): include sub-rows in flatRows when maxLeafRowFilterDepth skips recursion #6395) is explicitly parent-first, to keep "the flat representation ... consistent with the visible tree".The fix takes the row's slot before descending, and overwrites that slot when the branch is cloned, so
flatRowsstill holds the same instance asrows— which the previoussortedFlatRows.push(cloned)guaranteed and a plainpushbefore recursion would have lost.Deliberately not included
filterRowModelFromLeafsandfilterRowModelFromRootflatten the same way, sogetFilteredRowModel().flatRowsis post-order too. I left those alone:column_getAutoSortFnandcolumn_getAutoFilterFnsamplegetFilteredRowModel().flatRows.slice(0, 10), so reordering that array changes which rows drive automatic fn detection (cf. #4908). That felt like a bigger behavioral call than this fix — happy to do it in a follow-up if you'd like it changed.✅ Checklist
pnpm run test:pr.pnpm run test:pris green —test:eslint, test:sherif, test:knip, test:lib, test:types, test:build, buildacross 408 projects, with@tanstack/table-core:test:libat 62/62 files and 1288/1288 tests.Two tests added to
createSortedRowModel.test.ts, both verified to fail onmainand pass with the fix:flattens each parent ahead of its own sub-rows— the ordering itself.flattens a branch clone rather than the row it replaced— pinsflatRows[0] === rows[0]so the clone, not the row it replaced, is what lands inflatRows.🚀 Release Impact
Summary by CodeRabbit
Bug Fixes
Tests