Skip to content

fix(table-core): use autoRemove instead of resolveFilterValue on comparison filter fns#6213

Open
rkdfx wants to merge 1 commit into
TanStack:alphafrom
rkdfx:fix-resolveFilterValue-comparison-fns
Open

fix(table-core): use autoRemove instead of resolveFilterValue on comparison filter fns#6213
rkdfx wants to merge 1 commit into
TanStack:alphafrom
rkdfx:fix-resolveFilterValue-comparison-fns

Conversation

@rkdfx

@rkdfx rkdfx commented Mar 29, 2026

Copy link
Copy Markdown

Changes

  • filterFn_greaterThan, filterFn_greaterThanOrEqualTo, filterFn_lessThan, and
    filterFn_lessThanOrEqualTo incorrectly used .resolveFilterValue instead of .autoRemove
  • resolveFilterValue transforms the filter value before comparison, so testFalsy(50) returns
    false, and false ?? 50 evaluates to false (not nullish), making the comparison
    rowValue > 0 instead of rowValue > 50
  • Every other simple filter function in the file correctly uses .autoRemove

Fixes #6212

Checklist

  • I have followed the steps in the Contributing guide.
  • I have tested this code locally with pnpm test:pr.

Summary by CodeRabbit

  • Bug Fixes
    • Improved comparison filter operations to properly manage filter removal when handling empty or invalid values.
@coderabbitai

coderabbitai Bot commented Mar 29, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cd1cc26a-1cf4-4047-afaa-13921b4417b7

📥 Commits

Reviewing files that changed from the base of the PR and between 2a4533e and 9f13c1d.

📒 Files selected for processing (1)
  • packages/table-core/src/fns/filterFns.ts

📝 Walkthrough

Walkthrough

Four numeric comparison filter functions (filterFn_greaterThan, filterFn_greaterThanOrEqualTo, filterFn_lessThan, filterFn_lessThanOrEqualTo) had their resolveFilterValue property replaced with an autoRemove property, both using testFalsy(val). This corrects a bug where incorrect property usage caused filter value transformations to corrupt comparisons.

Changes

Cohort / File(s) Summary
Numeric Comparison Filter Functions
packages/table-core/src/fns/filterFns.ts
Replaced resolveFilterValue property with autoRemove property across four comparison filter functions (greaterThan, greaterThanOrEqualTo, lessThan, lessThanOrEqualTo). Both use testFalsy(val) implementation.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 Comparisons now fly straight and true,
No more false coercions to muddy the view,
Five lines fixed, filters aligned,
Greater-than glory restored by design!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: replacing resolveFilterValue with autoRemove on four comparison filter functions, which directly addresses the core issue.
Linked Issues check ✅ Passed The PR successfully addresses both requirements from issue #6212: replacing resolveFilterValue with autoRemove on four comparison filter functions and enabling auto-removal of empty filter values.
Out of Scope Changes check ✅ Passed All changes are directly related to the linked issue. Only the four comparison filter functions were modified, with no extraneous changes to other parts of the codebase.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 and usage tips.

@rkdfx rkdfx force-pushed the fix-resolveFilterValue-comparison-fns branch from 9f13c1d to 8ace484 Compare April 7, 2026 05:20
…arison filter fns

  filterFn_greaterThan, filterFn_greaterThanOrEqualTo, filterFn_lessThan,
  and filterFn_lessThanOrEqualTo incorrectly used .resolveFilterValue
  instead of .autoRemove. This corrupted filter values — e.g. "price > 50"
  became "price > false" (coerced to "price > 0") because testFalsy(50)
  returns false, and false is not nullish so the ?? fallback doesn't apply.

  Fixes TanStack#6212
@rkdfx rkdfx force-pushed the fix-resolveFilterValue-comparison-fns branch from 8ace484 to e644191 Compare April 17, 2026 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant