fix(column-filtering): include sub-rows in flatRows when maxLeafRowFilterDepth skips recursion#6395
Open
sanjibani wants to merge 1 commit into
Open
Conversation
…lterDepth skips recursion `filterRowModelFromRoot` was discarding sub-rows when the parent passed the filter but `depth >= maxLeafRowFilterDepth`. The parent row got pushed into `flatRows` and `rowsById`, but the children stayed reachable only via `row.subRows`, leaving `flatRows` an incomplete view of the visible tree. `column.getFacetedUniqueValues()` (which iterates `flatRows`) returned consistent counts before filtering but inconsistent counts after, because the pre-filter `flatRows` from the core row model includes sub-rows while the filtered `flatRows` did not. Add a small helper `addSubRowsToFlatArrays` that recursively pushes each surviving sub-row into `flatRows` and `rowsById`, and call it from the `else if` branch the parent-pass / depth-skipped case falls into. The branch is reached only when `row.subRows.length` is truthy, so the helper always has work to do. Closes TanStack#6361 (refs TanStack#5987)
Contributor
|
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 (1)
📝 WalkthroughWalkthroughAdds a recursive helper ChangesflatRows Consistency Fix
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 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 file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6361 (refs #5987).
Bug
`filterRowModelFromRoot` discarded sub-rows when the parent passed the filter but recursion was skipped because `depth >= maxLeafRowFilterDepth`. The parent row was pushed into `flatRows` and `rowsById`, but the children stayed reachable only via `row.subRows`, leaving `flatRows` an incomplete view of the visible tree.
`column.getFacetedUniqueValues()` (which iterates `flatRows`) returned consistent counts before filtering but inconsistent counts after, because the pre-filter `flatRows` from the core row model includes sub-rows while the filtered `flatRows` did not.
Fix
Add a small helper `addSubRowsToFlatArrays` that recursively pushes each surviving sub-row into `flatRows` and `rowsById`, and call it from the `else if` branch the parent-pass / depth-skipped case falls into. The branch is reached only when `row.subRows.length` is truthy, so the helper always has work to do.
`flatRows` and `rowsById` now contain the same set of rows that `rows` plus the surviving `row.subRows` paths cover, so faceted-unique counts and other flat-row consumers agree with the visible tree.
`filterRowModelFromLeafs` already pushed every passing leaf (including sub-rows that came from recursion) into `flatRows` via its own loop, so the bug was specific to `filterRowModelFromRoot`.
Tests
Local test harness is not available in this environment (no `node_modules`), but the change is small and the cases are mechanical:
A follow-up commit can add a regression test once the test harness is wired up locally; happy to do that in response to review feedback.
1 file changed, +20/-0.
Summary by CodeRabbit