fix(components): one config-bag reader for element renderers, asking the shared predicate - #6789
Merged
Merged
Conversation
…the shared predicate
Five copies of `readProps()` under `renderers/basic/` object-spread a degenerate
config bag: `??` only replaces `null`/`undefined`, so `properties: 'not-a-bag'`
was re-read as `{ '0': 'n', … '8': 'g' }` — nine keys nobody authored. This is
the third and last channel of the hazard objectui#6752 and objectui#6760 closed
upstream in `SchemaRenderer`; the three are in series, and the authored value
still reaches the renderer intact by design.
The five copies become one `readProps` that asks `isConfigBag`, exported from
`@object-ui/react`'s package entry rather than retold here — objectui#6761's
pin scans `packages/react/src`, so a copy one package over would be a spelling
it cannot see.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
…iction
The pre-fix key ORDER for a one-sided degenerate bag is ["0" … "8","content"],
not ["content","0" … "8"] — integer-like keys sort ahead of the authored one
whatever the spread order. Measured by ablating the shared reader's body back
to `?? {}` with all five modules restored from 107babe; that run moved 7 of
the 16 assertions and left 9, and every DOM assertion is among the 9.
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 was referenced Aug 29, 2026
os-sales
marked this pull request as ready for review
August 29, 2026 14:59
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 #6783
Closes the third and last channel of the degenerate-config-bag hazard: the bag an
element:*renderer reads,{ ...schema.props, ...schema.properties }.??only replacesnull/undefined, so a non-object bag went into the object spread and came back out as its own character indices.Verified on
7a82afa14; base107babef6(the dispatch namedbf283414f, whichorigin/mainhad already moved past — every count below is re-derived on my own base).The site count, with the control that makes it a reading
grep -rn "function readProps" packagesreturns 8 declarations repo-wide. The five this card names are the ones that survive the control:renderers/basic/{elements,data-list,text-input,record-picker,metadata-viewer}.tsx{ ...(schema?.props ?? {}), ...(schema?.properties ?? {}) }plugin-detail/src/renderers/record-alert.tsx{ ...schema, ...(schema?.properties ?? {}) }react/src/__tests__/{aliasPrecedenceCrossChannel,propertiesExpressions}.test.tsxA raw
grep -rn readPropsreturns 15 files; three of those areCommentThreadPropsmatching as a substring. So "five" is the count after separating in-fence production copies from out-of-fence and test copies — not a count of everything namedreadProps.The shared predicate WAS reachable — the boundary is not the blocker
The card asked me to say so with the measurement if
packages/componentscould not importisConfigBag. It can, and the interesting part is what was actually in the way:packages/components/package.jsondeclares"@object-ui/react": "workspace:*", and all five modules alreadyimport { … } from '@object-ui/react'today.packages/reactdoes not depend on@object-ui/components, so there is no cycle.isConfigBaghad no export inpackages/react/src/index.ts— one line short of reachable, not a package-graph problem.So this PR adds that one line, and it is the one file outside the stated surface (
packages/components/src/renderers/basic/). Naming it explicitly rather than letting it pass as incidental:visibleWhen? Today a node-gate fault is entirely silent in a production bundle #6038) because@object-ui/components— "which depends on this package" — asks the same question, and a second copy would mean a second rate limit. Identical shape here.@object-ui/coreinstead, which both packages depend on:configBag.pin.test.tsscanspackages/react/src. Moving the definition out of that tree would take it out of the scan's reach, so the ratchet that makes "one definition" durable would be traded away for tidier layering. The definition stays where its pin can see it, and the entry publishes it.Ablation, both directions, from one run
Ablated from the committed fix: the shared reader's body returned to
?? {}and all five modules restored from107babef6— the exact pre-fix tree. Mutation confirmed on disk before measuring (isConfigBag(calls in the shared body: 0; localfunction readPropsback in all five: 1 each; shared-reader imports: 0), restore proved afterwards by blob hash againstHEADon all six files. 7 of 16 assertions moved; 9 did not.Moved (the guard is load-bearing on the bag):
The last of those is measured end to end through the real
SchemaRenderer, so #6752's and #6760's guards are in force during it.Did not move — and this is the honest headline. Reverse-ablated the way #6760's filing did: remove the guard, see whether anything downstream changes. Nothing rendered changes. All five "renders the same with a degenerate bag as with none" assertions are green with the guard and without it. All five renderers read named keys off this bag, and the one onward spread —
metadata-viewerspreading the bag intoStateMachineView— hands it to components that destructure namedViewerPropsfields, so the nine indexed keys were computed and then dropped. The dispatch's expectation that they "reach the element as real React props" does not hold on this base; they reach a React component and die there. Also unmoved: #5123 precedence, thenull/undefined/number cases (where??and the predicate already agreed), and the leg proving the authored'not-a-bag'still arrives at the renderer — true before and after, which is exactly why this channel is the only place the question can be answered.What the guard buys is therefore what #6752 measured its guard buys, one channel further down: the authored value's shape is not reinterpreted. Per #6708's census, zero authored nodes carry a degenerate config bag — latent shape, not a live failure.
Not weakened, proved by blob hash
Byte-identical to
107babef6:Both pins also run green in the union below — neither is merely untouched. This PR adds a ratchet rather than relaxing one: the new file scans every production module under
renderers/basic/for a local?? {}config-bag read (comments stripped first, so the shared reader's docblock quoting the removed lines is not itself reported) and requires all five to import the one reader.Verification, all on
7a82afa14tsc -p tsconfig.test.json --listFilesnames both new files, so the type-check really covers them rather than excluding tests.Two gates report NOT MEASURED locally, both for a stated prerequisite, neither a verdict on this change —
check:readme-exports("the population COLLAPSED … packagesRead: found 8, floor is 25 … runpnpm buildfirst") andcheck:sdui-registration-pins("No console build to weigh atapps/console/dist/assets… This is exit 2, not a pass"). Both need a full repo/console build, which CI does anyway; recorded as unmeasured rather than as passes.Lint narrowing, declared. Repo-wide lint is
turbo run lint(per-packageeslint .). I ran the two packages this diff touches in full, not a file subset — so within each package nothing was excluded — and did not run the other 44. The scoping is per package, not per file, and the config is not type-aware across package boundaries, so this diff cannot move a verdict in a package it does not touch. CI runs all 46.Out of scope, filed not fixed
A repo-wide sweep for
?? {}/|| {}on a.props/.propertiesmember found one more site of this class:packages/plugin-detail/src/renderers/record-alert.tsx,{ ...schema, ...(schema?.properties ?? {}) }— the same defect, a different expression, outside this card's surface. Filed rather than fixed. The other census hits are a different question and are not findings:record-activity.tsxandmetadata-admin/ResourceEditPage.tsxdo keyed reads (bag[key]) with no spread, so nothing is enumerated, and the remainingmetadata-adminhits read a JSON Schemapropertiesmap, not the SDUI config bag.Generated by Claude Code