fix(react): stop the properties hoist enumerating a degenerate bag - #6784
Merged
Conversation
The hoist that copies `properties.*` onto a node's top level walked the bag
with `Object.entries` unconditionally, so a non-object value was enumerated
rather than skipped: measured on `c6732825d`, `{ type, properties: 'not-a-bag' }`
reached the element as nine React props named `0` … `8`, and
`properties: ['x', 'y']` as `0`, `1`.
Arm (a) of the two objectui#6760 left open — guard the hoist with the shared
`isConfigBag` predicate (objectui#6761) rather than rule that the hoist may
enumerate anything handed to it. The reasoning is written at the hoist: under
objectui#5123 a key gets one answer whichever channel reads it, and the other
arm would have answered one authored mistake two ways, with the reinterpreting
half falling on `properties` — the spec spelling — while the quiet half fell on
its `props` legacy alias.
Also corrects two comments the change falsifies: the evaluation memo's note
that non-objects "reach the hoist exactly as they do today", and
`propsWithoutCanonicalKeys`' explanation of its degenerate-`properties`
carve-out by what the hoist does with the value.
The objectui#6752 pin's `properties` leg is ratcheted, not weakened: it pinned
that card's known-and-open asymmetry (nine indexed props, recorded not fixed)
and now asserts none.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
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 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 #6760
The
propertieshoist walked the bag withObject.entriesunconditionally, so adegenerate (non-object) value was enumerated instead of skipped. Measured on
c6732825d, this branch's base, with no part of this card in the tree:{ type, properties: 'not-a-bag' }012345678(one per character){ type, properties: ['x', 'y'] }01{ type, properties: 42 }/trueObject.entrieswas already empty for theseNobody authored those keys; they are the walk's reading of a string's character indices.
The arm, and the reasoning (which is written into the code)
The card left two arms open and triage delegated the choice to the implementer rather than the
decision box, since both are cheap, reversible, and off the manual floor (the objectui#6708
census found zero authored degenerate config bags):
Arm (a). The full reasoning lives at the hoist in
packages/react/src/SchemaRenderer.tsx;in short, and in the order it was weighed:
2026-08-18). Arm (b) breaks it in the loudest available place: after objectui#6752,
props: 'not-a-bag'contributes no keys, so arm (b) would answer the same authoredmistake two ways depending on which of two spellings of one bag was used — and the
reinterpreting half would fall on
properties, the spec spelling, while the quiet halffell on
props, its annotated legacy alias. A rule that punishes the canonical spelling isnot a rule anyone can teach.
objectui#6752 established that by ablation rather than by reading a comment. The hoist is a
third site asking the same question about the same authored value; a third answer is drift,
which is what
isConfigBagexists to end (objectui#6761) — so the guard reads the sharedpredicate rather than inlining a seventh spelling.
properties: 'text'where a bag belongs is exactly the mistake an author — increasingly, an AI writing metadata
— makes. Arm (b) turns it into nine plausible-looking props with no warning anywhere; arm
(a) makes it inert, so the mistake stays legible as the value that was actually written.
The falsified comment — the unconditional half had already landed
Triage's unconditional half was "correct the evaluation memo's falsified comment, whichever arm
you pick". On this base that was already done: PR #6763 (for objectui#6752) landed
3fe64634bat08:04:45Z, and the triage comment was written at08:28:38Zagainst thepre-merge text.
git blameputs the replacement paragraph on that commit. So the sentence"because that value feeds the hoist" is gone from the tree already, and re-deleting it was
not available as work.
What this PR does instead is keep that corrected paragraph true under the new behaviour, and
fix two further comments this change falsifies:
hoist now refuses them, and the memo now records the measured consequence that the two guards
are in series rather than redundant (ablation 2 below);
propsWithoutCanonicalKeys's doc explained its degenerate-propertiescarve-out bysaying "the hoist and
readProps()both merely object-spread it". Half of that stopped beingtrue here. The carve-out itself is unaffected — it never rested on what the hoist does — and
the bullet now says so.
Ablations, in both directions
The card's own filing used ablation in reverse — to prove a guard was not responsible for
an output — and asked for the same here. Three legs, each on the committed fix, each proving the
mutation reached disk (blob hash moved) and each restored from
HEADwith the restore proven(
git hash-objectback to the HEAD blob andgit diff HEADempty). No rebuild is involved:the pins import
../SchemaRendererby relative path, so vitest reads source, never adist/.if (newSchema.properties)0…8return for the string bag,01for the array bag; both pins red. This card's guard is what removes them.isConfigBagback to bare truthiness, hoist guard keptpropertiesa renderer reads becomes{ '0': 'n', … }instead of'not-a-bag', because the bare spread manufactures a real bag that the hoist then enumerates legitimately. The two guards are in series.propsWithoutCanonicalKeys'sisConfigBag(propertiesBag)early return back to bare truthinessWhat does not move under the fix itself, measured by diffing the full probe reading before
and after: a real object bag hoists byte-identically (including the
type/idkeys the hoisthas always refused to copy, checked on a
page:tabsnode withproperties.type: 'line'); theauthored
propertiesvalue still reaches both channels unchanged;properties: 42andproperties: trueare byte-identical, because they were never part of the defect.Pins
packages/react/src/__tests__/SchemaRenderer.degeneratePropertiesHoist.test.tsx,with a
BASE_READINGcaptured onc6732825d(SchemaRenderer.tsxat blobe95eb4372) andpasted verbatim, so the "unchanged" legs are a real before/after comparison and the degenerate
legs pin a shape that was measured to be there before the guard removed it.
SchemaRenderer.degeneratePropsBag.test.tsx'spropertiesleg.It used to assert the whole
propertiesreading still equalledBASE_READING, and said soexplicitly as "recorded, not fixed" — it was pinning objectui#6752's known-and-open
asymmetry. This card closes it, so the equality is now false in the direction of less
leakage, and the assertion tightens with it: from "nine indexed props, exactly as before" to
"none at all". Everything that file asserts about the
propsbag is untouched, and thepre-fix shape it used to pin now lives as history in the new file.
configBag.pin.test.tsis identical atbase, at HEAD and in the worktree (
cb4c115f25d81a7bdc171bee82f1bee4988da521), as isconfigBag.ts(be08ebde7e0bd4c0b8c1ab5c8b57b748a2ad93d5). The guard reads the sharedpredicate; no seventh spelling was written.
Verification, on
00042324c(the final commit)Run from the repo root, serialised through the container's shared heavy-verify lock:
pnpm exec vitest run packages/react/—Test Files 68 passed (68),Tests 997 passed (997)pnpm --filter @object-ui/react type-check(tsc --noEmit && tsc -p tsconfig.test.json) —exit 0.
tsc -p tsconfig.test.json --listFilesconfirms both pin files andSchemaRenderer.tsxare in the checked set, so this is a measurement of the edit and not avacuously clean run over an excluded set.
eslinton the three changed files — 0 errors, 16 warnings, all pre-existingno-explicit-anyinSchemaRenderer.tsx; no added line carries a type-positionany.node scripts/check-control-bytes.mjs— OK, 5648 tracked text files.node scripts/check-changeset-no-major.mjs— no changeset declaresmajor.node scripts/check-changeset-presence.mjs— 3 released-source files changed, 1 changeset.Filed separately, out of scope here
objectui#6783 — five
readProps()copies inpackages/components/src/renderers/basic/stillobject-spread a degenerate bag on the third channel (the config bag a renderer reads). Outside
this card's fence, which is
packages/react/src/only. Unassigned, for triage.Generated by Claude Code