Skip to content

fix(table-core): flatten filtered parent rows ahead of their sub-rows - #6545

Open
waterWang wants to merge 3 commits into
TanStack:mainfrom
waterWang:fix/filter-rows-parent-first-flatrows
Open

fix(table-core): flatten filtered parent rows ahead of their sub-rows#6545
waterWang wants to merge 3 commits into
TanStack:mainfrom
waterWang:fix/filter-rows-parent-first-flatrows

Conversation

@waterWang

@waterWang waterWang commented Aug 12, 2026

Copy link
Copy Markdown

🎯 Changes

getFilteredRowModel().flatRows has the exact same post-order problem that
#6529 fixed for
getSortedRowModel().flatRows.

Root cause

Both filterRowModelFromLeafs (used when filterFromLeafRows: true) and
filterRowModelFromRoot (default) recurse into a row's sub-rows before
pushing the row itself to flatRows, so every descendant precedes its own
parent in the flattened list.

Fix

  • filterRowModelFromRoot: reserves the parent's flat-array slot before
    descending (same pattern as fix(table-core): flatten sorted parent rows ahead of their sub-rows #6529's createSortedRowModel), then replaces
    it with the cloned row when recursion returns.
  • filterRowModelFromLeafs: records the insertion point before recursion
    and splices the parent ahead of its surviving children when the parent is
    retained.

Before

table.getCoreRowModel().flatRows.map((r) => r.original.name)
// ['keep-a', 'keep-a1', 'drop-a1a', 'drop-a2', 'drop-b', 'keep-b1',
//  'keep-c', 'keep-d', 'drop-d1']

table.getFilteredRowModel().flatRows.map((r) => r.original.name)
// before: ['keep-a1', 'keep-a', 'keep-c', 'keep-d']
// after:  ['keep-a', 'keep-a1', 'keep-c', 'keep-d']

Checklist

  • filterRowModelFromLeafs fix
  • filterRowModelFromRoot fix
  • Update existing test expectation (the maxLeafRowFilterDepth: 1 test
    that explicitly documented the old post-order behaviour)
  • Add new tests for both modes

Closes #6536

Summary by CodeRabbit

  • Bug Fixes

    • Corrected filtered row ordering so parent rows consistently appear before their surviving sub-rows.
    • Aligned flat row results with the displayed filtered row hierarchy across all filtering modes.
  • Tests

    • Added coverage for hierarchical filtering and updated depth-limited filtering expectations.

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

getFilteredRowModel().flatRows now follows pre-order traversal in both root-first and leaf-first filtering. Tests cover hierarchical and depth-limited filtering, and a patch changeset documents the correction.

Changes

Filtered flat-row ordering

Layer / File(s) Summary
Filtering traversal order
packages/table-core/src/features/column-filtering/filterRowsUtils.ts
Both filtering modes reserve parent positions before recursion and replace those positions with the filtered parent rows.
Ordering validation and release note
packages/table-core/tests/implementation/features/column-filtering/createFilteredRowModel.test.ts, .changeset/great-pugs-sniff.md
Tests verify parent-first ordering for root-first, leaf-first, and depth-limited filtering. The changeset documents a patch release.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

Possibly related PRs

  • TanStack/table#6503 — Both PRs update filtering flattening logic and hierarchical filtering tests.
  • TanStack/table#6529 — Both PRs correct parent-before-child ordering in a row model’s flatRows.
  • TanStack/table#6541 — Both PRs update leaf-first filtering and related flatRows tests.

Suggested reviewers: kevinvandy

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the primary fix to flatten filtered parent rows before their sub-rows.
Description check ✅ Passed The description clearly explains the root cause, fix, tests, and affected filtering modes, with only minor template sections omitted.
Linked Issues check ✅ Passed The changes implement the linked issue objective by ordering parents before descendants in both filtering paths and adding coverage.
Out of Scope Changes check ✅ Passed The code, tests, and changeset directly support the linked issue and PR objectives without unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
packages/table-core/src/features/column-filtering/filterRowsUtils.ts (1)

68-84: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Avoid repeated splice operations in leaf-first filtering.

flatIndex points before the recursive output. Each splice(flatIndex, 0, row) shifts all surviving descendants. A chain of n rows can therefore perform O(n²) element moves.

Reserve the slot before recursion, replace it with newFilteredFlatRows[flatIndex] = row, and remove it when the parent and all descendants are discarded. This matches the root-first implementation and avoids repeated subtree shifts.

Proposed change
         const flatIndex = newFilteredFlatRows.length
+        newFilteredFlatRows.push(row)

         newRow.subRows = recurseFilterRows(row.subRows, depth + 1)
         row = newRow

         if (filterRow(row) && !newRow.subRows.length) {
-          newFilteredFlatRows.splice(flatIndex, 0, row)
+          newFilteredFlatRows[flatIndex] = row
           filteredRows.push(row)
           newFilteredRowsById[row.id] = row
           continue
         }

         if (filterRow(row) || newRow.subRows.length) {
-          newFilteredFlatRows.splice(flatIndex, 0, row)
+          newFilteredFlatRows[flatIndex] = row
           filteredRows.push(row)
           newFilteredRowsById[row.id] = row
           continue
         }

+        newFilteredFlatRows.pop()
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/table-core/src/features/column-filtering/filterRowsUtils.ts` around
lines 68 - 84, Update the leaf-first branch in recurseFilterRows to reserve
flatIndex before recursing, assign the parent with
newFilteredFlatRows[flatIndex] = row instead of splice, and remove the reserved
slot when the parent and all descendants are discarded. Preserve the existing
filteredRows and newFilteredRowsById updates while ensuring surviving
descendants retain their order without repeated shifts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/table-core/src/features/column-filtering/filterRowsUtils.ts`:
- Around line 68-84: Update the leaf-first branch in recurseFilterRows to
reserve flatIndex before recursing, assign the parent with
newFilteredFlatRows[flatIndex] = row instead of splice, and remove the reserved
slot when the parent and all descendants are discarded. Preserve the existing
filteredRows and newFilteredRowsById updates while ensuring surviving
descendants retain their order without repeated shifts.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2b28d6ec-f353-4f7b-806d-dd93abfc66d2

📥 Commits

Reviewing files that changed from the base of the PR and between 8b60ea1 and e33dbc5.

📒 Files selected for processing (3)
  • .changeset/great-pugs-sniff.md
  • packages/table-core/src/features/column-filtering/filterRowsUtils.ts
  • packages/table-core/tests/implementation/features/column-filtering/createFilteredRowModel.test.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

getFilteredRowModel().flatRows is post-order, same bug #6529 just fixed for getSortedRowModel

1 participant