refactor(view): remove ObjectView's filter/sort bar, which was never connected - #3087
Merged
Merged
Conversation
…connected `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: 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 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.
The correction first
#3081 reported two
ObjectViewdefects. Only one was live. I am correcting my own claim.table.defaultFiltersdroppedand, "reachable whenever a view with a filter meets a user filter value"The shape itself is genuinely broken — a live server answers it with
400 INVALID_FILTER, 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, and the changeset said otherwise. #3081's changeset is unreleased, so this PR amends it rather than letting the wrong claim ship in release notes.What was dead
ObjectViewcarried its own filter and sort bar:filterValues/sortConfigstate, afilter-uischema, asort-uischema, ~80 lines of field introspection to build them.None of it was wired.
setFilterValuesandsetSortConfigare never called;filterSchemaandsortSchemaare each declared once and never rendered. Both states sat at their initial empty value for the component's entire life.Removed, not wired
The real filter and sort UI belongs to the renderer this component delegates to —
showFilters,showSortandfilterableFieldsare forwarded downstream andListViewimplements them (its own filter panel, plus afilterableFieldswhitelist). Connecting the local copy would have produced a second filter bar competing with that one.Why dead state wasn't harmless
It left a branch in every merge path that could never run, and those branches read as live code:
baseFilterwith auserFilterthat was always[];mergedFilters— what therenderListViewslot receives (Studio design surface) — opened with a branch that replaced the view's filter with the user's instead of combining them. That would have been a real bug had the state ever been written.That is what made the #3081 misreading possible. It happened twice in one session: I nearly reported
{$and:[…]}as a third defect too, before measuring showed it reaches the same server predicate on both routes. Code that looks live and cannot run is a trap for the next reader, which is the argument for deleting it.Verification
No behaviour change — every removed branch was unreachable. Net -85 lines.
4 new tests pin what the component hands the delegated renderer, alongside the 5 from #3081 covering what it queries with. Full suite 763 files / 8915 tests green;
tscclean; eslint 0 errors.Refs #3081, #3072
🤖 Generated with Claude Code