fix(types): one SchemaNode and one label vocabulary — the union wins, labels resolve where the locale lives (#4580) - #4608
Merged
Conversation
… labels resolve where the locale lives (#4580) Piece B: core's hand-declared `interface SchemaNode` becomes a re-export of `@object-ui/types`' union (ruling 1). Piece A: `BaseSchema.label`/`.description` widen to `string | I18nLabel`, resolved at READ time against the display locale (revised Q1-A ruling, comment 5284973826) — the compiler inventory named four read sites, all one class. Piece C: the spec bridges declare `BaseSchema` returns instead of the union (ruling Q4-B), a type annotation only. 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. |
…anode-round2 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
|
1 similar comment
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
…4580) `typesNode` and `acceptsCoreNode` are `declare`d — type-level fictions with no runtime existence — so the top-level `const coreNodeHoldsTypesNode = typesNode` type-checked correctly but threw `ReferenceError: typesNode is not defined` the moment vitest imported the module, failing the suite before any case ran. Round 1 only ever type-checked this file, so the defect was latent in the preserved pin. Moved into a never-called function: `tsc` checks the body just as thoroughly and it never executes. Re-verified that the pin still goes red when core's re-export is reverted, naming both `dist` identities. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
…anode-round2 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
|
1 similar comment
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
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 #4580.
Executes, as ONE seat with one repo-wide canary, the three pieces #4580's rulings left open: the revised Q1-A label-vocabulary ruling, the SchemaNode reconciliation (original ruling 1), and Q4-B (bridge return types).
The ruling chain
label/descriptionstaystring; bridges resolvelabel/descriptionwiden; resolve at READ timeBaseSchemaThe revision is the load-bearing one. Q1-B was ruled first and PR #4603's seat measured its premise false on every leg — the bridge is a plain class method that cannot call a hook,
BridgeContextdeclares no locale,updateContext()has zero callers. The PM revised rather than leaving the reconciliation permanently blocked:Piece A — the widening, and the inventory it produced
BaseSchema.labeland.descriptionare nowstring | I18nLabel, whereI18nLabelis the spec's INLINE locale map. Measured against the installed@objectstack/spec17.0.0-rc.6 rather than assumed:so
string | Record< string, string >— a real widening, not a no-op.The compiler inventory IS the audit. The widening turns every blind
schema.label-as-string read into a named TS2322. Repo-wide downstream canary,--continueso every package reports:components/src/renderers/complex/filter-builder.tsx:29{schema.label}into a< label >text node@object-ui/componentscomponents/src/renderers/navigation/sidebar.tsx:108{schema.label}into< SidebarGroupLabel >@object-ui/componentscomponents/src/renderers/overlay/dropdown-menu.tsx:61{schema.label}into< DropdownMenuLabel >@object-ui/componentsplugin-dashboard/src/DashboardGridLayout.tsx:328{schema.title || schema.label || 'Dashboard'}into< h2 >@object-ui/plugin-dashboardAll four report the identical error, e.g.:
Four sites, one class — well under the card's ~25-site STOP threshold, so piece A shipped.
descriptionproduced zero read-site errors. Both packages already depend on@object-ui/i18n, so both are i18n-reachable and nopackages/layout-style no-i18n-dependency workaround was needed.Red-first — the harm is a THROW, not
[object Object]Measured on the unfixed source before any edit, by invoking each registered renderer directly. All four sites, byte-identical message:
A text node is a position React refuses outright rather than stringifying, so the pre-fix harm is the whole subtree failing to render, not a cosmetic mis-render. This is worth stating because the card predicted
[object Object]or a throw; it is the throw.dropdown-menufirst measured as an EMPTY container with no throw — Radix mountsDropdownMenuContentlazily, so the label is unreachable with the menu closed. A case written that way would have shipped looking green while proving nothing.defaultOpen: trueis load-bearing in the shipped test for exactly that reason, and the test header says so.The fixes
Sites 1-3 resolve with the spec's own
resolveI18nLabelagainstuseDisplayLocale(), aliased on import following PR #4169's convention (andAppSchemaRenderer.tsx:44's use of it), because this repo has its ownresolveKeyedI18nLabelover a different vocabulary. Hooks are legal in these renderers: the registry renders its entries withReact.createElement(SchemaRenderer.tsx:621), andelements.tsx:380already callsuseDisplayLocale()in one. Two renderers gained a block body solely to host the hook.Site 4 deliberately uses
pickLocalizedagainst the active UI language instead, and this is a departure from the card's letter that I want reviewed.DashboardGridLayoutalready resolves the sibling slot —widget.title, the same inline-map vocabulary — withpickLocalized(widget.title, language)seventy lines below. Adding the spec resolver plususeDisplayLocale()would put two resolvers AND two locale channels in one component for one vocabulary, and those channels genuinely disagree:useDisplayLocale()prefers the tenant's regional default over the active UI language, so a heading and the widget titles beneath it could resolve to different languages in the same render. This is not a lenient fallback standing in for the real resolver —pickLocalizedis objectui's limb-for-limb twin ofresolveI18nLabel(objectstack#6765 aligned them deliberately), differing only in how each spells a miss (''vsundefined), pinned inplugin-list/src/__tests__/i18nLabel-resolver-parity.test.ts. The||chain is preserved exactly, and a test pins that the'Dashboard'backstop still fires.The resolver's documented fallback is pinned, not assumed
Cases cover exact match, base/region (
zh-CN↔zh), and the last-resort limb ({ 'ja-JP': … }viewed asfr), per the resolver's own doc. A resolver that only ever hits the exact limb is indistinguishable from a lookup that ignores the locale, so the other two limbs are pinned separately. String-passthrough-unchanged is pinned at every site.PIN MOVED
base-schema-label-vocabulary.test.ts'sassertionLabel/assertionDescriptionpinnedlabel/descriptionasstringunder Q1-B, and that file's header predicted in advance that "a future card that 'fixes' the bridge defect by wideningBaseSchema.labelturns them red on purpose." That is exactly what happened, on the very next card. Measured red before the move:They now pin the widened unions, annotated
PIN MOVED (objectui#4580 revised Q1), with the superseded history kept rather than rewritten. A newassertionVocabulariesAreDistinctpins thatlabelandariaLabeldo NOT collapse into each other — without it, an edit that "simplified" one union into the other would leave bothEqualpins green while silently swapping which resolver owns the slot. EveryEqualis invariant, for the reason #4593 established: a one-wayextendsis vacuous for a widening in both directions.The bridges are untouched by piece A — verified, not asserted
The reverse verification proves this directly. With piece C still applied and ONLY piece A reverted, the two bridge sites go red again at exactly the lines #4593 measured:
So it is piece A's widening — not piece C's annotation — that makes
node.label = spec.labelandnode.description = spec.descriptioncompile, and both assignment expressions are byte-unchanged.Piece B — one
SchemaNodepackages/core/src/types/index.ts's hand-declaredinterface SchemaNodebecomesexport type { SchemaNode } from '@object-ui/types';. No structural copy: a copy reproduces the defect the moment either side moves.Core's entry surface is unchanged — measured, both rounds:
packages/core/dist/index.d.tsis byte-identical across the change (sha2563d1bf0ff…21fb25on both trees).The preserved pin, restored and red-first
Round 1's withdrawn reconciliation pin was preserved byte-identical (sha256
1294d525…8e4d4) and is restored aspackages/react/src/__tests__/SchemaNode.reconciliation.test.ts. It lives in@object-ui/reactbecause the collision is only observable from a package resolving BOTH throughnode_modules; that package'stsconfig.test.jsonsets"paths": {}precisely so the twodistidentities are real.With piece B reverted, all three of its written-first predictions hold, and the error text names both identities verbatim — the #4548 class:
The pin as preserved could not run under vitest.
typesNodeandacceptsCoreNodearedeclared — type-level fictions with no runtime existence — so the top-levelexport const coreNodeHoldsTypesNode: CoreSchemaNode = typesNode;type-checked correctly but threw on import:failing the whole suite before a single case ran (
Test Files 1 failed | 44 passed,Tests 604 passed— zero test failures, a load failure). Round 1 only ever type-checked this file, so the defect was invisible there. It is moved into a never-called function, whichtscchecks just as thoroughly and which never executes. The pin was then re-verified red against a reverted core after the fix — the errors above are from that second run, so the fix is proven not to have made it vacuous.Piece C — the bridges return
BaseSchemaBridgeFn,SpecBridge.transform/transformListView/transformFormView, and both bridge functions now declareBaseSchema. Both bridges end in a singlereturn nodeon an object literal (list-view.ts:226,form-view.ts:195), so the union described nothing real while forcing a narrowing at every read.Reverse verification, piece C alone reverted — 272 errors, matching round 1's measurement exactly, across the same five suites:
P1SpecBridge.test.tsSpecBridge.test.tsFormViewSpecConformance.test.tsRowHeightDensityAgreement.test.tsListViewExportOptionsLift.test.tsby class: 134 × TS18049 (
'node' is possibly 'null' or 'undefined'), 131 × TS2339 (Property 'formType' does not exist on type 'string | number | boolean | BaseSchema'), 4 × TS2322, 2 × TS2533, 1 × TS7053. Piece C kills all 272 — no residue needed narrowing in test files.Canary — per class, before and after
Repo-wide, PREFIX filter = the downstream consumers (the direction that matters for a widening):
main; 6 errors when pinnedlist-view.tslabel/descriptionFinal state, post-merge, all three directions:
...@object-ui/types...@object-ui/core...@object-ui/reactThe collision class is worth one note: on
maintoday the canary is green, because PR #4578 sidestepped the collision rather than leaving it live. The 19-of-35 figure is #4548's measurement under its option A, and the honest way to show the defect still exists is the pin — which is why the pin, not the canary, carries piece B's red.Tests
Repo-root
vitest run --maxWorkers=2over the consumption radius, split into four runs (the single combined run exceeded the 600 s cap and was re-run split, not abandoned):Emit discipline
Both trees built with
dist/and every*.tsbuildinfocleared, through turbo so the dependency closure is built identically on both sides.@object-ui/typesb9a2769d…41d4cbboth sides@object-ui/core1c4ebaa7…839d042both sides — piece B proven types-only@object-ui/reacttransform; executable code byte-identical after comment strip@object-ui/components@object-ui/plugin-dashboardPiece A's runtime footprint is exactly the four inventory sites and nothing else. In the components bundle: one added import and exactly three resolver calls —
Bn(e.label, i),Bn(e.label, n),Bn(e.label, r)— against zero in the baseline. The remainder of that bundle's diff is a minifier alpha-rename cascade triggered by the new import shifting the identifier sequence (Bn→Vn→Hn→Un→…), not a semantic change. In plugin-dashboard the entire executable delta is one line:Per-package
.d.tsand grading — measured.d.tsdiffering@object-ui/typesbase.d.ts(the two widened unions + evidence comments);zod/index.zod.d.tsis order-only@object-ui/coretypes/index.d.ts— declaration moved to a re-export; entryindex.d.tsbyte-identical@object-ui/reactspec-bridge/types.d.ts,spec-bridge/SpecBridge.d.ts— return types@object-ui/componentsui/command.d.ts— order-only, unrelated to this change@object-ui/plugin-dashboardtypesis minor by position analysis: the diff ADDS members to unions on two authored-input-dominant properties, removes nothing, and nothing that type-checked before stops doing so — the #4593/#4603 line.coreis minor on #4569 (declaration moved, entry surface intact).reactis minor by measurement rather than by rule: the return type is NARROWED (SchemaNode→BaseSchema), which is free for callers reading the result but tightens the contract for anyone implementing aBridgeFn.components/plugin-dashboardare minor as a behavior move on the #4495/#4271/#4479 line. Never major.Two
.d.tsdiffs were checked rather than waved away, because an unexplained diff in a published declaration is exactly what a grading claim must not contain:types/zod/index.zod.d.ts— identical as a multiset of lines; pure key-order churn in tsc's zod inference, the same class PR fix(types): ariaLabel declares the keyed vocabulary, disabled accepts the predicate string (#4581) #4603 recorded.components/ui/command.d.ts— two lines,"onChange" | "value" | "type"versus"type" | "onChange" | "value"inside anOmit. Set-identical after sorting union members, and the file mentions none ofSchemaNode/BaseSchema/I18nLabel— tsc union-ordering nondeterminism, not reachable from this change.must-not-change
ComponentRendererPropsstays dual-declared (finding(types): ComponentRendererProps is the second dual declaration in the same two files as SchemaNode — core's is non-generic, types' is generic #4594). The two declarations still differ — types' is generic, core's is not — and that card measured zero consumers. Left alone deliberately, with a comment in core saying so. No side-effect widening was observed: core'sComponentRendererPropscompiles unchanged against the re-exportedSchemaNode, and core's entry.d.tsis byte-identical.toRenderableSchemainpackages/reactremains. SchemaRenderer's component-level union deliberately excludesnumber/boolean(finding(react): SchemaRenderer carries the #4422 prop erasure in a spelling every sweep and both guards are blind to —Record< string, any >instead of[key: string]: any#4548 ruling Q2), so the bridge still normalizes those after reconciliation.packages/types/src/views.ts,packages/i18nspec-formatter files, their CLAIM_DEBT lines. finding(types,i18n): eight doc comments claim alignment with @objectstack/spec symbols the spec does not export #4597 landed mid-task (PR fix(types,i18n): doc comments stop citing retired @objectstack/spec symbols (#4597) #4606) and was merged forward; finding(tooling): rule 2's tie test is symbol-agnostic, so a declaration can cite a retired spec symbol and pass because it references a different, live one #4607 (PR chore(tooling): a spec-alignment claim is judged by the symbols it cites, not by incidental ties (#4607) #4609) landed after it and was merged forward too. Neither overlaps a file this branch touches.content/docs/releases/**untouched.git checkout origin/main -- PATHSand patch/keep files throughout, nevergit stash(sharedrefs/stash). All three pieces restored and verified byte-identical by sha256;git diff HEADempty after each restore.Verification
core/src/types/index.ts, whose hand-declared interface (data?: anyplus an index signature) became a re-export. The 3 new files add 2 warnings, 0 errors.Filed, not fixed
Noted for #4605's zod-mirror drift census, per the card: this widening adds
labelanddescriptionto it.packages/types/src/zod/base.zod.tsstill declares both asz.string(), now narrower than the TypeScript declarations they mirror — the same class #4603 filed forvisible/disabled/ariaLabel. Not touched here: widening a published validator is a contract decision, and for these two slots it is the same INLINE-versus-KEYED question #4605 already holds.Generated by Claude Code