Uh oh!
There was an error while loading. Please reload this page.
Record tree and chart as host-composition-only view types, and give tree its icon (#5321) - #5589
Merged
Merged
Conversation
…its icon (#5321) `ObjectView.generateViewSchema` renders eight view types; the unions an author writes (`ObjectViewSchema.defaultViewType`, `NamedListView.type`) admit six of them, so `tree` and `chart` are reachable only through the component's `views` prop. Per the maintainer ruling of 2026-08-20, both stay recorded as host-composition-only surfaces rather than being added to those unions, following the objectui#5097 precedent. - `OBJECT_VIEW_HOST_COMPOSITION_VIEW_TYPES` records the exemption next to the #5097 one, with the two branches noted in place and the switch fenced by a `#region` so the branch set can be re-derived. - The record is pinned in the #5097 ledger: the branch set comes from the fence, and type-level assertions fail `type-check` if either authored union grows a `tree` or `chart` member. - Reachability is measured in a new suite: a host `views` prop still drives both branches. - Behaviour: `iconMap` is now total over `ViewType` and gains `tree: 'list-tree'`, so a host tree view stops falling through to the `'table'` fallback and the grid glyph. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012u2pRjcqAYtoEjgr3wwhnK
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-sales
marked this pull request as ready for review
August 21, 2026 15:47
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes#5321
Verified at
d5895ecaf.The ruling this implements
ObjectView.generateViewSchemaswitches on eight view types. The unions an author writes admit six of them —ObjectViewSchema.defaultViewTypeandNamedListView.typeare the same seven-value union, and neither spellstreeorchart. The only segment that can is the component'sviewsprop, typedViewType, which carries both.The maintainer ruled on 2026-08-20 (verbatim 「其他接受你的建议。」): option B — record the host-composition-only exemption, following the objectui#5097 precedent, rather than adding two members to the authored unions. Two more authored members is a permanent authoring-surface obligation with no measured pull; the exemption gets revisited the day a real metadata-authoring need arrives. Option A (widening the unions) was considered and rejected, so
packages/typesis untouched and this PR widens no authoring surface.What landed
1. The record —
OBJECT_VIEW_HOST_COMPOSITION_VIEW_TYPESinpackages/plugin-view/src/ObjectView.tsx, in the shape of the objectui#5097 constant beside it: the verdict, the ruling with its provenance, the live host path, and where each half of the claim is pinned.2. Noted at the branches — the
treeandchartcases carry a note saying they are host-only and that theviewOptions.tree.*surface is host config rather than authoring surface. Without it the next reader sees a maintained config surface and reasonably concludes it is authorable, which is how this card was filed in the first place. The switch is fenced with a#regionmarker so the branch set can be re-derived from source.3. The pins —
objectViewHostSurface.test.tsxbecomes a two-record ledger. The new record 2 asserts the fenced branch set is the eight the card measured, that no recorded name is stale, and that the record still names exactly the two the ruling was made on. The compile-time half is pinned as type-level assertions, because "an author cannot select these" is a claim about what the compiler accepts and cannot be tested at runtime — types are erased before vitest runs, so a force-castdefaultViewType: 'tree'would reach the branch. A new suite,ObjectView.hostOnlyViewTypes.test.tsx, measures the other half: a hostviewsprop still drives both branches. That reachability is the exemption's whole basis, so it has to keep being measured rather than asserted in prose.4. The one behaviour change —
viewSwitcherSchema'siconMapgainstree: 'list-tree'. It had an entry for every view type excepttree, so a tree view fell through to the|| 'table'fallback and was drawn with the grid glyph. This is live, not hypothetical: the console'sCreateViewDialogofferstreeamong the view types a user can create, and those records arrive here as theviewsprop.One bounded widening, declared
The
iconMapannotation moved fromRecord< string, string >toRecord< ViewType, string >, which is more than adding the one missing key. Naming it explicitly because it is the kind of change that should not arrive unannounced:chart, by adding a single key. Nothing recorded that the map had to be complete, so the next missing member went unnoticed. One more key closes one more instance; the annotation closes the class — a futureViewTypemember now failstype-checkinstead of silently rendering as a grid.ViewSwitcher's ownDEFAULT_VIEW_ICONS— the consumer of these strings, in the same package — has always been declaredRecord< ViewType, LucideIcon >. The duty already existed on the consumer side; only the producer was partial, and that asymmetry is the defect.tree: 'list-tree'PascalCases toListTree, the same glyphDEFAULT_VIEW_ICONS.treealready names for this view type. A test asserts the two sides keep agreeing.type-checkalready ran on this package.The runtime
|| 'table'fallback stays:v.typearrives from a host prop and nothing validates it, so the compiler's totality is not a runtime guarantee. A test pins the fallback as the control.Verification
All at
d5895ecaf, each exit code captured before any pipe, each quoted from the gate's own verdict line.pnpm --filter @object-ui/plugin-view type-checktsc --noEmit && tsc -p tsconfig.test.json(silent on success)pnpm --filter @object-ui/plugin-view lint229 problems (0 errors, 229 warnings)pnpm exec vitest run packages/plugin-view/Test Files 19 passed (19)·Tests 194 passed (194)node scripts/check-control-bytes.mjscheck-control-bytes: OK (scanned 4638 tracked text file(s); skipped 85 binary)node scripts/check-changeset-presence.mjs3 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)node scripts/check-changeset-no-major.mjsNo changeset declares a major bumpnode scripts/check-changeset-fixed.mjsAll workspace packages are in the changeset fixed groupnode scripts/check-type-check-coverage.mjstest type-check coverage: 41/41 packages compile their testsnode scripts/check-lint-coverage.mjslint coverage: 46/46 packages linted, 0 with outstanding errorsTwo more gates were re-derived from the diff rather than taken from the dispatch list.
check-eager-closure-budget.mjsexits 2 here for a reason unrelated to this change — it readsapps/console/dist/eager-closure.json, which only exists after a console build, and reports itself as a broken gauge, not a passing budget. It is left to CI, which builds the console.published-dist-gatelikewise needs a full build; the new test file is excluded fromdisttwice over bytsconfig.json(by the__tests__directory rule and by the*.test.tsxname rule), which is the same path every sibling test file in this package takes.Ablation of the behaviour change. The
tree: 'list-tree'line was deleted and the suites re-run. Mutation proven on disk in both directions before reading any result — the anchored code line went1to0, the siblingchart: 'bar-chart-3'line stayed at1, andgit diff --statshowed exactly1 file changed, 1 deletion(-). Result: 1 test failed, 20 passed — only the icon assertion, so the pin is specific rather than a blanket. It failed withexpected 'table' to be 'list-tree', i.e. the exact pre-fix behaviour the card describes. Restore ran from atrap ... EXIT INT TERMand the tree came back byte-identical (git status --porcelainempty,git diff --statzero lines).Pin-liveness probe for the type-level assertions. A type-level pin that no project compiles is a phantom check, so it was proven to bite: flipping one assertion to claim
treeis authored turnedtype-checkred witherror TS2344: Type 'false' does not satisfy the constraint 'true'(exit 2) at that line. Mutation proven on disk both directions, restored by trap.No ablation for the record and the branch notes. They are documentation. There is nothing to mutate that would produce a meaningful red, and manufacturing one would be theatre.
Declared narrowing. The repo-wide
pnpm lintwas not run; the package-scopedeslint .was. Three pieces of evidence that this cannot hide a failure in the diff: (a) the population comes from eslint's own config resolution — the repo has exactly oneeslint.config.jsand no package-level config, so the package run resolves the same rule set as the repo-wide run; (b)--format jsonreports 34 files linted, 0 errors, 229 warnings, and all three changed source files are in that population by path; (c) that config enables no type-aware linting — noprojectService, noparserOptions.project, noproject:— so no rule reads type information and this diff cannot move the verdict on any file it did not touch. The four warnings the new suite adds areno-explicit-anyon its test doubles, matching every sibling suite in the directory.Found while measuring, filed separately
Two other entries in the same
iconMapsupply names that no longer resolve:chart: 'bar-chart-3'andgantt: 'gantt-chart'are absent from lucide 1.31'siconsrecord — lucide keeps them as deprecated named exports while dropping them from the by-name lookup, so the compiler cannot see it.getViewIcondoes not fall back to a default once an explicit icon is set, so those two view types render with no icon at all today. Different defect, different remedy (new names chosen, not a key added), out of this card's fence: filed as #5586. The new suite's resolvability assertion is deliberately scoped totreealone, with a comment saying why, so widening it lands with that card instead of as a surprise red.Generated by Claude Code