Uh oh!
There was an error while loading. Please reload this page.
fix(react): canonical properties wins on the React-prop channel too — one answer per key on both channels (#5123) - #5284
Merged
os-support-ai merged 2 commits intoAug 19, 2026
Conversation
A node carrying both `props` (legacy alias) and `properties` (spec spelling)
got a different answer for the same key depending on which channel read it, and
the two channels' precedence was opposite: the config bag (`readProps()` =
`{ ...props, ...properties }`) let `properties` win, while `createElement`
spread `...(evaluatedSchema.props || {})` last and let `props` win.
Maintainer ruling 2026-08-18: `properties` wins on BOTH channels. The config-bag
order was already correct and is untouched; this narrows the alias spread so it
no longer overrides a key the canonical bag also declares.
Closes objectui#5123Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
os-support-ai
marked this pull request as ready for review
August 19, 2026 00:52
Uh oh!
There was an error while loading. Please reload this page.
os-support-ai
deleted the
claude/issue-5123-properties-wins-both-channels
branch
August 19, 2026 00:52
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
1 participant
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#5123
Requirement (1) first: the co-occurrence scan, because the ruling made it a pre-condition
Count of nodes carrying BOTH
propsandpropertiesin shipped metadata or source: 0.One co-occurrence site exists in the whole repo, and it is a test fixture —
packages/react/src/__tests__/SchemaRenderer.propertiesExpressions.test.tsx(the objectui#4799pin), which deliberately writes both spellings to pin the config-bag merge. Its rendered output
does not change: it asserts the bag read (
propertieswins, unchanged) and the two bags'evaluated values on the schema (both preserved). It passes unmodified — see the full-suite run
below.
So the blast radius of this change on authored metadata is empty, and the honest risk the
dispatch stated ("one of the two currently-working readings changes behaviour") lands on no
existing site. That is a measurement, not an assumption — here is how it was taken, and the
counter-probe that proves the corpus was actually read.
Method
A walker (
JSON.parse+ recursive object walk for every.json; a brace-depth, string- andcomment-aware object-literal scan for every
.ts/.tsx/.js/.jsx/.mjs/.md/.mdx) over the wholerepo including
examples/. Both passes were validated against a positive control — a syntheticnode written both ways — and both detected it.
propspropertiesexamples/(JSON)Counter-probe — why the zeros are readings and not an empty scan
A zero is only worth as much as the probe that shows the same scan finds things it should:
examples/: the same walk parsed 2277"type"keys across 428 files in that tree. Thecorpus is genuinely being opened — it simply authors config flat on the node
(
{ "type": "statistic", "label": "…", "value": "…" }), and contains zero occurrences ofeither
"props"or"properties"as a key. The co-occurrence zero there is structural.props, 344 containingproperties, 12containing
readProps, 3 JSON nodes carrying exactly one of the two spellings, and 28non-test source literals declaring a
properties:bag against 1 declaring aprops:bag.the source pass alike, so a real site could not have been missed by a dead detector.
The ruling — binding, quoted
Maintainer, 2026-08-18 (verbatim 「其他接受你的建议」):
The defect, re-measured on current
mainrather than trusted from the cardThe card is from 2026-08-18 02:22 and the tree moved since, so the spread order was re-read at
97fba31a2. It is unchanged, and the two channels still disagree in opposite directions:schema.properties.x)propertieswinsreadProps()={ ...schema.props, ...schema.properties }xarriving as a prop)propswinscreateElement:...componentPropsfirst, then...(evaluatedSchema.props || {})overwritesReproduced through a real
SchemaRendererrender of one node carrying both, reading bothchannels in the same render:
The
readProps()family was re-verified on currentmainas the dispatch required. The fivenamed files are all still present and all still
{ ...fromProps, ...fromProperties }(
elements.tsx,text-input.tsx,record-picker.tsx,data-list.tsx,metadata-viewer.tsx).A sixth has appeared and it matters enough to name:
packages/plugin-detail/src/renderers/record-alert.tsxdefines its ownreadPropsas{ ...schema, ...schema.properties }— a different expression of the sameproperties-winsorder, so it agrees with the ruling and needs no change. Its suite is green (below).
PR #5122 (#4799) is confirmed not to be the cause — it preserved
readProps'spropertiesprecedence, and no side effect of that merge was found or looked for beyond that confirmation.
The change
One site, in
packages/react/src/SchemaRenderer.tsx. The React-prop channel moves; theconfig-bag channel is untouched, exactly as ruled.
The alias spread is narrowed so it no longer overrides a key the canonical bag also declares:
Why subtracting from
propsrather than re-spreadingpropertiesafter it — this is theload-bearing implementation choice.
properties.*is already on the node via the hoist, so itsvalues are in
componentPropshaving passed the metadata strip. Re-spreading the raw bagwould push stripped schema metadata back out as React props, which is precisely the regression
where a spec-documented
dataSourcebinding shadowed the injected adapter and broke thecomponent (objectstack#5576). Subtracting adds no key to the outgoing bag at all; it only
decides which of two co-present values a key carries.
Scope is deliberately narrow — a reading moves only where both bags declare the same key:
propsdeclares is untouched (the alias keeps working);propertiesdeclares is untouched (it already won);type/idare skipped, because the hoist never copies a canonical value up for them, sodropping the alias would delete the prop rather than replace it;
propertiesis left alone — the hoist andreadProps()bothmerely object-spread it, and there is no canonical bag to prefer.
Each of those four carve-outs is pinned by a test, so they are decisions on the record rather
than accidents of the implementation.
Requirement (2): the invariant is pinned cross-channel, not per-file
A test covering only the channel this PR edited would let the two drift apart again — which is
the defect class here, not the individual spread order. So both new files assert the same
key through both read paths, and the central assertion is an equality between the channels
(
expect(bagRead).toBe(reactPropRead)), which fails if either side is re-inverted later, evenif someone edits the per-channel expectations to match a new reading.
packages/react/src/__tests__/SchemaRenderer.aliasPrecedenceCrossChannel.test.tsx— 8 tests.One probe reads the config bag and its own React props in a single render of a single node.
packages/components/src/__tests__/alias-precedence-cross-channel.test.tsx— 4 tests, withno stand-in for either path:
element:text(the realreadProps()family) andbadge(the real spread path) driven through the real
SchemaRenderer.Requirement (3): adjacency to #4795's pending question ②
#4795 question ② asks whether the
propertiesenvelope is an officialui:*authoring channelat all. This PR does not answer that, and takes no position on it. What is settled here is
only precedence between two spellings when both are already present on a node. In particular
the
propsalias is not retired here — it keeps working wherever the canonical bag does notclaim the same key. That distinction is written into both test files so a later reader cannot
mistake these pins for a blessing of the envelope. #4795 stays open and unaffected.
Verification
All of the following measured at
aca492561, the final commit on this branch, re-run afterthe last commit so the reported green describes this tree and not an earlier one.
pnpm exec vitest run packages/react packages/components(repo root)pnpm exec vitest run packages/plugin-detail packages/layout packages/plugin-designerpnpm --filter @object-ui/react --filter @object-ui/components type-check... lint... buildnode scripts/check-control-bytes.mjsnode scripts/check-changeset-presence.mjsnode scripts/check-changeset-no-major.mjsnode scripts/check-changeset-fixed.mjsnode scripts/check-type-check-coverage.mjsnode scripts/check-lint-coverage.mjsSuites were run from the repo root, not via a package-level filter (objectui#3378).
Changeset:
patchon@object-ui/react, nevermajor.Reverse verification — predicted before running
The original
SchemaRenderer.tsxwas restored fromorigin/mainwith the new tests kept, andthe outcome was predicted before the run: 5 of 12 red — the 3 co-occurrence cases in
packages/react, plus thebadgespread-path case and the cross-family equality case inpackages/components; the 7 single-spelling and carve-out cases staying green.Observed: exactly that. 5 failed, 7 passed, and the 5 were the 5 named ones:
That the 7 stayed green is the half that matters for blast radius: the single-spelling readings
are provably not moved by this change, in either direction.
Is a rebuild in the ablation's resolution path? No — and it cannot hide a stale result. The
root
vitest.config.mtsaliases@object-ui/reacttopackages/react/src(line 252), so bothsuites resolve the renderer from source, never from
dist/. This was not taken on trust:the ablation was run with
dist/still containing the fixed build (verified: 2 occurrencesof the new helper in
packages/react/dist/SchemaRenderer.jswhilesrchad 0). The tests wentred anyway, which is positive proof that
distis not consulted — had a stale artifact been inthe path, the ablation would have gone green and reported a false negative. The fix was then
restored with
git checkoutof this branch and confirmed byte-identical toHEAD(
git diff --quietclean) before the final measurement above.Generated by Claude Code