Skip to content

feat(data-table): add filter operator allowlists - #462

Open
IzumiSy wants to merge 3 commits into
mainfrom
feat/data-table-filter-operators
Open

feat(data-table): add filter operator allowlists#462
IzumiSy wants to merge 3 commits into
mainfrom
feat/data-table-filter-operators

Conversation

@IzumiSy

@IzumiSy IzumiSy commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Motivation

DataTable.Filters currently decides the available conditions entirely from the filter type. That works for broad defaults, but it gives consumers no way to narrow the filter UI for a specific column.

This change adds a column-level allowlist so a table can expose only the conditions that make sense for that field, while keeping the existing collection control and persisted filter formats compatible.

Design Decision

Chosen approach

Add an optional operators property to Column.filter and treat it as a DataTable UI allowlist.

The configured order drives both the operator menu order and the default operator. The same override is also supported in inferColumns(..., { filter: { operators: [...] } }) so metadata-derived columns can use the feature without switching to manual filter config.

Compatibility boundary

The allowlist only affects the built-in DataTable filter UI. CollectionControl.addFilter(...), URL state, and saved filters still accept the broader backend operator set.

To avoid silently rewriting existing state, active filters whose operator is no longer in the configured allowlist are still preserved and rendered, but new operator choices come from the configured set.

Alternatives considered

A broader shared FilterConfig change would have mixed backend query semantics with DataTable-specific UI behavior. Keeping the allowlist at the DataTable column layer keeps the API smaller and avoids changing collection-wide contracts.

Summary

  • add filter.operators to DataTable columns to narrow the built-in filter UI per column
  • support the same operator override in inferColumns() metadata helpers
  • update DataTable filter behavior, tests, docs, and add a changeset for the new API

@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Code Metrics Report

main (de6ef76) #462 (29f02ac) +/-
Coverage 90.0% 90.1% +0.0%
Test Execution Time 2m4s 2m1s -3s
Details
  |                     | main (de6ef76) | #462 (29f02ac) |  +/-  |
  |---------------------|----------------|----------------|-------|
+ | Coverage            |          90.0% |          90.1% | +0.0% |
  |   Files             |            126 |            126 |     0 |
  |   Lines             |           5174 |           5201 |   +27 |
+ |   Covered           |           4661 |           4688 |   +27 |
+ | Test Execution Time |           2m4s |           2m1s |   -3s |

Code coverage of files in pull request scope (78.1% → 79.1%)

Files Coverage +/- Status
packages/core/src/components/data-table/field-helpers.ts 100.0% 0.0% modified
packages/core/src/components/data-table/toolbar.tsx 78.4% +0.9% modified

Reported by octocov

@IzumiSy

IzumiSy commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Code Review completed successfully!

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generated by Code Review for issue #462 · 42.3 AIC · ⌖ 8.61 AIC · ⊞ 5.9K
Comment /review to run again

Comment thread packages/core/src/components/data-table/toolbar.tsx
Comment thread packages/core/src/components/data-table/types.ts
@IzumiSy IzumiSy self-assigned this Aug 21, 2026
@IzumiSy
IzumiSy marked this pull request as ready for review August 21, 2026 07:03
@IzumiSy
IzumiSy requested a review from a team as a code owner August 21, 2026 07:03

@interacsean interacsean left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 must-fix, plus docs. No issue is linked to this PR, so I reviewed against the PR's stated goals and platform-planning#1509 (which fixed the current per-type default operators). The four code comments below are largely one root cause — see the comment on getConfiguredFilterOperators.

The backwards compatibility offered to support saved URLs and bookmarks — where filter operators that are no longer offered by default still get applied — is a solid choice (pre-existing, and hardened here with isUiOperatorAllowedForType). Noting it and asking whether it can ever be deprecated. I suggest not: we can never force a user to stop using an old bookmark.

