Skip to content

Compiled components dropped caller attributes, and reactive children froze - #2

Merged
pathscale merged 5 commits into
masterfrom
fix/layout-passthrough-and-children
Aug 14, 2026
Merged

Compiled components dropped caller attributes, and reactive children froze#2
pathscale merged 5 commits into
masterfrom
fix/layout-passthrough-and-children

Conversation

@pathscale

@pathscale pathscale commented Aug 14, 2026

Copy link
Copy Markdown
Owner

Two defects, one in each half of the project. Both are silent: no warning, no type error, no failed build. The component renders, looks right, and is wrong.

They were found by porting an application with 561 frontend tests onto a Layout-based library. It passes all 561 on the old library. On the new one it dropped to 483 passing, 78 failing, and not one of the failures was in the application or the library.

State Failing
before 78
passthrough fixed 6
children fixed, for one component 2

Compiled components dropped every plain HTML attribute

The root slot carries the props a caller passed that the recipe does not declare. That is what makes aria-label, title, data-testid and onClick reach the DOM at all, because a layout only ever sees slot and has no other route. The behaviour was gated:

name === rootSlot && !embedded

embedded is what the compiler sets on every component it produces, so the gate cancelled the behaviour for an entire real library. A stock button:

render(() => <Button aria-label="Send" title="tip" data-testid="probe">go</Button>);
<button class="button …" data-slot="button" type="button">go</button>

Nothing the caller passed survived. 72 of the 78 failures were this one condition, and each one read as an application bug.

The exemption looks deliberate rather than accidental: an embedded component does receive every prop on p, so a layout could in principle place them itself. Nothing does, and the compiler emits no such code, so the capability was theoretical and the loss was real. Recipe attributes still spread last, embedded or not, so a caller still cannot overwrite class or data-slot.

The existing test asserted the old behaviour, which means it asserted the bug. It now asserts the fix, and a second test covers the reported symptom directly.

children was destructured, so reactive children froze

The generated signature was ({ slot, children }, p). The runtime exposes children as a getter over Solid's resolved-children memo, so destructuring calls it once, when the layout runs, and freezes whatever it returns.

Solid's JSX compiler decides reactivity by the shape of the expression: a member expression becomes a getter it re-reads, a plain identifier is read once. Emitting _stable.children is what keeps the insert live.

Twelve lines reproduce it:

const [label, setLabel] = createSignal("Status");
render(() => <Button onClick={() => setLabel("Time")}>{label()}</Button>);
button.click();
// textContent: "Status"

This is why a browser built on this rendered blank pages. A <For> over a tab list that arrives asynchronously resolved to the empty list it saw on the first pass and never ran again, so no <web-view> element was ever created and every site loaded correctly into a document that was never attached. The workaround there was to keep a plain element between the Layout and the For, described in comments as a rule of Layouts. It is not a rule, and those comments should come out.

The diagnosis and the two compiler tests here are folded in from concurrent work in the shared checkout rather than written twice.

Verification

solid-layouts runtime 145 pass, 0 fail
layouts-transform crate 58 pass, 0 fail
fixture corpus 2 pass
library.test.js 7 pass
application.test.js 14 pass

What these have in common

Neither is caught by the existing suites, because they exercise recipes and class resolution rather than a mounted component with a caller's attributes and a changing child. A single fixture that renders one compiled Layout with an aria-label and a signal child would have caught both, and is worth adding regardless of these fixes.

Versions bumped for release: solid-layouts 0.1.3, solid-layouts-oxc 0.1.7, rsbuild-plugin-solid-layouts 0.1.4. Publishing is by tag through Trusted Publishing, so it happens from CI after this merges rather than from anyone's laptop, which is also the only way the native package gets a binary per platform.

Also fixes CI itself. master has been red since fc96dee because the application-host tests read solid-layouts's built entry off disk and nothing in that job built it. The failure reads as a broken resolver rather than an unbuilt dependency, which is presumably why it sat.

meh added 3 commits August 14, 2026 11:12
The root slot carries the plain-HTML props a caller passed, which is what makes
`aria-label`, `title`, `data-testid` and `onClick` reach the DOM at all: a layout
only ever sees `slot`, so there is no other route. That behaviour was gated on
`!embedded`, and `embedded` is what the compiler sets on every component it
produces, so a library built with this compiler dropped all of them from every
component.

Nothing reported it. The component rendered, looked right, and was missing
whatever the caller passed. It surfaced as 72 failures out of 78 when one
application moved onto a Layout-based library, and every one of them read as an
application bug.

The exemption looks deliberate: an embedded component does get every prop on `p`,
so a layout could in principle place them itself. None does, and the compiler
emits no such code, so the capability was theoretical and the loss was real.

Recipe attributes still spread last, embedded or not, so a caller still cannot
overwrite `class` or the `data-slot` that identifies the component. The test that
asserted the old behaviour asserted the bug; it now asserts the fix.
The generated layout signature destructured its first parameter as
`({ slot, children }, p)`. The runtime exposes `children` as a getter over
Solid's resolved-children memo, so destructuring calls that getter once, at the
moment the layout runs, and freezes the result.

Solid's JSX compiler is what makes the difference: it wraps a member expression
in a getter it re-reads, and leaves a plain identifier alone. Emitting
`_stable.children` keeps the insert reactive.

What it cost: a `<For>` over a list that arrives asynchronously resolved to the
empty list it saw on the first pass and never ran again. In a browser built on
this, no `<web-view>` was ever created, so every site loaded correctly into a
document that was never attached and the window stayed blank. In an application,
a button's label stuck on its first value and two test crashes turned out to be
downstream of an element that never updated.

`slot` moves onto the parameter with it, so the signature has one shape, and
because a layout may use `children` without ever mentioning `slot` and that case
used to compile to an unbound reference.

Folded in from concurrent work in the shared checkout rather than written twice:
the diagnosis and the two tests are theirs.
Two silent defects, one in each half. Both are the kind that a consumer cannot
diagnose from their own code, so this wants to reach the fleet before any more
of it is ported.
meh added 2 commits August 14, 2026 12:34
The application compiler resolves a consumer's declared runtime by reading
`solid-layouts`'s built entry off disk, and nothing in this job built it, so the
step failed on a missing `dist/index.js`.

master has been red on this since `fc96dee`, which is long enough that the
signal had stopped meaning anything. The failure reads as a broken resolver
rather than an unbuilt dependency, which is presumably why it sat.
The compiler emits `_stable.children` and `_stable.slot` now, so the four
generated components in the fixture change shape. The guard that compares them
against a fresh run is what caught it, which is the guard doing its job.
@pathscale
pathscale merged commit a107bf1 into master Aug 14, 2026
4 checks passed
@pathscale
pathscale deleted the fix/layout-passthrough-and-children branch August 14, 2026 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant