fix(table-core): include sub-rows in flatRows when maxLeafRowFilterDepth truncates recursion#6361
Conversation
…pth truncates recursion When filterRowModelFromRoot skips recursing into sub-rows because the current depth >= maxLeafRowFilterDepth, the parent row's sub-rows are still accessible through row.subRows and remain visible in the table. However, these sub-rows were never added to newFilteredFlatRows, making flatRows an incomplete flat representation of the visible tree. This causes getFacetedUniqueValues() to return different counts before and after filtering when maxLeafRowFilterDepth is set: before filtering flatRows contains all rows at all depths (via preRowModel), but after filtering flatRows only contains passing top-level rows. Fix: add addSubRowsToFlatArrays helper that recursively pushes sub-rows (and their descendants) into flatRows and rowsById when a passing row has sub-rows that were skipped due to depth >= maxDepth. Fixes TanStack#5987
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds recursive sub-row flattening in ChangesflatRows fix for
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@packages/table-core/src/features/column-filtering/filterRowsUtils.ts`:
- Around line 164-170: The depth-limit branch in recurseFilterRows is appending
visible subRows after the current row, which breaks the usual
descendant-before-parent flatRows order. Update the branch that handles
row.subRows with depth >= maxDepth so it preserves the same traversal ordering
as the normal recursion path, using addSubRowsToFlatArrays and the surrounding
filtered row accumulation logic to keep flatRows consistent regardless of
maxLeafRowFilterDepth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 10271759-df3d-4e9f-b330-c4cb64286554
📒 Files selected for processing (2)
packages/table-core/src/features/column-filtering/filterRowsUtils.tspackages/table-core/tests/unit/features/column-filtering/filterRowsUtils.test.ts
| // When sub-rows exist but weren't recursed into (depth >= maxDepth), | ||
| // they are still visible in the row tree. Add them to flatRows so that | ||
| // flatRows is a complete flat representation of all visible rows, | ||
| // consistent with the no-filter case. | ||
| if (row.subRows.length && depth >= maxDepth) { | ||
| addSubRowsToFlatArrays(row.subRows, newFilteredFlatRows, newFilteredRowsById) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve flatRows traversal order in the depth-limit branch.
recurseFilterRows() normally appends descendants before their parent, but this branch pushes row first and only then appends the skipped descendants. That makes flatRows order change when the only thing that changes is maxLeafRowFilterDepth.
Suggested fix
- filteredRows.push(row)
- newFilteredFlatRows.push(row)
- newFilteredRowsById[row.id] = row
// When sub-rows exist but weren't recursed into (depth >= maxDepth),
// they are still visible in the row tree. Add them to flatRows so that
// flatRows is a complete flat representation of all visible rows,
// consistent with the no-filter case.
if (row.subRows.length && depth >= maxDepth) {
addSubRowsToFlatArrays(row.subRows, newFilteredFlatRows, newFilteredRowsById)
}
+ filteredRows.push(row)
+ newFilteredFlatRows.push(row)
+ newFilteredRowsById[row.id] = row📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // When sub-rows exist but weren't recursed into (depth >= maxDepth), | |
| // they are still visible in the row tree. Add them to flatRows so that | |
| // flatRows is a complete flat representation of all visible rows, | |
| // consistent with the no-filter case. | |
| if (row.subRows.length && depth >= maxDepth) { | |
| addSubRowsToFlatArrays(row.subRows, newFilteredFlatRows, newFilteredRowsById) | |
| } | |
| // When sub-rows exist but weren't recursed into (depth >= maxDepth), | |
| // they are still visible in the row tree. Add them to flatRows so that | |
| // flatRows is a complete flat representation of all visible rows, | |
| // consistent with the no-filter case. | |
| if (row.subRows.length && depth >= maxDepth) { | |
| addSubRowsToFlatArrays(row.subRows, newFilteredFlatRows, newFilteredRowsById) | |
| } | |
| filteredRows.push(row) | |
| newFilteredFlatRows.push(row) | |
| newFilteredRowsById[row.id] = row |
🤖 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 164 - 170, The depth-limit branch in recurseFilterRows is appending
visible subRows after the current row, which breaks the usual
descendant-before-parent flatRows order. Update the branch that handles
row.subRows with depth >= maxDepth so it preserves the same traversal ordering
as the normal recursion path, using addSubRowsToFlatArrays and the surrounding
filtered row accumulation logic to keep flatRows consistent regardless of
maxLeafRowFilterDepth.
|
View your CI Pipeline Execution ↗ for commit 17dbb8e
☁️ Nx Cloud last updated this comment at |
![nx-cloud[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nx Cloud is proposing a fix for your failed CI:
We moved filteredRowModel and filterFns from the constructTable features option into the top-level tableFeatures() call, which is where TypeScript resolves valid filter function key names via ExtractFilterFnKeys<TFeatures>. This fixes both TS2322 ('includesString' not assignable) and TS2353 (filteredRowModel not a known property on the features object), bringing the test file in line with the canonical pattern used elsewhere in the workspace.
Tip
✅ We verified this fix by re-running @tanstack/table-core:test:types.
diff --git a/packages/table-core/tests/unit/features/column-filtering/filterRowsUtils.test.ts b/packages/table-core/tests/unit/features/column-filtering/filterRowsUtils.test.ts
index 2a06d8d8..fe30f6fa 100644
--- a/packages/table-core/tests/unit/features/column-filtering/filterRowsUtils.test.ts
+++ b/packages/table-core/tests/unit/features/column-filtering/filterRowsUtils.test.ts
@@ -12,7 +12,12 @@ import type { ColumnDef, ColumnFiltersState } from '../../../../src'
type Row = { name: string; subRows?: Row[] }
-const features = tableFeatures({ ...coreFeatures, columnFilteringFeature })
+const features = tableFeatures({
+ ...coreFeatures,
+ columnFilteringFeature,
+ filteredRowModel: createFilteredRowModel(),
+ filterFns,
+})
function makeNestedTable(opts?: {
maxLeafRowFilterDepth?: number
@@ -51,8 +56,6 @@ function makeNestedTable(opts?: {
},
features: {
...features,
- filteredRowModel: createFilteredRowModel(),
- filterFns,
coreReactivityFeature: storeReactivityBindings(),
},
})
Because this branch comes from a fork, it is not possible for us to apply fixes directly, but you can apply the changes locally using the available options below.
Apply changes locally with:
npx nx-cloud apply-locally dqWa-F83o
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
Bug
Closes #5987
When
filterRowModelFromRootskips recursing into sub-rows becausedepth >= maxLeafRowFilterDepth, the parent row's sub-rows remain accessible throughrow.subRowsbut were never added tonewFilteredFlatRows. This makesflatRowsan incomplete flat representation of the visible tree.Root cause
Impact
column.getFacetedUniqueValues()returns inconsistent counts:flatRows= all rows at all depths (frompreRowModel) — counts sub-rows ✓maxLeafRowFilterDepth: 0:flatRows= only passing top-level rows — misses sub-rows ✗Fix
Added
addSubRowsToFlatArrayshelper that recursively pushes sub-rows intoflatRowsandrowsByIdwhen a passing row has sub-rows that were skipped due todepth >= maxDepth.Verification
New test file:
packages/table-core/tests/unit/features/column-filtering/filterRowsUtils.test.tsAll 359 existing tests continue to pass.
Summary by CodeRabbit
maxLeafRowFilterDepthdifferences and expectedflatRowscounts.