Uh oh!
There was an error while loading. Please reload this page.
refactor(components): derive metadata-viewer's option shape from the spec's SelectOption - #7013
Merged
Merged
Conversation
…spec `metadata-viewer.tsx` declared a module-local `interface SelectOption` — four of the five keys `@objectstack/spec/data`'s `SelectOption` declares, with `visibleWhen` dropped by silence: no `Omit` naming the narrowing, no comment saying the drop was deliberate. It is the same defect class this repo has been closing for several cards running (one contract, several hand-written descriptions, each only ever reconciled against itself). Derived now, as `type StateOption = Omit<SpecSelectOption, 'visibleWhen'>` — the `Omit`-with-named-narrowings form `metadata-admin/form-spec.ts` already uses for `FormFieldSpec.options`. The dropped key and the retained `default` are each written out beside their reason. Zero behaviour change: the type is module-local, the value it annotates arrives from `useMetadataItem` as `any`, and `StateMachineView`'s `labelOf` / `colorOf` / `initial` logic is untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
os-sam
marked this pull request as ready for review
August 31, 2026 12:36
os-sam
enabled auto-merge
August 31, 2026 12:36
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
|
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#6887
Declaration-only.
metadata-viewer.tsxhand-wrote its own shape for a select option — four of the five keys the spec declares, with the fifth dropped by silence. It is now derived from the spec, with the drop named. Zero behaviour change:StateMachineView'slabelOf/colorOf/initiallogic, and which keys it reads, are untouched.Must-answer 1 — where the authoritative declaration lives
@objectstack/spec, subpath export@objectstack/spec/dataSelectOption(export type SelectOption = z.input.of.SelectOptionSchema,packages/spec/src/data/field.zod.ts:1905; the schema itself at:217)Verified on disk, not on trust — three independent checks:
The installed package really exports it.
node_modules/@objectstack/spec@17.2.0re-exportsSelectOptionandSelectOptionParsedfromdist/data/index.d.mts, and the bundled declaration carries exactly five keys:This repo has already sanctioned it as the authority for THIS contract.
packages/app-shell/src/views/metadata-admin/form-spec.ts:91importstype SelectOptionfrom@objectstack/spec/dataand derives at:159in theOmit-with-named-narrowings form. That convergence is objectui#6247 / PR Honour per-optionvisibleWhenin the metadata-admin renderer #6618 — so the authority for the select option was settled there, not by [app-shell] metadata-admin 的FormFieldSpec没有声明dependsOn,但两个 widget 都读它 —— 类型化的 form spec 写不出这两个 widget 的必备配置 #5040 / [finding] A THIRD inline copy of the form-field authoring contract lives in apps/console FormPage.tsx — objectui#5040 converged only the app-shell two #5542 (those convergedFormFieldSpecitself; A per-optionvisibleWhenwritten in a*.form.tsis silently inert — the metadata-admin renderer never reads it #6247 converged itsoptionselement type, which was the last hand-written position in that file).A second, independent binding to the same authority already exists.
packages/types/src/zod/form.zod.ts'sSelectOptionSchemais a spec derivation, pinned bypackages/types/src/__tests__/select-option-spec-parity.test.ts— which asserts the local schema carries every spec key and extends it by exactly two documented UI-only keys.Two consumers already deriving from the same export, one of them test-pinned, is what makes this settled rather than a choice I made. ⛔ Nothing here derives the fifth copy from the fourth.
What changed
packages/components/src/renderers/basic/metadata-viewer.tsx— the module-localinterface SelectOptionis replaced by a derivation, and the local binding is renamed so a narrowed shape no longer sits under the spec's own export name (the planted-premise hazardcheck-spec-symbol-derivation.mjsexists to stop):Each narrowing is now named, with its reason next to it:
visibleWhenis DROPPED, and now says so. This view draws the declared state set — every option the field declares becomes a chip, and the graph comes from thestate_machinerule. Per-optionvisibleWhenwithdraws an option from a picker for one record and onecurrent_user(ADR-0068); a diagram of the machine has no such subject, so there is nothing here to evaluate it against.defaultis KEPT, and is read atinitial. That is the whole reason this is a spec derivation and not a narrower local shape:defaulton the object-field face is ruledenforce(objectstack#7246, implemented by PR #7388), so the state this view calls "initial" is the one the platform actually writes.SelectOption(z.input) rather thanSelectOptionParsed, even though this is a read model: on the four retained keys the two are the same type — none oflabel/value/color/defaultcarries a.default()or a transform — so the entire input/output difference lives on the omitted key.Structural equivalence — one difference, stated precisely
The derived type is not structurally identical to the hand-written one. It admits one shape less:
labellabel?: stringlabel: string(required){ value }with no labelvaluevalue: stringvalue: stringcolorcolor?: stringcolor?: stringdefaultdefault?: booleandefault?: booleanThe hand copy had relaxed the spec's required
labelto optional — by silence, the same way it droppedvisibleWhen. It is the same divergence objectui#5899's census recorded forAppLike/ObjectLike/RemoteTable/FlowRuntimeState(each relaxing a spec-required key) and forFlowNode(which also makeslabeloptional where the spec requires it).Nothing turned on the relaxation, and no behaviour follows from removing it:
useMetadataItemasany(getFieldreturnsany), so nothing is type-checked against this declaration in either direction;tscis green.labelOffalls back to the raw value for any state the option list does not describe at all — transition sources and targets are not required to be declared options — so the fallback is load-bearing for a case that has nothing to do with a missinglabel, and is unchanged.Must-answer 2 — the enumeration: a sixth copy exists, and a seventh
"The fifth" was the card's self-report. Measured now: 8 further hand-written descriptions of this contract remain, in three tiers. ⛔ None is touched by this PR.
Tier 1 — named types describing the same contract (3)
packages/types/src/field-types.tsSelectOptionMetadata—label,value,color?,icon?,disabled?,visibleWhen?defaultis absent. This is the object-definition field face, the very facemetadata-viewerreads, and it cannot describe the ruled-enforcekey. Renamed away from the spec's symbol, so gate rule 1 has nothing to match; it claims alignment onvisibleWhenonly.packages/types/src/form.tsSelectOption— addsdisabled/icon, widensvalueselect-option-spec-parity.test.ts+zod-mirror-parity.test.ts. Hand-written TS, transitively backed.packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.tsxinterface Option { value: string; label?: string; color?: string }labeldivergence this PR just removed, plusdefaultandvisibleWhenboth absent. No claim comment, no pin — and this one is the object-field options editor, i.e. an authoring surface, unlike the read model here. The closest sibling to this card; the strongest candidate to fold next.Tier 2 — anonymous inline restatements of the same triple (5)
packages/app-shell/src/services/MetadataService.tsoptions?: Array.of.{ label: string; value: string; color?: string }onFieldMetadataPayloadpackages/types/src/data-display.tsoptions?: Array.of.{ value: any; label: string; color?: string }packages/types/src/reports.tsoptions?: Array.of.{ value: string | number; label: string; color?: string }packages/plugin-dashboard/src/recordFields.tsxoptions?: Array.of.{ value: any; label: string; color?: string }onFieldMetapackages/plugin-list/src/UserFilters.tsxResolvedOption { label; value; color?; count? }(adds a resolved-onlycount)Tier 3 — declared-deliberate, already derived downstream (1, listed for completeness)
packages/core/src/evaluator/optionRules.ts:40—OptionLike { label; value; visibleWhen? }, documented in place as a deliberately minimal reader shape kept structurally assignable-from the richer option types.packages/fields/src/widgets/MultiSelectField.tsx:10,RadioField.tsx:10andCheckboxesField.tsx:10derive from it (extends/ alias) rather than restating it — the behaviour this class wants.Two conduits deliberately excluded (both already reasoned in place, both carrying a
[key: string]: unknowncatch-all so they do not restate the vocabulary):ActionParamOption(packages/core/src/actions/ActionRunner.ts:582) andRawActionParamOption(packages/app-shell/src/utils/resolveActionParams.ts:69). Likewise excluded:RadioOption/ComboboxOption(distinct component vocabularies with their own spec schemas) and the variousFieldOptionshapes inplugin-tree/plugin-view(field pickers —valueis a field name, a different concept).One measured negative worth recording: after this PR,
packages/types/src/form.ts:299is the only declaration left in the tree that carries a select option'sdefaultkey. The object-definition face (SelectOptionMetadata, #1 above) does not, and neither does the authoring editor (#3).Gate readings — before and after, unchanged, and why
node scripts/check-spec-symbol-derivation.mjs, both runs exit 0:Identical, as expected — this fix is invisible to the gate, and the card's account of why is half the story. The card noted the declaration carries no alignment claim, so rule 2 has nothing to match on. The other half is in the script's own header: both rules skip any declaration without an
exportmodifier (hasExportModifier, once per scanner). This declaration is module-local, so rule 1 could not see it either — even though it was named after a spec export and had drifted, which is exactly rule 1's trigger. objectui#5899 measured that hole: forcing the filter true takes rule 1 from 18 findings to 47, of which 22 of 30 additional sites are real mirrors and 12 carry a divergence measurable today. This declaration was one of that population; it no longer is.⇒ The gate numbers not moving is the correct outcome, not a missing step. Closing the class the gate cannot see is the card's third candidate direction and is left to the PM.
Verification
All at
2843b62e7, the final commit.pnpm --filter '@object-ui/components^...' buildpnpm --filter @object-ui/components type-check(=tsc --noEmit && tsc -p tsconfig.test.json)pnpm vitest run packages/components/src/__tests__/metadata-viewer.test.tsx(root-relative)npx eslint packages/components/src/renderers/basic/metadata-viewer.tsx --format jsonno-explicit-anywarnings, none on a line this diff adds (the diff adds zeroany)pnpm check:spec-symbolspnpm check:control-bytespnpm check:phantom-deps@objectstack/specis already a declared dependency of@object-ui/componentspnpm check:self-import,pnpm check:esm-specifiersnode scripts/check-changeset-presence.mjsnode scripts/check-changeset-no-major.mjsCoverage is measured, not assumed.
packages/components/tsconfig.jsonexcludessrc/__tests__and**/*.test.tsx, so the firsttscleg says nothing about test files;tsc --listFilesconfirms the edited file is in the main project (1 hit) and the test file is intsconfig.test.json(1 hit). The eslint run is a declared narrowing to the one changed file: the flat config usestseslint.configs.recommendedwith noparserOptions.projectand noprojectService, so type-aware linting is off and this diff cannot move the verdict on any file it does not touch; the repo-wide pass belongs to CI.Reverse verification — the derivation is really bound to the spec
Run from the committed state, so the restore leg has a real reference point. Two probes injected at the type alias; predicted direction red, observed red:
TS2353 proves the
Omitreally removed the key and that the alias resolves to a real object type out of the spec's.d.mtsrather than collapsing toany. TS2741 independently confirms thelabeldivergence documented above. Restore was then proven, not assumed:git checkout HEAD -- ABSOLUTE_PATH,git diff HEADempty, restored blob35a8621dequal to the HEAD blob, probe residue 0 — andtscback to exit 0 on the restored tree.Out of scope, deliberately
default(enforce, objectstack#7246 / PR #7388) and the alias table (isDefault/selected) are untouched. This PR is about where a shape is declared, never about what a surface honours.Generated by Claude Code