fix(types): ariaLabel declares the keyed vocabulary, disabled accepts the predicate string (#4581) - #4603
Merged
Merged
Conversation
… the predicate string (#4581) Executes #4580's label-vocabulary rulings Q2-B and Q3-A. PR #4593 landed the `visible` half and escalated the rest. Q2-B: `BaseSchema.ariaLabel` widens from `string` to `string | KeyedI18nLabel`, where `KeyedI18nLabel` is a new exported type in `@object-ui/types` naming the keyed form `{ key, defaultValue?, params? }` that `SchemaRenderer.tsx:111` actually resolves via `resolveKeyedI18nLabel`. Deliberately NOT the spec's `I18nLabel` (the inline locale map) — PR #4593 measured that spelling accepting the keyed fixture only vacuously, rejecting `params`, and type-checking a genuine locale map that renders an empty aria-label. The three inline copies of the keyed shape were verified identical before the name was minted; react/utils/i18n.ts and layout/NavigationRenderer.tsx now import it. app-shell/utils/index.ts keeps its inline spelling because PR #4208 has a pending change to that file — comment added naming the type and the wait. Q3-A: `BaseSchema.disabled` widens from `boolean` to `boolean | string`, same evidence as `visible` (SchemaRenderer.tsx:466 evaluates it through the same `evaluateCondition`; `disabledOn?: string` sibling). Three `as unknown as BaseSchema` casts drop (aria :59, expressions :132/:141), closing the class the #4581 census named. Q1-B is NOT implemented: its premise measured false. The spec bridges have zero production callers, `BridgeContext` has no locale field, the display-locale channel is a React hook unusable from a plain class method, and nothing re-runs the bridge on a locale change. `BaseSchema.label`/`.description` stay `string` and are pinned that way. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
…tead of forwarding it raw (#4581) Found by the downstream type-check sweep over consumers of @object-ui/types, not by a test: with `BaseSchema.ariaLabel` honestly declared as `string | KeyedI18nLabel`, `aria-label={schema.ariaLabel}` in packages/components/src/renderers/form/toggle.tsx stops type-checking against Toggle's `aria-label: string | undefined`. The runtime harm is narrower than it first looks, and the test says so. Through SchemaRenderer the bug is INVISIBLE: SchemaRenderer injects its own already resolved `aria-label` via `...ariaProps` (SchemaRenderer.tsx:599/:625) and this renderer spreads `{...props}` after its own attribute, so the resolved value wins. Invoked directly the raw forward emits `aria-label="[object Object]"`. My first prediction — that the harm would show through SchemaRenderer — was wrong, and the test header records that rather than being rewritten to match: a SchemaRenderer-driven case is green in both directions and would have shipped looking like proof. The discriminating case invokes the registered renderer directly via ComponentRegistry.get('toggle'). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
This was referenced Aug 13, 2026
yinlianghui
marked this pull request as ready for review
August 13, 2026 18:44
This was referenced Aug 13, 2026
Closed
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
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 #4581.
Executes the label-vocabulary rulings recorded on #4580 (comment 5284007579) — the follow-up seat that ruling asked for. PR #4593 landed the
visiblehalf and escalated the rest; this lands Q2-B and Q3-A, and stops on Q1-B because its stated premise measured false.Q2-B —
ariaLabeldeclares the KEYED vocabulary, and the shape gets a nameBaseSchema.ariaLabelis nowstring | KeyedI18nLabel, whereKeyedI18nLabelis a new exported type in@object-ui/types:That is the vocabulary the renderer actually resolves —
packages/react/src/SchemaRenderer.tsx:111:Not
I18nLabel. The original #4581 text asked forstring | I18nLabel; PR #4593 measured that spelling wrong in three ways and the ruling withdrew it.I18nLabelis the spec's INLINE LOCALE MAP (string | Record< string, string >), so it accepted the shipped keyed fixture only vacuously — as a locale map whose "locales" are namedkeyanddefaultValue— rejected the same label carryingparams, and type-checked a genuine{ en: 'Owner' }thatresolveKeyedI18nLabelreturnsundefinedfor, rendering an empty aria-label. The two vocabularies are structurally confusable: #4167's exact hazard.The keyed-shape census (verified before the name was minted)
All three inline copies agreed exactly on the object half:
packages/react/src/utils/i18n.ts:38{ key: string; defaultValue?: string; params?: Record< string, any > }string | … | undefinedpackages/layout/src/NavigationRenderer.tsx:261string | …(noundefined)packages/app-shell/src/utils/index.ts:63string | … | undefinedSo
KeyedI18nLabelnames the object half only; each site keeps its own wrapper. It is a name for what was already there, not a new capability.#4208 train-overlap decision, per file — measured, not assumed
The card flagged PR #4208 (the rc.6 train, open and blocked on #4165). GitHub's file list for #4208 is computed against a stale merge-base (
6314e87f2), so it lists files whose changes have since landed onmain. I measured the content delta instead (git diff origin/main refs/remotes/pr/4208 -- PATH):mainpackages/react/src/utils/i18n.tspackages/layout/src/NavigationRenderer.tsxpackages/app-shell/src/utils/index.ts./appRoutere-export blockapp-shell/utils/index.tsas the overlapping one. On the stale-merge-base file list both appear; on the content measurement the app-shell judgement is right and the react one is right for a different reason than stated. I followed the measurement. For the record, the app-shell hunk sits ~25 lines from the inline copy (the re-export block, not the signature), so a swap there would most likely not have conflicted either — I still left it alone, because the rule was file-granular and app-shell is also #4024's in-flight package. Only that file's doc comment is touched there; nothing else in app-shell.Q3-A —
disabledaccepts the predicate stringBaseSchema.disabledis nowboolean | string, on exactly thevisibleevidence one slot over:SchemaRenderer.tsx:466evaluates it through the sameevaluateCondition((condition: string | boolean | undefined, context?) => boolean), and thedisabledOn?: stringsibling exists for the same reason. The asymmetry was accidental.Casts dropped — the census is now closed
SchemaRenderer.expressions.test.tsx:132/:141(the twodisabledcasts) andSchemaRenderer.aria.test.tsx:59(theariaLabelcast) are gone, and both files'BaseSchemaimports went with them. The expressions header explicitly said "drop them when that lands" — this is that landing. A new aria case pins a keyed label carryingparams, the limb the withdrawn spelling rejected.Of the six
as BaseSchemacasts PR #4593's census recorded, this card's ruled widenings close the three that were this gap. The remaining three are the other classes that census identified and are untouched:components/__tests__/html-anchor-links.test.tsx:33(aRecord< string, unknown >spread), andplugin-dashboard's two #4548 narrowing casts.The consumer sweep found a real defect —
@object-ui/componentsDeclaring the vocabulary honestly is what surfaced it, and a downstream
type-checksweep is what caught it, not a test.packages/components/src/renderers/form/toggle.tsxis one of the few renderers that writesaria-labelITSELF instead of relying on SchemaRenderer'sresolveAriaProps, and it forwarded the value raw:Under
string | KeyedI18nLabelthat stops type-checking againstToggle'saria-label: string | undefined. It now resolves. Invoked directly, the raw forward emittedaria-label="[object Object]"— announced verbatim by a screen reader.My first prediction about this was wrong, and the test header records that rather than being quietly rewritten. I predicted the harm would show through
SchemaRenderer. It does not: SchemaRenderer injects its own already-resolvedaria-labelvia...ariaProps(SchemaRenderer.tsx:599+:625), and this renderer spreads{...props}AFTER its own attribute, so the resolved value always wins and the raw expression is shadowed. A SchemaRenderer-driven test is therefore green in both directions — vacuous, and it would have shipped looking like proof. The discriminating case invokes the registered renderer directly viaComponentRegistry.get('toggle'), and that one measured, verbatim:Both paths are kept in
packages/components/src/__tests__/toggle-aria-label-keyed.test.tsxand each is labelled for what it can and cannot show.⛔ Q1-B — STOPPED. The premise measured false on both halves.
The ruling made Q1-B conditional: "the bridge's invocation context can know the locale AND re-translates on locale change — if measured otherwise, STOP and report." Measured otherwise, so no bridge code is touched here.
(a) The call path, and who calls it.
SpecBridge.transformListView(SpecBridge.ts:38) →transform()(:30) →return bridge(spec, this.context)(:34) →bridgeListView(bridges/list-view.ts:166), sites:180and:224.BridgeContext(spec-bridge/types.ts:12-20) declares exactlyuser?,variables?,objectDefs?— no locale field.An exhaustive grep for
SpecBridge|bridgeListView|bridgeFormView|transformListView|transformFormViewacross all ofpackages/andapps/finds zero production callers. Every reference is the module itself, the barrel re-export (spec-bridge/index.ts,react/src/index.ts:14), or a test — five suites underreact/src/spec-bridge/__tests__/plusplugin-grid/src/__tests__/specBridgeExportFormats.test.tsx:73. Every in-repo construction isnew SpecBridge()ornew SpecBridge({ user: … }). It is a published API surface with no in-repo consumer.(b) Can it know the locale? No. The display-locale channel is
useDisplayLocale()—packages/i18n/src/useDisplayLocale.ts:53, a React hook composed of two further context hooks (useLocalization,useObjectTranslation).bridgeListViewis a plain function invoked from a plain class method, not a component render. Rules of hooks, not a style preference.(c) Does a locale change re-run it? No.
SpecBridgeassignscontextonce in its constructor to a mutable field and reads it at call time. There is no memoization, no React subscription, no invalidation channel;updateContext()(:47) has zero callers.transform()returns a plain object, so a resolved string is frozen into it at transform time.So even if
BridgeContextgainedlocale?: string, resolving at the bridge would bake one audience's language into the node tree with no re-translation channel — the freeze the ruling named as a stop condition. The spec's own resolver doc warns about this class of defect in as many words: thelocaleparameter is "positional rather than optional so that a producer cannot silently ship one audience's language to every audience by forgetting an argument — the defect #6761 records".BaseSchema.labeland.descriptiontherefore staystring(which is the ruling), and the twolist-view.tssites are unchanged. #4580's Q1-B blocker is NOT cleared by this PR — the seat's report carries the measured alternative (read-time resolution in the renderer againstuseDisplayLocale(), which is how every other localized read site in this repo already works) and the tension that makes it a real question rather than a mechanical follow-up: read-time resolution requires the renderer to accept an inline map inschema.label, i.e. theBaseSchema.labelwidening Q1-B deliberately declined.Red-first — predictions written into the test header before the run
packages/types/src/__tests__/base-schema-label-vocabulary.test.ts, the sibling of #4593'sbase-schema-visible-predicate.test.ts. Againstorigin/main(52d878a3b),tsc -p packages/types/tsconfig.test.jsonreported, verbatim:Post-fix: clean, exit 0.
One prediction was half wrong, and it is left standing in the header rather than rewritten to match. I predicted
assertionAriaLabelandassertionKeyedShapewould both report TS2344 pre-fix; onlyassertionKeyedShape(120) did — TypeScript suppresses the cascade once the import is an error type, soassertionAriaLabel(126) reported nothing. That is why the header flagged those two as non-independent evidence in advance: a pin whose pre-fix silence is a compiler artifact proves nothing on its own, and the load-bearing reds are the other four.Every
Equalis invariant, for the reason #4593 spelled out and this card inherits: asatisfies-style or one-wayextendscheck is vacuous for a widening in both directions — the narrowstringis assignable to the widestring | KeyedI18nLabel, so a widening that never happened and a widening that overshot toanywould both stay green.BaseSchema's[key: string]: anymakes the overshoot live, not hypothetical.must-not-change
label/descriptionstaystring— pinned, not merely left alone.assertionLabelandassertionDescriptionare the Q1-B ruling written down: the only two pins in the file expected green pre-fix, and a future card that "fixes" the bridge defect by wideningBaseSchema.labelturns them red on purpose. Confirmed in the published.d.ts:label?: string,description?: string, unchanged.git checkout origin/main -- PATHS(nevergit stash— sharedrefs/stash), the pins re-reported the same errors, then restored from a patch file and verified byte-identical by sha256 across all 7 tracked files.Per-package grading — measured
Both trees built with
dist/and*.tsbuildinfocleared, then every published.d.tscompared file-by-file..d.tsfiles differing@object-ui/typesbase.d.ts(+KeyedI18nLabel, two widened unions, evidence comments),index.d.ts(+the export) — plus 2 zod files that are order-only, see below@object-ui/reactutils/i18n.d.ts—resolveKeyedI18nLabel's parameter names the type instead of inlining it@object-ui/layoutNavigationRenderer.d.ts— same, forresolveLabel@object-ui/app-shellutils/index.d.ts— doc comment only; the inline copy is deliberately not swapped@object-ui/componentstypesis minor by position analysis, not assumption: the diff ADDS an exported name and ADDS members to unions on two authored-input-dominant properties. It removes nothing, and nothing that type-checked before stops doing so — the #4593/#4586/#4591 shape.react/layoutare patch because the type is structurally identical: naming a shape is a spelling change in the.d.ts, not a contract change.app-shellis patch under the "grade patch if the published.d.tschanges at all" rule — flagged plainly because no consumer can observe a comment, so a reviewer who would rather drop that entry has my measurement to do it with.Never major (major tracks
@objectstack).The two zod
.d.tsdiffs are build nondeterminism, not this changetypes/zod/complex.zod.d.tsandtypes/zod/index.zod.d.tsdiffer between the two builds. They are identical as a multiset of lines (verified by sorting both and comparing hashes) — pure key-ORDER churn in tsc's zod inference, of thepriority: { medium, critical, low, high }reordering kind. Neither mentionsKeyedI18nLabel; no zod.d.tsin the package does. I checked rather than waved it away because an unexplained diff in a published declaration is exactly what a grading claim must not contain.Verification
vitest run --maxWorkers=2over the radius (react, components, types, layout, app-shell) — 603 files, 5947 passed, 1 skipped. Touched suites re-run separately, green....@object-ui/types→ 77/77 packages,...@object-ui/react→ 67/67. Zero errors. The first run of that sweep is what found thetoggle.tsxdefect above.no-explicit-any;KeyedI18nLabel's ownparamsadds one back).git checkout PATHSat the fork point (nevergit stash— sharedrefs/stash), the six pins re-reported the same errors, then restored and verified byte-identical by sha256 on all six files.Filed, not fixed
#4605 — the zod mirror in
packages/types/src/zod/base.zod.tsstill declaresvisibleanddisabledasz.boolean()andariaLabelasz.string(), now narrower than the TypeScript declarations they mirror.visiblehas been drifted since PR #4593; this PR adds the other two. Measured as observation-class: nothing enforces it on the render path (core'svalidateSchemais hand-written and imports only the TS type;SchemaRenderer's__DEV__check only sets adata-obj-schema-invalidattribute), but@object-ui/types/zodis a published validator that would reject spellings the published types invite. Widening it is a contract decision — especially forariaLabel, where the keyed object must not be confused with the spec's inline map — so it is filed rather than guessed at here.Generated by Claude Code