Everything else is mechanically clean: operator derivation stays confined to toolbar.tsx (grepped getAddFilterOperators, STRING_OPERATORS, DATE_OPERATORS, NUMERIC_TEMPORAL_OPERATORS, BOOLEAN_OPERATORS — no other consumer), the out-of-allowlist preservation path is consistent across all six editors and both surfaces, every Extract<OperatorForFilterType[T], …> narrowing resolves non-never, PanelValueEditor handles a single-operator between allowlist without malformed commits, the interfacetype change on MetadataFieldOptions stays assignable from the un-parameterized form, and vite build emits declarations cleanly.

function getConfiguredFilterOperators(config: DataTableFilterConfig): FilterOperator[] {
const defaults = getDefaultFilterOperators(config.type);
const configured = config.operators as readonly FilterOperator[] | undefined;
if (!configured || configured.length === 0) return defaults;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible for config.operators to be required at the type level to have at least one item? That would avoid the confusion of passing an empty array and having the defaults appear.

Claude finding:

A non-empty tuple (readonly [Op, ...Op[]]) would close the empty-array case, and it's worth going one step further, because this line and the fallback on :175 are the root cause of most of the other comments on this PR. Three distinct states are collapsed into one, and that one state means "show everything":

  • not configured — still routes through here, which is how an unconfigured string column ends up defaulting to eq (see the comment on :190)
  • configured empty — your point above
  • configured invalid — an allowlist that is entirely wrong for the column type yields [] at :175 and silently becomes no allowlist. Verified: a string column with operators: ["gt"] renders all six string conditions, no error anywhere.

Suggested shape, which is less code than what's here rather than more:

  1. Don't route the unconfigured path through the allowlist machinery at all — absent config.operators should mean DEFAULT_OPERATOR[type] for the default and the full set for the menu, full stop.
  2. Non-empty tuple, so empty is unrepresentable.
  3. Derive the UI operator unions in types.ts from the runtime consts rather than hand-writing both. Right now DataTableStringFilterOperator / …NumericTemporal… / …Date… / …Boolean… plus the inline ["in"] and ["eq"] in getDefaultFilterOperators are six parallel definitions across two files with nothing enforcing agreement (only boolean is structurally derived, and that's coincidence). (typeof STRING_OPERATORS)[number], with the consts declared as const satisfies readonly OperatorForFilterType["string"][], matches the pattern OPERATORS_BY_FILTER_TYPE already uses in collection.ts.

Drift between those lists is silent in both directions today, and it's silent because of the fallback: add an operator to STRING_OPERATORS only and the UI offers it while allowlists reject it at compile time; add it to types.ts only and it typechecks, gets filtered out here, and the allowlist widens to everything. With 1–3 in place, invalid becomes near-unreachable and the runtime filter drops to belt-and-braces — where it belongs — so whatever remains can just warn in dev instead of widening.

}

function getDefaultFilterOperator(config: DataTableFilterConfig): FilterOperator {
return getConfiguredFilterOperators(config)[0] ?? DEFAULT_OPERATOR[config.type];

@interacsean interacsean Aug 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes the fallback of string types from contains to now be eq. While I don't expect this to affect business flows, I think this is worth fixing regardless: we wouldn't want our default string search to be eq, since users typically intend a loose match.

Claude finding:

getDefaultFilterOperator returns getConfiguredFilterOperators(config)[0], and for a column with no operators config that is STRING_OPERATORS[0]"eq", not DEFAULT_OPERATOR["string"] ("contains"). Because getConfiguredFilterOperators never returns an empty array, the ?? DEFAULT_OPERATOR[config.type] fallback is dead and DEFAULT_OPERATOR is now unreachable.

Verified by running the same probe against both branches. On main, opening Add filter on a plain { type: "string" } column preselects contains and commits ["name","contains","Bob",{caseSensitive:false}]; on this branch it preselects is and commits ["name","eq","Bob",…]. String is the only type where the two disagree (getDefaultFilterOperators(t)[0] === DEFAULT_OPERATOR[t] for all seven others), so this is one bug — but it reaches every existing consumer with a text column, ships under a minor, contradicts the operator-default table in platform-planning#1509, and no test catches it (all 286 data-table tests pass).

The same root cause kills the ?? "contains" fallback in StringFilterEditor at :1626, so a chip carrying a persisted non-UI string operator (regex, in) now resets to eq rather than contains.

Fix: only let list order pick the default when config.operators is explicitly set — or better, take the structural route in the comment on :170, which removes this line's dependence on the allowlist entirely. Either way please add a regression test asserting an unconfigured string column defaults to contains; there's currently nothing pinning it.

*/
export type HeaderRenderContext = NonSortableHeaderRenderContext | SortableHeaderRenderContext;

type DataTableStringFilterOperator = Extract<

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we export DataTableFilterConfig? As it stands consumers can't name the type of column.filter, so they'll keep annotating with FilterConfig — which quietly makes that the compat surface we're then obliged to keep assignable.

Claude finding:

Worth correcting the rationale in the PR description while we're here, since it affects how this type gets maintained. The description frames the split as avoiding a breaking change to the shared FilterConfig. It wouldn't have been one: I patched origin/main to add operators?: readonly OperatorForFilterType[<type>][] to all eight FilterConfig branches, rebuilt core, and typechecked packages/core, both examples, and packages/sdk-plugin — identical error sets before and after, zero new failures. Adding an optional property to a discriminated union is purely additive in both assignability directions.

What does justify the split is narrowing. OPERATORS_BY_FILTER_TYPE admits in, nin, regex, notHasPrefix and notHasSuffix for string, and gt/lt/ne for date — none of which the filter UI can render. On FilterConfig the field would have to accept the full backend set, so operators: ["regex"] on a string column would typecheck and then be silently dropped at runtime. Only a DataTable-specific type makes that a compile error. So the boundary is the right call, just on narrowing grounds rather than compatibility grounds.

No change requested beyond the export — the doc comments on this type and on ColumnBase.filter are accurate about the UI-only scope.

@@ -30,9 +57,9 @@ export function inferColumns<
>(tableMetadata: TTable): ColumnInferFn<TRow, TTable> {

@interacsean interacsean Aug 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tracking this separately for the next major (ticket to follow, labelled next-major) — nothing needed in this PR beyond the runtime guard discussed on getConfiguredFilterOperators, however I suggest adding a deprecated now here (as below)

Claude finding:

const TTable extends TableMetadata = TableMetadata on the line above is the mechanism behind the type hole. TypeScript has no partial type-argument inference, so supplying TRow explicitly pins TTable to this default instead of inferring it from the argument; InferredFieldFilterType then collapses to never and the per-field operator narrowing silently switches off. Confirmed: inferColumns<TaskRow>(md)("title", { filter: { operators: ["gt"] } }) and ("dueDate", { filter: { operators: ["notContains"] } }) both typecheck clean, while only the createColumnHelper form errors — which is why the @ts-expect-error cases added in this PR all use that form.

Also worth tightening the note at types.ts:590: "If the metadata has already widened to TableMetadata, mismatched operators are still filtered out at runtime" understates it on two counts — the trigger is supplying TRow explicitly rather than the metadata being widened at the call site, and the allowlist is discarded wholesale rather than merely filtered.

Suggested path: @deprecated the explicit-TRow call form now, pointing at createColumnHelper<TRow>().inferColumns(metadata) (already the documented preference and correct here), then drop TRow as an explicit type parameter in the next major so TTable is always inferred.

| `operators` | `FilterOperator[]` | Optional DataTable UI allowlist. Order controls menu order and the default operator. |

`operators` only narrows what the built-in DataTable filter UI shows. Programmatic `CollectionControl.addFilter(...)`, URL state, and saved/persisted filters still use the broader backend operator set.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may also be worth updating packages/sdk-plugin/README.md — line 38 onward documents inferColumns and its filter derivation too.

Claude finding:

Relatedly, the inferColumns(tableMetadata) options table further down this file (line 648) still lists filter as boolean, default true, "Set to false to suppress the auto-generated filter config." The changeset advertises "inferColumns() now also accepts filter: { operators: [...] }", so that half of the feature is currently undocumented at its own reference site. Suggest widening the row to boolean | { operators?: FilterOperator[] } and carrying over the UI-only caveat added at line 615.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants