fix(bindx-dataview): start a missing filter at its initial artifact - #134
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TPdGxo89UiPkZHxreAxvFD
A registered filter that the artifact record does not name (a stored record written before the filter existed, or a restored preset that omits it) was read three ways: `filters` materialized the handler default, while `getArtifact`, `resolvedWhere` and `hasActiveFilters` skipped it. The UI and the query disagreed, and a filter's `initialArtifact` was ignored. The record is now resolved once over the initial artifacts (`initialArtifact ?? handler.defaultArtifact()`), and every view, including a `setArtifact` updater, reads that. `resetFilter` still clears to the handler default and `resetAll` still returns to the initial artifacts. A DataGrid column can declare where its filter starts with `filterInitialArtifact`, threaded from `ColumnLeafProps` into the filter definitions of `useDataGridSetup`. The built-in scalar, enum, relation and generic columns accept it. Closes #125 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ge3kRufpi9b9HMBSBjRze5
`ColumnComponent`'s filter artifact parameter defaulted to the whole `FilterArtifact` union. `filterInitialArtifact` sits in the props parameter, so a column built by `createColumn` (typed by its own artifact) was no longer assignable to the unparameterized `ColumnComponent`. The parameter now defaults to `never`: an unknown column takes no initial artifact, and every specific column fits. `EnumFilterArtifact` and `EnumListFilterArtifact` take the enum value type (default `string`), so an enum column's `filterInitialArtifact` accepts only values of its enum. Also documents `getArtifact` and the fallback for a stored `null` entry, and adds pass-through tests for a `createColumn` column and a relation column plus type-level tests for the assignments above. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ge3kRufpi9b9HMBSBjRze5
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.
Closes #125.
The bug
useFilteringStateread a registered filter that its artifact record does not name in three different ways:filters.get(name).artifacthandler.defaultArtifact()(ignoringinitialArtifact)getArtifact(name)undefinedresolvedWhere,hasActiveFiltersA record misses a name when it was stored before the filter existed, or when
setAllArtifactsrestores a preset that omits the filter. A filter UI built onfiltersthen showed one state while the query applied another, and a filter meant to start constrained started unconstrained.A
DataGridcolumn also had no way to declare where its filter starts:useDataGridSetupregistered each column filter as{ handler }only.The fix
useFilteringStateresolves the record once:{ ...initialArtifacts, ...stored }, where a filter's initial artifact isinitialArtifact ?? handler.defaultArtifact()(the same value it already used when nothing was stored).filters,getArtifact,resolvedWhere,hasActiveFiltersand thesetArtifactupdater all read that record. Nothing extra is written to storage.ColumnLeafProps.filterInitialArtifact(optional) is passed to the filter definitions asinitialArtifactbyuseDataGridSetup. The built-in columns accept it: everycreateColumncolumn (typed by the column's artifact type), the enum and enum-list columns (headless andbindx-ui, values narrowed to the enum), the relation columns (RelationFilterArtifact) and the genericDataGridColumn.ColumnComponent<TExtraProps, TFilterArtifact>gains the artifact parameter, defaulting tonever, so a specific column stays assignable to the unparameterizedColumnComponent(const C: ColumnComponent = createColumn(textColumnDef, …)compiles). Such an erased column takes nofilterInitialArtifact.EnumFilterArtifact/EnumListFilterArtifacttake the enum value type (<TValue extends string = string>), sovalues/notValuesof an enum column's initial artifact must be values of its enum.Behaviour changes
getArtifact(name)for a registered filter the record does not name returns its initial artifact (initialArtifact ?? handler.defaultArtifact()), notundefined. It returnsundefinedonly for an unregistered name.setAllArtifacts({})(or any record that omits a registered filter) puts that filter at its initial artifact, so a filter with aninitialArtifactbecomes active. To clear it, pass the handler default for it explicitly.filters,getArtifact,resolvedWhere,hasActiveFiltersand asetArtifactupdater now agree for such a filter (before,filtersshowed the handler default while the query skipped it).Semantics chosen
Two resets existed already, and this PR keeps both:
resetFilter(name)andsetArtifact(name, undefined)clear the filter: they write the handler's default (inactive) artifact.resetAll()returns to where the grid starts: the initial artifacts.A name missing from the record now reads as "never touched", so it reads as where the grid starts: its initial artifact. For
setAllArtifactsthis means that a restored record that omits a filter puts that filter at its initial artifact, not at the handler default. A caller that stores only the active filters of a preset, and wants the omitted ones cleared, passes the handler default for them explicitly. Without aninitialArtifactthe two are the same value, so nothing changes for filters that do not declare one.One existing assertion changed for that reason: after
setAllArtifactsomits a filter with no initial artifact,getArtifactreturns the handler default{}instead ofundefined. The query is unchanged (the filter is inactive).Tests
tests/react/dataview/filterMissingFromStoredState.test.tsx(the failing repro frombug/grid-filter-initial-artifact-and-missing-default, extended):getArtifact,filters,resolvedWhere,hasActiveFilters;setAllArtifactswith a record that omits a filter → initial artifact everywhere;setArtifactupdater receives the initial artifact of an omitted filter;resetFilter→ handler default,resetAll→ initial artifact;DataGridwith<DataGridEnumColumn filter filterInitialArtifact={…} />starts filtered, with and without a stored record that predates the filter;filterInitialArtifactreaches the filter for acreateColumncolumn (DataGridTextColumn, rows filtered) and a relation column (DataGridHasOneColumn, resolvedwhere;MockAdapterdoes not evaluate relation conditions).tests/unit/types/columnFilterInitialArtifact.test.ts(type level): acreateColumncolumn andDataGridTextColumnassign toColumnComponent; a column accepts an initial artifact of its own type only; enum columns reject values outside their enum.Against
main, 6 of the 7 fail; the reset case pins existing behaviour and passes on both.Verification
bun run typecheckcleanbun run lint0 errors (no warnings in touched files)bun run test: 2121 pass, 0 fail🤖 Generated with Claude Code
https://claude.ai/code/session_01Ge3kRufpi9b9HMBSBjRze5