Skip to content

fix: resolve live() in an attribute hole so a falsy ?bool omits it - #1444

Merged
vivek7405 merged 3 commits into
mainfrom
fix/live-in-attribute-hole
Aug 20, 2026
Merged

fix: resolve live() in an attribute hole so a falsy ?bool omits it#1444
vivek7405 merged 3 commits into
mainfrom
fix/live-in-attribute-hole

Conversation

@vivek7405

Copy link
Copy Markdown
Collaborator

Closes #1443

What was wrong

The SSR renderer resolved live() only in a child hole. In an attribute hole the directive's wrapper object reached the emit sites raw, and each of the three kinds failed its own way:

binding before after
?open=${live(false)} <details open=""> <details>
?open=${live(true)} <details open=""> <details open=""> (unchanged)
title=${live('hi')} title="[object Object]" title="hi"
.foo=${live(1)} on a custom element data-webjs-prop-foo="{&quot;_$webjs&quot;:&quot;live&quot;,...}" data-webjs-prop-foo="1"

The bool case is the one that shipped. The wrapper is truthy, so a falsy live() could never omit its attribute. website/components/site-nav-menu.ts binds ?open=${live(this.open)} on a <details>, so webjs.dev served the mobile nav menu open, the browser painted it open, and hydration then closed it.

Pre-existing, not from #1430: the SSR path is byte-identical across that merge and the component has not changed since #1223.

Why one unwrap instead of three fixes

The client has always unwrapped live() once, before its attr/bool/prop dispatch (render-client/parts.js applyPart). And render-client/reconciler.js effectiveFormAttr, whose docstring says it mirrors render-server.js exactly, calls resolveHoleValue() to unwrap when simulating what SSR emitted. So the repo already contained both models of the same emit, contradicting each other, and the form-action reconcile was judging on the wrong one.

Unwrapping at the matching single point on the server makes the SSR bytes agree with the client by construction, rather than through three per-kind rules that can drift apart again. Both server machines get it, since the buffered and streaming renderers are separate dispatch sites that have drifted before.

Scoped to live() deliberately: it is the only directive the client accepts in attribute position. Everything else is child-only on both sides already. render() / streamRender() keep their own isLive branch for a live() nested inside an array child, which a hole-level unwrap never sees.

Side effect worth noting

The action-leak guards now fire through live(), because a wrapper is not a function. action="${live(serverAction)}" previously slipped past them and emitted action="[object Object]", a form posting to a garbage url. It is now refused like the bare form.

Verification

  • unit (packages/core/test/rendering/live-in-attribute-hole.test.js) 10 tests, every case asserted against both server machines: falsy/truthy bool, bare-vs-wrapped parity, unquoted / quoted / mixed attribute, live(null) still emitting title="" per the documented server asymmetry, .prop on a custom element, live() nested in an array child, and the action guards.
  • browser (packages/core/test/rendering/browser/live-attribute-hydration.test.js) 3 tests on Chromium, Firefox and WebKit. This is the layer that models the reported bug: the component's real SSR bytes are built into a detached container, a MutationObserver starts, and only then is it appended, which is the parse-then-upgrade window the flash lives in. A correct hydration touches open zero times.
  • counterfactual reverting the unwrap reds 26 of 34 unit assertions and 2 of 3 browser tests on all three engines. The truthy-bool case correctly stays green, since it was right by accident before.
  • Bun parity (test/bun/live-attribute-hole.mjs) required, render-server is runtime-sensitive. Green on Node 26.7.0 and Bun 1.3.14. Full matrix 333 pass, 2 fail (both pre-existing linked-worktree artifacts, confirmed identical without this change).
  • full suites browser 78 files green on all 3 engines; npm test 4496/4503, the 6 failures reproduced with the change stashed, so all pre-existing.
  • webjs check clean in gallery/, examples/blog/ and website/.
  • end to end the website booted from this branch now serves the <details> with no open attribute, so the menu paints closed and there is no flash.

Docs

website/app/docs/directives/page.ts and .agents/skills/webjs/references/components.md now state that live() is transparent at SSR in every hole position, alongside the SSR semantics already documented for guard and watch. No AGENTS.md change: the html expression prefixes section lists three server/client exceptions and this change moves live() from silently violating that rule to obeying it.

The SSR renderer resolved live() only in a child hole, so the directive's
wrapper object reached the attribute emit sites raw. Each kind failed its
own way: the wrapper is truthy, so `?open=${live(false)}` emitted `open=""`
whatever it wrapped; `attr=${live(v)}` stringified to `[object Object]`;
and `.prop=${live(v)}` serialized the wrapper into the hydration payload.

