fix(view,list,core): a view's filter no longer disappears, or arrives as a predicate on columns that don't exist - #3081
Merged
Conversation
… as a predicate on columns that don't exist Sweeping the other `$filter` producers after #3078 turned up two live defects in `ObjectView`, which fetches its own data for calendar / kanban / gallery / timeline (grid delegates to `ObjectGrid`). 1. AN OBJECT FILTER WAS DROPPED, AND ONLY FOR NON-GRID VIEWS. `table.defaultFilters` is declared `Record<string, any>`, and the merge tested `baseFilter.length > 0` — `undefined > 0` for an object. The filter vanished and the view returned EVERY RECORD. `ObjectGrid` assigns the same value straight to `params.$filter`, so one view definition filtered correctly as a grid and returned everything as a calendar. 2. RULE OBJECTS WERE SPREAD INTO THE `and`, NOT WRAPPED. `['and', ...baseFilter, ...userFilter]` is only correct when the source is an array of AST nodes. `activeView.filter` is a spec `ViewFilterRule[]`, so spreading put bare rule objects where the AST expects nodes: isFilterAST(['and', {field:'stage',operator:'eq',value:'won'}, ['owner','=','me']]) // false → 400 since objectstack#4121 parseFilterAST(same) // {$and:[{field:'stage',operator:'eq',value:'won'}, {owner:'me'}]} The second line is a predicate over three columns named `field`, `operator` and `value` — which do not exist. Reachable whenever a view with a filter meets a user filter value. New in core: `toFilterNode` normalizes one source (rule array / AST / MongoDB object) and `mergeFilterNodes` combines sources as siblings under one `and`. `ObjectView` and `ListView.buildEffectiveFilter` both use them, so the three filter shapes are reconciled in one place instead of by hand at each renderer. The adapter also now translates a bare rule object sitting directly under a logical node — the chokepoint defence for any producer still emitting the spread shape. Only rule-SHAPED objects are touched; a child with no `field` is a genuine MongoDB condition and passes through untouched. CORRECTING A COMMENT SHIPPED IN #3078. `buildEffectiveFilter` documented the dropped-object case as unreachable, "nothing in this repo produces one for a list view". That was wrong: `ObjectView` passes `mergedFilters` straight into that schema's `filter`, and its last fallback is `table.defaultFilters`. The case is now handled rather than explained away. Also checked and clean: the dashboard widgets (metric/chart/pivot/data-table) pass a single resolved filter with no cross-vocabulary merge; gallery, gantt, calendar and map forward `schema.filter` unchanged; `filter-converter`'s own `['and', ...conditions]` spreads tuples it built itself. `ObjectView` was the only other producer merging two vocabularies. Verification: 19 tests across the four packages; reverting each source file fails the ones covering it. Emitted filters are asserted against the spec's own `isFilterAST` / `parseFilterAST`, including an executable pin on what the old spread shape produced. Full suite 761 files / 8856 tests green; tsc clean; eslint 0 errors. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
This was referenced Jul 31, 2026
Contributor
Author
|
Correction to this PR's second finding. I reported two
The shape is genuinely broken (a live server answers The changeset is unreleased, so #3087 amends it rather than letting the claim ship in release notes, and removes the dead filter/sort machinery that made the misreading possible. |
os-zhuang
added a commit
that referenced
this pull request
Jul 31, 2026
…connected (#3087) `ObjectView` carried its own filter and sort bar: `filterValues` / `sortConfig` state, a `filter-ui` schema and a `sort-ui` schema, ~80 lines of field introspection to build them. None of it was wired. No setter was ever called and neither schema was ever rendered — both states sat at their initial empty value for the component's entire life. Removed rather than wired, because the real filter and sort UI belongs to the renderer this component delegates to. `showFilters`, `showSort` and `filterableFields` are forwarded downstream and `ListView` implements them for real (its own filter panel, and a `filterableFields` whitelist). Connecting the local copy would have produced a SECOND filter bar competing with that one. The dead state was not inert, though — it left a branch in every merge path that could never run, and those branches read as live code: - the fetch path merged `baseFilter` with a `userFilter` that was always `[]`; - `mergedFilters` (what the `renderListView` slot receives, used by the Studio design surface) opened with a branch that REPLACED the view's filter with the user's instead of combining them — a real bug, had the state ever been written. CORRECTION TO #3081. That PR reported two ObjectView defects. Only the first was live: the object `table.defaultFilters` drop sat on the always-taken path. The second — rule objects spread into the `and` — required a non-empty user filter, so it could not run here. The shape is genuinely broken (a live server answers it with a 400, measured) and the adapter-level defence added alongside is still warranted for any producer that emits it, but that site was dead code, not a live defect. #3081's changeset is unreleased and now carries the correction. Keeping code that looks live and cannot run is what made that misreading possible — twice in one session — which is the argument for deleting it rather than leaving it for the next reader. No behaviour change: every removed branch was unreachable. Net -85 lines. The surviving paths are pinned by 4 new tests covering what the component hands the delegated renderer, alongside the 5 from #3081 covering what it queries with. Full suite 763 files / 8915 tests green; tsc clean; eslint 0 errors. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Swept the other
$filterproducers after #3078.ObjectViewwas the only other one merging two filter vocabularies, and it had two live defects. Everything else came back clean — the audit table is at the bottom.1. An object filter was dropped — and only for non-grid views
ObjectViewfetches its own data for calendar / kanban / gallery / timeline; grid delegates toObjectGrid. The merge testedbaseFilter.length > 0, which isundefined > 0for an object — andtable.defaultFiltersis declaredRecord<string, any>.So the filter vanished and the view returned every record.
ObjectGridassigns the same value straight toparams.$filterand was unaffected — meaning one view definition filtered correctly as a grid and returned everything as a calendar.2. Rule objects were spread into the
and, not wrapped['and', ...baseFilter, ...userFilter]is only correct when the source is an array of AST nodes.activeView.filteris a specViewFilterRule[], so spreading put bare rule objects where the AST expects nodes. Measured against the spec's own functions:That second line is a predicate over three columns named
field,operator,value— which don't exist. Reachable whenever a view with a filter meets a user filter value.['and', ...x]survived because it is right whenxis an array of nodes.The fix
New in
@object-ui/core:toFilterNodenormalizes one source (rule array / AST / MongoDB object),mergeFilterNodescombines sources as siblings under oneand.ObjectViewandListView.buildEffectiveFilterboth use them, so the three shapes are reconciled in one place rather than by hand at each renderer.ObjectStackAdapteralso translates a bare rule object sitting directly under a logical node — the chokepoint defence for any producer still emitting the spread shape. Only rule-shaped objects are touched; a child with nofieldis a genuine MongoDB condition ({status:'active'}) and passes through untouched.Correcting a comment I shipped in #3078
buildEffectiveFilterdocumented the dropped-object case as unreachable — "nothing in this repo produces one for a list view". That was wrong.ObjectView:1010passesmergedFiltersstraight into that schema'sfilter, and its last fallback istable.defaultFilters. The case is now handled instead of explained away, and a test pins it.The rest of the sweep — no action needed
schema.filterunchangedObjectGriddefaultFiltersdirectly — never had the.lengthbugfilter-converter's['and', ...conditions]DashboardFilterBar, lookup/field widgets, app-shell reads$filterobjectsVerification
19 tests across the four packages. Reverting each source file fails the tests covering it (adapter 2, ObjectView 1, ListView 2, plus the core merge suite). Emitted filters are asserted against the spec's own
isFilterAST/parseFilterASTrather than restated literals, including an executable pin on what the old spread shape produced — so the fix can't be undone by someone who finds the wrapping verbose.Full suite 761 files / 8856 tests green;
tscclean across core, data-objectstack, plugin-list, plugin-view; eslint 0 errors.Refs #3078, #3072, objectstack#4121
🤖 Generated with Claude Code