Feat faceted filters - #12
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
arrIncludesSome calls .includes() on the cell value, so it throws on numeric columns and substring-matches on string columns (selecting "Admin" also matched "Super Admin"). facetedFilterFn checks the cell value for exact membership in the selected-values array getFacet actually writes, which is what faceted columns need. Corrects the status/category fixture in useFacetedFilters.test.tsx to declare it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Introduce isRangeFilterFn as the single source of truth for what counts as a range filterFn, used by both the read path (isRangeColumn) and the setRange warning guard. Previously only the read path checked identity against facetedFilterFn; setRange relied on needsRangeFilterFn alone, which returns false for any function and so silently accepted facetedFilterFn as a valid range target.
…ter state normalizeSelected returned the array filter value by reference, so facet.selected was the literal array stored in columnFilters and mutating it would mutate table state. emptyFacet() already guards against this on options via a factory; normalizeSelected now does the same by returning [...filterValue].
- facetsEntry.test.ts: assert the root export is the same function
object as src/utils/facets' facetedFilterFn — isRangeFilterFn depends
on this identity holding through the package root.
- useFacetedFilters.test.tsx: add a test that builds the table with
pagination: { pageSize: 2 } and asserts getFacet counts still
describe the full dataset, not the current page. Every existing
facet test used pagination: false, so the default paginated path was
previously unexercised.
…tions
- The 'Required column setup' and Usage snippets now import
facetedFilterFn instead of assuming it is already in scope, with a
short paragraph naming it as a root-entry export.
- Limitations: facet values must be JSON-round-trippable primitives
(a Date comes back from URL persistence as an ISO string and will
never match facetedFilterFn's equality check again), and
useTable({ columnFilters: false }) makes facets inert with no
warning.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
Summary
Adds
useFacetedFilters, the data layer behind a faceted filter sidebar — the"Status: Active (24), Archived (7)" checkbox list, and numeric range sliders
bounded by the column's real values.
Selections are written straight to
columnFilters, so URL sync and persistencekeep working with no extra wiring.
useTablenow passes TanStack's threefaceted row models unconditionally — they create one memoised closure per column
and compute only on access, so there is nothing to opt into and nothing to
remember. Ships from the root entry; no new dependency.
Why
Filtering is wrapped already, but nothing could answer the question every filter
UI needs: what values are in this column, and how many rows does each have?
Without it a consumer either hardcodes the option list — which then drifts from
the data silently — or rederives it from the raw rows, duplicating work the table
already does and getting the interaction subtly wrong.
The detail that makes it work
Counts come from
column.getFacetedUniqueValues(), which is built on thefaceted row model. That model deliberately excludes the column's own
filter:
Reading
getFilteredRowModel()instead would collapse every other option in afacet to zero the moment the user ticked one box, making the rest unclickable.
A useful side effect: because a facet's counts can't be changed by its own
filter, the option list never reorders under the cursor while you click it —
which is what makes count-descending sorting safe.
There are paired tests for this: one asserts a facet's own counts are unchanged
after toggling one of its values, the other asserts they do change when a
different column is filtered. Both would have to be rewritten to make a
regression pass.
facetedFilterFn— a new root export, and why it existsTanStack ships no built-in meaning "cell value is one of the selected
values", and the obvious candidate is a trap.
arrIncludesSomecalls.includes()on the cell value, not the selection:It exists for columns whose cell value is itself an array. On the scalar columns
faceting actually targets, a numeric column throws (
.includes is not a function) and a string column silently substring-matches — selectingAdminalso matchesSuper Admin. Verified against the v8 source.So faceted columns declare tablecraft's own:
This was found mid-implementation, after the spec, plan, docs and every test
fixture had already standardised on
arrIncludesSome. The existing tests passedonly because the fixture values happened to share no substrings.
Development warnings
Four conditions the hook can detect but not fix, each
NODE_ENV-guarded andfired once per column per reason:
toggleon a column missingfacetedFilterFnsetRangeon a column not configured for rangesmanualPaginationtableA custom
filterFnnever warns — it may well handle the value shape, and thehook can't know. This is deliberately unlike
useTableExport'srows: 'selected'case, which stayed undetectable and was left to documentation.
Limitations, documented rather than worked around
Server-side tables.
useServerTableanduseQueryTablehold one page, notthe dataset, so counts derived from them would describe the current page —
plausible and wrong. On a
manualPaginationtable the facets are empty and thehook says why. Left unguarded this would not have failed loudly, which is what
made the guard worth writing.
Facet values must be JSON-round-trippable primitives.
src/utils/url.tsJSON round-trips the filter array, so a
Datereturns as an ISO string and theequality check will never match it again.
Array-valued columns yield one option per distinct array, not per tag.
TanStack's
getUniqueValuescolumn option is the escape hatch.useTable({ columnFilters: false })makes facets inert —togglewritesfilter state that never filters anything.
Testing
check-entry-deps: OKconfirms the root entry still reaches onlyreactand@tanstack/react-table.Assertions use exact counts and exact option order throughout. Set-based and
toBeGreaterThanassertions are excluded on purpose — that pattern previouslyhid a real duplicate-rows bug in the export work by passing against wrong data.
Breaking changes
None. Additive only.
useTablegains three row-model options internally; nopublic signature changes.
Notes for the reviewer
Three defects were caught during implementation and review that are worth
knowing about, since two of them originated in the plan rather than the code:
arrIncludesSome, above — the whole documented setup was wrong.table.getColumn(id)console.errors on an unknown id wheneverNODE_ENV !== 'production'. That would have fired on every render for anyconsumer whose facet config carried a stale column id. Replaced with a
findColumnhelper overgetAllLeafColumns().setRangedidn't warn on a value-list column. The read path was hardenedto refuse reporting a value list as a range, but the write path never caught
up — so
setRange([10, 50])on such a column wrote a filter matching onlyrows priced exactly 10 or 50, read back
undefined, and warned about nothing.Both paths now share one
isRangeFilterFnpredicate.Two things I'd flag for a second opinion:
getFacethas no guard mirroringgetRangeFacet's. On an'inNumberRange'column holding[20, 40],selectedreads[20, 40]andthose options render checked. Within contract — mismatching the two accessors
is documented as inert — but only one direction got hardened.
useTableA11y'suseCallback-wrapped prop-getters, so a consumeruseEffectkeyed on themre-fires.
[table, isServerTable]would be correct and stable deps; theoriginal rationale for skipping memoisation was wrong. Left as-is to avoid
churn at this stage.
package.jsonis unchanged and the CHANGELOG entry sits under[Unreleased]—this would be 3.2.0, and 3.1.0 is still unpublished (npm serves 3.0.0, and
there's no
v3.1.0tag).