The client has always unwrapped live() once, before its attr/bool/prop
dispatch, and the form-action reconcile even simulates SSR by unwrapping
while claiming to mirror the server exactly. So the two renderers held
contradictory models of the same emit, and the falsy-bool case shipped:
webjs.dev's mobile nav menu was served open and hydration then closed it.

Unwrap at the same single point on the server, in both the buffered and
streaming machines, which makes the SSR bytes match the client by
construction rather than through three per-kind rules that can drift.
Scoped to live(), the only directive the client accepts in attribute
position. render() and streamRender() keep their own isLive branch for a
live() nested inside an array child, which a hole-level unwrap never sees.

The action-leak guards now fire through live() as well, since a wrapper is
not a function: `action="${live(serverAction)}"` previously slipped past
them and emitted a form posting to a garbage url.

Closes #1443
@vivek7405
vivek7405 marked this pull request as ready for review August 20, 2026 19:18
Review findings on the #1443 fix, all three folded in:

The client's attr-mixed commit read every group piece raw from allValues,
bypassing applyPart's top-of-function unwrap, which only sees the anchor
hole's own value. Every hole inside a QUOTED attribute is attr-mixed to
the client compiler (single-hole included), so the SSR bytes the server
half of the fix now gets right for `title="${live(v)}"` were rewritten
to the wrapper's stringification by the first client commit: served
correct, corrupted on upgrade. Unwrapping per piece also puts the real
inner value in front of the per-piece function guard, and matches the
reconciler's effectiveFormAttr, which already unwraps each group piece.

Also pinned the one shape the server unwrap changes most: an unquoted
`action=${live(fn)}` on a form now enters the #1155 bound-form dispatch
instead of stringifying the wrapper into the attribute, asserted as
strict parity with the bare `action=${fn}` on both machines.

And added the dedicated ci.yml Bun step for test/bun/live-attribute-hole.mjs
that its wrapper's docstring described, matching the convention every
other cross-runtime proof follows.
@vivek7405

Copy link
Copy Markdown
Collaborator Author

Review round (single pass, per owner instruction) + fixes

One reviewer pass over the whole diff. Three findings, all folded into a6f65ee2:

1. CONFIRMED (must-fix): the client's attr-mixed commit never unwrapped live(), so the SSR fix alone created a NEW server/client divergence for quoted and multi-hole attribute holes. The client compiler classifies EVERY quoted attribute hole as attr-mixed (single-hole included), and that commit path reads each group piece raw from allValues, bypassing applyPart's top-of-function unwrap. So title="${live(v)}" was served correctly by this PR and then rewritten to [object Object] by the first client commit. Fixed by unwrapping per piece inside the attr-mixed loop, which also puts the real inner value in front of the per-piece function guard and matches effectiveFormAttr, which already unwrapped each group piece. Pinned at two layers: live-in-client-attr-mixed.test.js (linkedom, 4 tests, counterfactual 4/4 red with the unwrap disabled) and a real-engine client-commit case in the browser hydration test (green on Chromium/Firefox/WebKit).

2. Untested behaviour change: unquoted action=${live(fn)} on a form. The unwrap routes it into the #1155 bound-form dispatch instead of stringifying the wrapper into action=. Verified the outcome is exact parity with the bare action=${fn} (identical refusal for a non-action function; a real 'use server' export binds, which the client's resolveHoleValue already models). Pinned as a strict bare-vs-wrapped parity assertion on both server machines.

3. Doc/CI mismatch: the Bun wrapper's docstring claimed a dedicated bun test/bun/live-attribute-hole.mjs CI step that did not exist. Added the step to ci.yml, matching the convention every other cross-runtime proof follows.

Out-of-scope finding, reported for the owner's decision (not filed): nav-scroll-anchor-restore.test.js:824 ("the window closes once the restore is over, leaving no residue") flakes Firefox-only under FULL-suite load. Reproduced on pristine main (ad81d4b3) at 3-of-4 full Firefox runs, 8/8 green in isolation, identical assertion. Pre-existing from #1430, unrelated to this diff (this branch measures 2-of-6, indistinguishable-or-better).

Verification after the fixes: unit 4483/4483 (0 fail), browser 78 files green on all three engines, the two new/extended test files green under bun test (15/15), the Bun proof green on Node 26.7.0 and Bun 1.3.14, webjs check clean, ci.yml parses.

