Uh oh!
There was an error while loading. Please reload this page.
fix(sdui-parser): keep the space between a text run and an adjacent inline element - #5722
Merged
Merged
Conversation
…ibling element `parseChildren` collapsed each text run's whitespace to a single space — correct, that is HTML's own whitespace model — and then `.trim()`ed it, which is not: HTML collapses a whitespace run to ONE space, it does not delete it. The deleted space is the one that separates a run from an adjacent inline sibling, so every authored sentence with emphasis or a link in the `kind:'html'` tier rendered with its words run together: `A <strong>x</strong> page` came out as `Axpage`. Keep one leading space when a sibling precedes the run and one trailing space when a sibling element follows it; the parent's own start/end still drop theirs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EuPCi56cnGyykygi3z9w4m
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 22, 2026 22:45
Uh oh!
There was an error while loading. Please reload this page.
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#5661
The defect
packages/sdui-parser/src/parse.ts, inparseChildren's children loop (line numbersre-derived from the tree at
origin/main190fbd01d, not taken from the card — thecard's
:177citation is off by one for the first of the two lines):The collapse (
\s+→ one space) is correct and is HTML's own whitespace model. The.trim()is not: HTML collapses a whitespace run to one space, it does not delete it.What it deleted is exactly the space separating a text run from an adjacent inline
sibling, so every authored sentence carrying emphasis or a link in the tier the guide
recommends by default rendered with its words run together — silently, since the page
rendered, the structure was right, and no diagnostic fired.
The rule implemented — triage's option (b)
Collapse the run, then keep one leading space when a sibling precedes the run and
one trailing space when a sibling element follows it. The parent's own start
and end still drop their edge space, so
<p> hi </p>is unchanged.Deliberately mechanical: it invents no block/inline boundary taxonomy for a schema
tree that has none — that was the rejected option (a), a design project rather than
this fix.
The bounded cost, pinned rather than asserted
Under this rule a whitespace-only run between two siblings survives as a single
space, so a pretty-printed
<ul>gains one' 'string child per gap between its<li>s. The<ul>control in the new suite states the bound as the things that couldhave gone wrong instead:
' '— never the source's newline andindentation;
own edges still trim);
<li>'s own text is byte-identical to what was authored.A
' 'child renders as a text node between list items — the same node a browserputs in the DOM for the same HTML source, and one
<ul>layout ignores.Evidence
New suite
packages/sdui-parser/src/__tests__/inline-whitespace.test.tsx, 10 tests.It pins the three cases the card measured, the collapse itself, the parent-edge
behaviour that must not change, and the
<ul>control. Two of its cases renderthrough the real
SchemaRendererand asserttextContent, because that is the termthe bug was reported in — a parse-tree assertion alone cannot say the words stopped
running together on screen.
Reverse verification (fix committed first, then
parse.tsalone reverted toorigin/main, suite re-run, then restored): 9 of the 10 tests go red, and the onethat stays green is
still drops the edge space at the parent's own start and end—i.e. exactly the half of
.trim()that was already correct. The render case reproducesthe reported string verbatim:
Snapshot sweep (triage asked for it):
packages/sdui-parserholds no snapshots atall, and the repo has exactly two
.snapfiles, both inpackages/components(
snapshot.test.tsx.snap,snapshot-critical.test.tsx.snap). Both tests wereexecuted in the runs below and both passed with the files unmodified —
git statusstayed clean, so no snapshot changed and none was updated. Nothing in the repo
depended on the old trimming.
Test union, all green, run under this container's shared verify lock:
packages/sdui-parser/ packages/layout/ packages/types/ apps/console/packages/components/src/__tests__/packages/components/src/renderers/ src/ui/ src/notifications/That set is the parser's reachable blast radius: every package that imports
@object-ui/sdui-parser, plus every test that renders akind:'html'page (whosesourcepackages/components/src/renderers/layout/page.tsxcompiles at render time).packages/plugin-gridandexamples/schema-catalogimport onlymanifestFromConfigs/
validateTreeand never parse; no test underpackages/app-shellparses anhtml-tier source (measured by grep for
sdui-parser,kind:'html'andcompile(across its test files).
Twin check (required by triage)
The twin is real.
objectstack-ai/objectstackpackages/sdui-parser/src/parse.tscarries the identical statement at
:167/:168on itsorigin/main(acb4dbc).Not fixed here, per triage — filed as
objectstack-ai/objectstack#11148.
The two files have drifted and are not a byte copy: diffed in full, their only
difference is objectui's html-tier provenance stamp (objectui#4000) — one import plus
a 9-line block at the node-construction site, which is precisely the offset between
:176/:177here and:167/:168there. The defective statement itself isbyte-identical. The twin card says so rather than implying a cherry-pick will apply.
Out of scope, established rather than assumed
The second
.trim()in this file (:275,const trimmed = raw.trim()) is not thesame defect. It sits in
interpretBrace(raw), normalising the source text of a bracedattribute before
JSON.parse/ before it is kept as an{ $expr }marker. That isexpression syntax, not the rendered content model, and no space it removes was ever
separating anything. Untouched.
Gates
Run at the head of this branch unless noted; all green.
turbo run type-check lint --filter=@object-ui/sdui-parserTasks: 9 successful, 9 total— type-check clean; lint0 errors, 7 pre-existing warnings, none in a changed filepnpm check:control-bytesOK (scanned 4770 tracked text file(s); skipped 85 binary)pnpm check:phantom-depsEvery in-scope import is declared by the package that publishes it.pnpm check:self-importNo package names itself inside its own src/.pnpm check:esm-specifiersno un-ledgered package emits an extensionless relative specifierpnpm changeset:checkprivatePackages declared·No changeset declares a major bump.pnpm lint:coverage46/46 packages linted, 0 with outstanding errorspnpm type-check:coverage45/46 via type-check·41/41 packages compile their testspnpm lintwas narrowed to this package deliberately:lintisturbo run lint, i.e.each package's own
eslint .plus//#lint:root(which ran here as a dependency, 0errors), only one package has changed files, and
eslint.config.jsconfigures notype-aware linting — so no untouched package's verdict can move.
pnpm check:eager-closurewas not run: it reads
apps/console/dist/eager-closure.jsonand reports a broken gaugewithout a console
vite build; this diff adds no import anywhere in that closure.A patch changeset for
@object-ui/sdui-parseris included (the rendered output ofevery existing
kind:'html'page changes).Generated by Claude Code