Uh oh!
There was an error while loading. Please reload this page.
fix(components): route flex/stack/container/text through toDomProps (#5574) - #5839
Merged
Merged
Conversation
…#5574) The four renderers objectui#5574 named as candidates all leaked, and the leak was their OWN declared props: `align`, `gap`, `justify`, `direction`, `padding`, `maxWidth`, `content`, `value` were consumed off `schema` to build the class list and then spread onto the host element a second time, as attributes HTML does not define. Converged on `toDomProps`, the whitelist `grid.tsx` was converged on by objectui#4787 / PR #5573. Measured across `examples/schema-catalog` through the real `SchemaRenderer`: 1194 illegitimate attributes over 1141 nodes before, 0 after, with `grid`'s 26 nodes reading 0 in both runs as the control that says the probe is not blind. The four ledger rows in the DOM-leak sweep are DELETED rather than edited — the gate's two-way expiry makes the fix fail until they go — and the case that pinned them as leaking is inverted to pin that no row may re-absorb this family. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EuPCi56cnGyykygi3z9w4m
…shape (#5574) `<div align="start" gap={4}>` does not type-check, which is the point: the attributes this file measures reached real elements through an untyped spread, never through JSX attributes. The self-check now spreads a `Record<string, unknown>` — the defect's own shape — instead of hand-writing attributes the compiler would have caught anyway. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EuPCi56cnGyykygi3z9w4m
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-zhuang
marked this pull request as ready for review
August 23, 2026 14:46
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 23, 2026
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
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#5574
Routes
flex,stack,containerandtextthroughtoDomProps— the whitelist PR #5573 landed ongrid.tsx— and deletes their four rows from the DOM-leak sweep's ledger, which is the only way the gate goes green again.This is the burn-down half of #5574. The measurement half landed in PR #5629 (the sweep widening plus the 119-row arrival ledger); the post-merge reading in comment 5379776530 is what said the card was measured but not delivered.
The reading, re-taken rather than trusted
Every
examples/schema-catalognode of these types rendered through the realSchemaRenderer, attributes read off the node's own host element:text[content]container[padding]flex[align]container[maxwidth]flex[gap]flex[direction]stack[gap]stack[align]flex[justify]text[value]gridread 0 across its 26 nodes in both runs — the control the dispatch asked to be kept. A fixed renderer reads clean while its unfixed siblings read dirty, so the instrument is demonstrably not blind.Two deltas from the numbers in comment 5379776530, both explained rather than waved at:
flex[gap]is 193, not 184. PR Author the nine catalogspace-x-*flex nodes asgap, and ratchet the tenth out #5826 re-authored ninespace-x-*flex nodes asgapafter that reading was taken. Renderer-side nothing moved; this is the "grows with the examples" property that comment already called out. Every other layout figure reproduces exactly.textleaks too, and it is the largest single reading in the family — 523 attributes. The card body namedtextas a candidate; the post-merge measurement did not cover it. The same probe shows it leaking, so it is fixed here with the count stated. All but one of those attributes arecontent— the key the renderer renders as its children, duplicated into the markup as an attribute.textalso carries the phantom-clean caveat in miniature: of its 699 nodes, 176 rendered no element at all (the bare-fragment return, taken when a node has neither designer id nor className). Those 176 were never evidence of safety. Recorded in the source and asserted, not hidden.Reverse-verification, on a real commit
Fix committed first (
3ed962a73), then the four renderer sources — and only those — reverted toorigin/main, mutation confirmed on disk (toDomPropscount 4→0 per file, the bare-spread marker'data-obj-type': dataObjTypeback,git diff HEAD --statnaming exactly 4 files), both instruments re-run:Restored (the script carries a
trap … EXIT INT TERM, so a foreground-cap kill mid-mutation could not leave the tree mutated for later measurements),git diff HEAD --statempty, re-run: 202 passed.Legs that do NOT discriminate, named rather than counted as evidence:
gridleg. It reads 0 in every configuration of this change, so on its own it can never fail and proves nothing. It earns its place only jointly — as the reading that stays clean in the same run where its siblings flip.NODE_CENSUSassertion. Identical across the mutation — the same nodes render either way. It guards the blind-walk failure.What landed, and what deliberately did not
Renderers — the
grid.tsxtreatment, not a second approach: destructurestyle(forwarded by name, the #4435 route, as the designer's sizing channel), spreadtoDomProps(hostProps), letdata-obj-id/data-obj-typearrive through the opendata-*family instead of by hand. Every leaked key was already being consumed offschemato build the class list, so nothing an author writes renders differently — the markup loses attributes that never had meaning and keeps the styling that did.The sweep's ledger — four rows deleted, not edited. The gate's exact-set-equality-in-both-directions is what forced it: the fix cannot go green while the rows stand. The docblock keeps the arrival reading (119) in prose and records the burn-down; the assertions carry current truth (115).
The case that pinned the four as leaking is inverted, not deleted. It now asserts all five converged renderers are ABSENT from the ledger and still SWEPT. The sweep already fails if one regresses; what this adds is that the regression cannot be made green by putting the row back — the one repair the ledger's shape invites, and the one that turns a measurement into an allowlist entry.
The sweep's target list is NOT widened here — PR #5629 already did that, so the gap the card names is closed. Stated because the dispatch asked either way.
No renderer outside the measured set was touched. 115 rows remain, in the seven other measured shapes; they are the PM seat's burn-down parent to sequence.
Verification
Union re-run after the final commit, at
64ca95fe5, working tree clean, every exit code captured before any pipe:pnpm exec vitest run(repo root) — sweep +examples/schema-catalog+packages/componentsTest Files 192 passed (192)/Tests 3469 passed (3469)type-check— components, app-shell, example-schema-catalogDone×3, exit 0lint— same three packagesDone×3, exit 0 (0 errors; 906 / 2598 pre-existing warnings, none on a changed line)node scripts/check-changeset-presence.mjs✅ 5 source file(s) of 2 released package(s) changed, and this change declares 1 changeset(s)node scripts/check-changeset-no-major.mjs✅ No changeset declares a major bumpnode scripts/check-control-bytes.mjs✅ OK (scanned 4857 tracked text file(s); skipped 85 binary)Vitest always from the repo root, never
pnpm --filter(#3378's guard). Every heavy command through the container's shared verify lock. The dependency closure was built first —pnpm --filter "PKG^..." build, with PKG standing in for the package name because GitHub's body sanitizer eats angle-bracket-delimited runs, which it did to the first revision of this line. Without that build,type-checkreportsTS2307: Cannot find module '@object-ui/core'on 25 untouched files and reads like a broken diff.Lint narrowing, declared: repo-scope
pnpm lintis CI's run and was not taken. What replaces it is package-scopeeslint .on all three affected packages — a config-derived population, a provable superset of this diff at package granularity — plus a 6-file targeted run reportingfiles linted: 6, errors: 0from--format json.eslint.config.jsconfigures noproject/projectService, so linting is not type-aware and this diff cannot move the verdict on any file it does not touch.Generated by Claude Code