The delta review found the attribute half was complete but the CHILD half
was not, and the gap predates this PR. applyPart unwraps live() for the
hole's OWN value, while the server RECURSES through isLive inside render()
and streamRender(), so the server resolves a live() nested one level down
and the client did not. Served correct, corrupted to "[object Object]" on
upgrade: the same divergence class the attribute fix closes.

Four consumers of a child value each needed it, and each is independently
load-bearing (verified by disabling one site at a time):

  applyChildInnerRaw  a directive WRAPPING a live(), the keyed / cache /
                      guard recursion, which re-enters with an inner value
  buildArrayItem      an array item on the fresh-build path
  reconcileArray      an array item updated IN PLACE, where the array keeps
                      its shape so nothing is rebuilt
  renderToNodes       a live() yielded into asyncAppend / asyncReplace

Deliberately NOT fixed: a live() inside a NESTED array. The client
stringifies a nested array rather than recursing, so a nested
TemplateResult renders "[object Object]" too. That is a pre-existing gap
independent of live(), and special-casing this one directive would leave
an inconsistent surface.

Also tightens two tests the review found weaker than they read. The
mixedAnchor case used live() in both slots, so the anchor hole differed on
every render and the attribute rebuilt at i=0 without the noop path ever
being under test; the anchor piece is now a plain constant, so only the
re-apply can explain the update. And the array-child test asserted the
server alone under a name that read as both, so it now says SERVER and
points at the client file. The browser case observes the real
SSR-then-upgrade transition through a component rather than a bare client
render, with the direct-render case kept alongside it.
@vivek7405

Copy link
Copy Markdown
Collaborator Author

Delta round (scoped to a6f65ee2) + fixes

Verdict on a6f65ee2: correct and complete for its stated scope. The reviewer verified the per-piece unwrap is the only raw group-piece read in the client, that guard ordering matches the server, that the ci.yml step is right (it ran the Bun proof for real), and that the new tests are non-vacuous. No regressions: full core node and browser suites green.

It surfaced one genuine defect of the SAME class, pre-existing (present at ad81d4b3, not introduced by this PR). Fixed in dafb411a, because it lives in parts.js, a file this PR already touches.

The client resolved live() only for a hole's OWN value, while the server RECURSES. render() and streamRender() each recurse through isLive, so the server resolves a live() nested one level down and the client did not. Served correct, corrupted to [object Object] on upgrade.

I reproduced it before fixing (SSR=<i>ab</i> vs CLIENT=[object Object]b), and found the suggested single-line fix was not sufficient. There are four independent consumers of a child value, each verified load-bearing by disabling one site at a time:

site shape it covers tests red when disabled
applyChildInnerRaw a directive wrapping a live(): keyed, cache, guard 1
buildArrayItem array item, fresh build 3
reconcileArray array item updated in place 1
renderToNodes live() yielded into asyncAppend 1 (probe)

Deliberately not fixed: a live() inside a NESTED array. I checked whether that was live()-specific and it is not: the client stringifies a nested array rather than recursing, so ${[[htmlx]]} renders [object Object] too. That is a broader pre-existing gap, and special-casing one directive would leave an inconsistent surface. Reported, not filed, so it is your call.

Both style notes taken, because both concerned a test's ability to observe what it claims

  • The mixedAnchor test used live() in both slots, so the anchor hole differed on every render and the attribute rebuilt at i=0, meaning the noop re-apply path was never actually under test. The anchor piece is now a plain constant, so only the re-apply can explain the update.
  • The array-child test asserted the server alone under a name that read as both. It now says SERVER and points at the client file.
  • The browser case now observes the real SSR-then-upgrade transition through a component (the stronger hydrate() shape), with the direct-render case kept alongside it.

Verification

Unit 4488/4488 (0 fail), browser 78 files green on Chromium, Firefox and WebKit, Bun matrix 337 pass with 0 genuine failures, the three new and extended files green under bun test (20/20), webjs check clean in gallery, website and blog. Counterfactuals: each of the four client sites reds its own tests, and the browser cases red 2-of-5 on all three engines with the attr-mixed unwrap reverted.

Confirmed no scaffold surface is affected: the one gallery demo using live() binds .value=${live(...)} on a native <input>, a .prop hole that drops at SSR by design and was already handled by the client's top-of-function unwrap.

@vivek7405
vivek7405 merged commit e0abcf9 into main Aug 20, 2026
10 checks passed
@vivek7405
vivek7405 deleted the fix/live-in-attribute-hole branch August 20, 2026 20:56
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.

dogfood: SSR does not resolve live() in an attribute hole, so ?open=${live(false)} emits open=""

1 participant