fix(table-core): respect custom filterFn.autoRemove in shouldAutoRemoveFilter

[djk01281]

🎯 Changes

Fixes #6101

Problem

In shouldAutoRemoveFilter, the hardcoded (typeof value ==='string' && !value) check runs after filterFn.autoRemove via ||, so it can override a custom autoRemove that returns false for empty strings.

This makes it impossible to use '' as a valid filter value with custom filter functions:

// User defines: "only remove when undefined"
filterFn.autoRemove = (val) => val === undefined

// But shouldAutoRemoveFilter ignores this and removes '' anyway
column.setFilterValue('') // → filter silently removed

Before

2026-02-22.4.52.43.mov

After

2026-02-22.5.27.43.mov

What changed

When filterFn.autoRemove is defined, only its result is used, without additional hardcoded checks overriding it.
Built-in filter functions are unaffected since they all define autoRemove via testFalsey.

Notes

Same issue exists in v9 alpha branch: packages/table-core/src/features/column-filtering/columnFilteringFeature.utils.ts

✅ Checklist

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

🚀 Release Impact

• This change affects published code, and I have generated a changeset.
• This change is docs/CI/dev-only (no release).

Summary by CodeRabbit

• Bug Fixes

  • Filter auto-removal now correctly respects custom autoRemove logic, preventing unintended removal of valid filter values.

• Tests

  • Added comprehensive tests covering custom autoRemove hooks and default behavior across empty, undefined, null, zero, and false values to ensure consistent filter retention/removal.

[changeset-bot]

🦋 Changeset detected

Latest commit: 4c4dcf7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages

Name	Type
@tanstack/table-core	Patch
@tanstack/angular-table	Patch
@tanstack/lit-table	Patch
@tanstack/qwik-table	Patch
@tanstack/react-table	Patch
@tanstack/solid-table	Patch
@tanstack/svelte-table	Patch
@tanstack/vue-table	Patch
@tanstack/react-table-devtools	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR fixes shouldAutoRemoveFilter to respect a custom filterFn.autoRemove callback by invoking it first; adds a changeset for a patch release and new tests covering custom and default autoRemove behaviors across multiple filter values.

Changes

Cohort / File(s)	Summary
Changeset ​.changeset/fix-should-auto-remove-filter.md	Adds a patch changeset documenting the fix: respect custom autoRemove in shouldAutoRemoveFilter.
Core Functionality packages/table-core/src/features/ColumnFiltering.ts	Refactors shouldAutoRemoveFilter to early-return when filterFn.autoRemove exists, calling filterFn.autoRemove(value, column) before falling back to undefined/empty-string checks.
Tests packages/table-core/tests/shouldAutoRemoveFilter.test.ts	Adds tests for three filterFn variants (with custom autoRemove, with autoRemove returning false, and without autoRemove) covering values: undefined, '', null, 0, and false to validate expected removal behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A tiny hop, a tidy tweak,
autoRemove now gets the peek.
Empty strings may stay or go,
My callback tells you so! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: fixing shouldAutoRemoveFilter to respect custom filterFn.autoRemove implementations.
Linked Issues check	✅ Passed	The code changes directly address issue #6101 by refactoring shouldAutoRemoveFilter to prioritize custom autoRemove callbacks over hardcoded checks.
Out of Scope Changes check	✅ Passed	All changes are scoped to the fix: implementation refactor, comprehensive test coverage, and changeset documentation. No extraneous modifications detected.
Description check	✅ Passed	The PR description comprehensively covers all required sections: detailed problem statement with code examples, before/after demonstration, specific change explanation, and all checklist items completed.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (2)

packages/table-core/tests/shouldAutoRemoveFilter.test.ts (2)

51-59: Consider adding null coverage to the "without filterFn" block for symmetry.

The noAutoRemove suite already asserts null → false. Adding the same assertion when filterFn is undefined would make the two "no autoRemove path" blocks consistent and complete.

✅ Suggested addition

   describe('without filterFn', () => {
     it('should remove undefined filter', () => {
       expect(shouldAutoRemoveFilter(undefined, undefined)).toBe(true)
     })

     it('should remove empty string filter', () => {
       expect(shouldAutoRemoveFilter(undefined, '')).toBe(true)
     })
+
+    it('should keep null filter', () => {
+      expect(shouldAutoRemoveFilter(undefined, null)).toBe(false)
+    })
   })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/table-core/tests/shouldAutoRemoveFilter.test.ts` around lines 51 -
59, Add a test case in the "without filterFn" describe block to cover null
input: call shouldAutoRemoveFilter(undefined, null) and assert it returns true
to mirror the existing noAutoRemove suite's null check; locate the test file
tests/shouldAutoRemoveFilter.test.ts and update the describe('without filterFn')
block that contains the existing undefined and '' expectations for
shouldAutoRemoveFilter.

---

5-35: Add a test documenting the new capability: custom autoRemove returning false for undefined.

The primary behavioral change introduced by this PR — beyond the empty-string fix — is that a custom autoRemove returning false for undefined now prevents filter removal, whereas the old code would always remove for undefined. The current suite doesn't cover this case, so a regression would go undetected.

✅ Suggested additional test fixture and case

+const neverAutoRemove: FilterFn<any> = (row, columnId, filterValue) => {
+  return filterValue === row.getValue(columnId)
+}
+neverAutoRemove.autoRemove = (_val) => false
+
 describe('shouldAutoRemoveFilter', () => {
   describe('with custom autoRemove defined', () => {
     // ... existing tests ...
+
+    it('should keep undefined filter when custom autoRemove returns false for it', () => {
+      expect(shouldAutoRemoveFilter(neverAutoRemove, undefined)).toBe(false)
+    })
   })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/table-core/tests/shouldAutoRemoveFilter.test.ts` around lines 5 -
35, Add a test that verifies a custom filter's autoRemove returning false for
undefined prevents removal: create a FilterFn (e.g., customNoRemoveUndefined)
whose autoRemove = (v) => v !== undefined (or explicitly returns false when v
=== undefined) and add an it block using
shouldAutoRemoveFilter(customNoRemoveUndefined, undefined) expecting false;
place this alongside the other tests referencing customAutoRemove and
shouldAutoRemoveFilter so the regression is covered.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/table-core/tests/shouldAutoRemoveFilter.test.ts`:
- Around line 51-59: Add a test case in the "without filterFn" describe block to
cover null input: call shouldAutoRemoveFilter(undefined, null) and assert it
returns true to mirror the existing noAutoRemove suite's null check; locate the
test file tests/shouldAutoRemoveFilter.test.ts and update the describe('without
filterFn') block that contains the existing undefined and '' expectations for
shouldAutoRemoveFilter.
- Around line 5-35: Add a test that verifies a custom filter's autoRemove
returning false for undefined prevents removal: create a FilterFn (e.g.,
customNoRemoveUndefined) whose autoRemove = (v) => v !== undefined (or
explicitly returns false when v === undefined) and add an it block using
shouldAutoRemoveFilter(customNoRemoveUndefined, undefined) expecting false;
place this alongside the other tests referencing customAutoRemove and
shouldAutoRemoveFilter so the regression is covered.