fix(react): guard a degenerate props bag against the object spread (#6752) - #6763
Merged
Merged
Conversation
A node written `{ type: 'card', props: 'not-a-bag' }` reached `createElement`
carrying nine React props named `0` through `8`, one per character, because
`{ ...'not-a-bag' }` enumerates a string's character indices. Nothing threw and
nothing was logged.
Triage dispatched this measurement-first: read why the sibling `properties`
branch carries a wider guard, then let the reason pick the arm. The comment
claimed the guard was hoist-specific, and that did not survive measurement —
ablating it leaves the indexed keys the hoist puts on the node unchanged (the
hoist's own `Object.entries` walk enumerates a string either way) and moves only
whether `schema.properties` still holds the authored value. The reason is
channel-independent, and `propsWithoutCanonicalKeys` already states it that way
at a non-hoist site. So `props` was missing the same guard for the same reason.
Both spread sites are covered — measured: widening the evaluation memo alone let
the nine props back in via `{ ...outgoingPropsBag }`. One `isConfigBag`
predicate now serves both bags, and the `properties` comment states the measured
reason instead of the old one.
Unchanged and pinned: a normal object `props`, objectui#5123's two-bag
precedence, the `properties` hoist, and objectui#6708's diagnostic.
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
|
os-sales
marked this pull request as ready for review
August 29, 2026 08:04
This was referenced Aug 29, 2026
finding(react): five spellings of one "is this a real config bag?" predicate in packages/react
#6761
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 #6752
Base:
b76ca6764(includes PR #6753 / card #6708,46b9bc989). Head:53185b807.The dispatch was measurement-first, so this reports the reason before the arm
Triage's order of operations: read WHY the sibling
propertiesbranch carries a wider guard, then let the reason pick the arm. Channel-independent reason means the missingpropsguard is an oversight (widen it); hoist-specific reason means the asymmetry is intended (fix the comment instead).The reason as the code states it, in the evaluation memo, verbatim before this PR:
Read alone that is hoist-specific. But the same file states the same reason a second time, at a site the hoist never reaches —
propsWithoutCanonicalKeys, the objectui#5123 narrowing:The two readings disagree, so I measured instead of picking.
What the measurement says
Ablating the
propertiesguard to bare truthiness and re-rendering{ type, properties: 'not-a-bag' }through the realSchemaRendereronb76ca6764:schema.properties0…8"not-a-bag"0…8{ "0": "n", "1": "o", … }The indexed keys are identical. The hoist's own
Object.entrieswalk enumerates a degenerate string whatever the memo guard did, so the guard does not protect the hoist — the stated reason does not survive measurement. Exactly one thing moves: whetherschema.propertiesstill holds the value the author wrote.⇒ what the guard buys is the authored value's shape, which is channel-independent, and is what the second site already says. Arm 1: the
propsbranch was missing the same guard for the same reason.Reproduction, before any change
Measured on
b76ca6764through the realSchemaRenderer, one node per row:That pre-fix reading is pasted verbatim into the pin as
BASE_READING, so the "unchanged" legs are a real before/after comparison rather than a self-fulfilling snapshot.Why the fix has two halves — also measured, not assumed
Widening the evaluation memo alone did not remove the indexed props. The bag reached
propsWithoutCanonicalKeysas the authored string, was returned unchanged, and{ ...outgoingPropsBag }at thecreateElementcall re-enumerated it. Both sites object-spread the bag, so both needed the predicate:schema.propskeeps the authored value;propsWithoutCanonicalKeys— so the spread does not re-enumerate it. This also makes its declared return type true: it saidRecord<string, any>while it could hand back a string.One
isConfigBagpredicate now serves both bags, thepropertiesguard adopts it (semantics identical), and its comment states the measured reason instead of the old one.Result
props: 'not-a-bag'0…8propscarries"not-a-bag"props: ['x','y']0,1propscarries the arrayprops: 42/truepropsprop was{}propscarries42/trueprops: { title, data }properties: 'not-a-bag'0…8Ablation
Trapped on
EXIT INT TERMwith absolute paths; mutation proven on disk by anchor counts flipping and the blob hash changing; restore proven bygit diff HEADempty and the restored blob hash equalling the HEAD blob hash (b3bdabd0c). No rebuild leg applies: the pin imports../SchemaRendereras a relative source path, so nothing resolves through packageexportstodist.Tests 3 failed | 5 passed (8). Red: the three degenerate legs. Green by name: "a normal objectpropsis byte-for-byte what it was before this card", "thepropertieschannel is untouched", and both objectui#6708 diagnostic legs.Tests 2 failed | 6 passed (8). The two indexed-prop legs go red while "the authored value survives" stays green, which is precisely the two-halves claim.Tests 8 passed (8).Verification, all on
53185b807with a clean worktreepnpm exec vitest run packages/react/Test Files 66 passed (66)·Tests 987 passed (987)pnpm --filter @object-ui/react type-checktsc --noEmitandtsc -p tsconfig.test.json, so the new test file is type-checked)eslint .inpackages/reactpnpm check:control-bytes✅ OK (scanned 5622 tracked text file(s))check-changeset-presence✅ 2 source file(s) of 1 released package(s) changed … declares 1 changeset(s)check-changeset-no-major✅ No changeset declares a major bumpforwardref-props-erasure.guard+check-node-esm-loadTests 47 passed (47)— the repo guards that pin this fileEvery exit code was captured by redirect-then-capture, never after a pipe.
Repo-wide
pnpm lintwas narrowed, and the narrowing is declared: every source file in the diff lives inpackages/react, and that package's own complete lint job ran (135 files from eslint's own--format jsonoutput, 0 errors).eslint.config.jssets noproject/projectService, so type-aware linting is off and a TypeScript change here cannot move the verdict on any file outside the diff. CI runs the full farm regardless.Scope
Unchanged and pinned: objectui#5123's two-bag precedence (a degenerate bag declares no key for either bag to win), the
propertieshoist, and objectui#6708's diagnostic (it reads the AUTHORED bag, andcollectDroppedPropsKeysalready refuses a non-object bag at both arguments). No new published surface.content/docs/releases/untouched.Filed rather than fixed here:
propertieshoist enumerates a degeneratepropertiesinto indexed node keys —properties: "text"still reaches the element as 0…8 #6760 — thepropertieshoist still enumerates a degeneratepropertiesinto0…8. Same hazard, the other channel; this card is explicitly forbidden from changing hoist behaviour. Pinned as known-and-accepted in the test rather than left silent.packages/react. This PR merged the two inSchemaRenderer.tsx; the other three live in other cards' modules.Generated by Claude Code