Skip to content

fix(web): duplicate data-lha on escalated live attrs; data calls during hydration - #4055

Merged
ryansolid merged 6 commits into
nextfrom
claude/project-thread-pe3qnr
Oct 14, 2026
Merged

ryansolid merged 6 commits into
nextfrom
claude/project-thread-pe3qnr

Conversation

@ryansolid

@ryansolid ryansolid commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

Summary

Closes #4032. Closes #4033.

#4032 (duplicate data-lha). Before: an in-tag hole whose value is pending inside a live region made the tag escalate, but the attr capture had already spliced its data-lha address into the tag. The tag then left with two addresses, and an attrs op could carry a stale data-lha. After: when the hole escalates, ssr() removes the address the capture injected, at each of the three escalation sites in packages/web/src/server.ts, so each tag carries one address. The removal is written inline rather than as a closure inside ssr(), which cost about 13% on the link-claim renderToString bench.

#4033 (data server functions never hit the network during hydration with components: true). Before: a data call made while the document was hydrating waited on awaitBoundary for a frame site that could never arrive, so its fetch never started. There were two causes. First, boundaryMayArrive read only _$HY.done, which flips a setTimeout after sharedConfig.done, so a call made in onHydrationEnd still looked like it might find a site. Second, under the hydration trace Promise is a stub that never runs its executor, so the waiter was registered with no resolver and every later call with the same id hung behind it. After: boundaryMayArrive also checks sharedConfig.done; waiters are released at hydration end (releaseArrivals, shared with the reveal subscriber); and a stub promise is never cached as a waiter.

The frames client grows 27 B br (cap 11.495 → 11.522 KB, ledger entry in scripts/size/scenarios.js). This PR also updates #4045's ledger wording now that it was accepted.

Size-Exception: renderToString floor 20.55 -> 20.58 KB (+39 B br / +131 B min on top of #4043, measured 20,570 B / 72,373 min against next @ f048691) for the #4032 address removal in ssr(). Accepted by the maintainer (2026-10-14) as one combined cap of about 20.58 KB together with #4043; this PR merges second and sets it.

adopted-fallback-residue.spec.tsx is now split in two files. The frames boundary index is module state, and that file's second page had been passing only because it read the first page's stale index. The nested case is now in adopted-fallback-residue-nested.spec.tsx (one page per file); it passes with and without this change.

How did you test this change?

  • New test/server/live-attr-escalation-4032.spec.tsx: a pending attr inside <Loading> fails before the fix and passes after; a nested guard case passes on both.
  • New test/hydration/data-call-during-hydration-4033.spec.tsx: a data call in onHydrationEnd, and a call made after a trace-stub call, each expect one fetch. Both fail on next and pass here.
  • pnpm build. Then the web server suite: 1673 passed; the only failure is the server-functions-adapter-request "rebuild failure" test, which is also red on next. Hydrate suite: 503 passed (16 expected fail). DOM suite: 1392/1392. Prettier clean.
  • After merging next (with fix(web): a deferred hole keeps its server component's claims arming #4043): DOM 1392/1392, server 1674 passed (same Node 22-only failure), hydrate 503 passed.
  • pnpm bench:server link-claim, A/B against next @ c818604 (5 runs each): level within noise after the inline change; the closure version was measurably slower.
  • Size scenarios measured locally against next @ f048691.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AV6Ya8uSU8gut3V3odqxck

@ryansolid ryansolid self-assigned this Oct 14, 2026
@changeset-bot

changeset-bot Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 9b9b921

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages
Name Type
@solidjs/web Patch
@solidjs/babel-plugin Patch
@solidjs/diagnostics Patch
@solidjs/element Patch
@solidjs/h Patch
@solidjs/html Patch
test-integration Patch
board-example Patch
todos-server-example Patch
@solidjs/compiler Patch
@solidjs/signals Patch
solid-js Patch
@solidjs/universal Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Size (brotli, eager entry chunk)

scenario head vs base minified vs base minified vs recorded cap lazy chunks (not counted)
signals: core floor (createSignal/Memo/Effect/Root/flush) 7.47 KB 0 B 0 B −8 B 7.48 KB ✅
signals: + createStore 14.94 KB 0 B 0 B −22 B 14.94 KB ⚠️ over by 3 B, 42 B minified headroom
signals: + isPending/latest 9.75 KB 0 B 0 B −8 B 9.77 KB ✅
app: render + one signal (the simple-app floor) 9.95 KB 0 B 0 B −8 B 9.98 KB ✅
app: hydrating (no stores) with Show/For/Loading/Errored/lazy 17.89 KB 0 B 0 B −65 B 17.93 KB ✅ lazy-page.js 0.04 KB
app: hydrating + every store primitive family 29.54 KB 0 B 0 B −83 B 29.57 KB ✅ lazy-page.js 0.04 KB
app: CSR with Show/For/Loading/Errored/lazy 13.04 KB 0 B 0 B −22 B 13.07 KB ✅ lazy-page.js 0.04 KB
app: CSR, observe tier (same app on the observe artifacts) 14.64 KB 0 B 0 B −22 B 14.64 KB ✅ lazy-page.js 0.04 KB
app: CSR, observe tier + attribution engine enabled 29.23 KB 0 B 0 B +14 B 29.24 KB ✅ lazy-page.js 0.04 KB
app: compiled floor (one template, one text hole, one delegated click) 10.16 KB 0 B 0 B −8 B 10.17 KB ✅
app: compiled CSR (JSX todo app: spread/merge/omit, events, class/style, keyed For, Show, Loading + lazy, store) 25.80 KB 0 B 0 B −99 B 25.79 KB ⚠️ over by 12 B, 119 B minified headroom stats.js 0.18 KB
app: compiled hydrating (the same JSX todo app through hydrate(), compiled hydratable) 31.69 KB 0 B 0 B −41 B 31.64 KB ⚠️ over by 54 B, 61 B minified headroom stats.js 0.20 KB
frames: eager client consumer (frames client + transport, lazy codec) 11.52 KB +27 B (+0.2%) +54 B 0 B 11.52 KB ✅
page: base server components (hydrating + dynamic + frames + sf reference) 34.20 KB +5 B (+0.0%) +33 B −1369 B 34.33 KB ✅ assets.js 0.78 KB, bind.js 1.83 KB, decode.js 6.23 KB, lazy-page.js 0.04 KB, regions.js 0.80 KB, trace.js 9.01 KB, wire.js 0.93 KB
page: live server components (base + live/GET + action + isPending/latest) 37.93 KB −34 B (−0.1%) +33 B −1438 B 38.15 KB ✅ assets.js 0.78 KB, bind.js 1.83 KB, decode.js 6.23 KB, lazy-page.js 0.04 KB, regions.js 0.80 KB, trace.js 9.02 KB, wire.js 0.93 KB
page: compiled base server components (the base page as JSX: templates with class/style/attributes/events, For/Show; no spread) 35.04 KB +42 B (+0.1%) +33 B −2459 B 35.66 KB ✅ assets.js 0.78 KB, bind.js 1.83 KB, decode.js 6.23 KB, regions.js 0.80 KB, sc-comments.js 0.20 KB, trace.js 8.99 KB, wire.js 0.93 KB
page: compiled live server components (the compiled base page + live/GET + action + isPending/latest) 40.71 KB +32 B (+0.1%) +42 B −2443 B 41.21 KB ✅ eager (counted): web.js 21.56 KB; assets.js 0.78 KB, bind.js 1.84 KB, decode.js 6.23 KB, regions.js 0.79 KB, sc-comments.js 0.19 KB, trace.js 8.97 KB, wire.js 0.93 KB
page: base + router (base page + @solidjs/router: createRouter, two routes, preload, useNavigate) 41.62 KB +14 B (+0.0%) +39 B −1369 B 41.79 KB ✅ assets.js 0.78 KB, bind.js 1.85 KB, decode.js 6.23 KB, lazy-page.js 0.04 KB, regions.js 0.80 KB, server.js 1.02 KB, serverForms.js 3.58 KB, trace.js 9.01 KB, wire.js 0.94 KB
page: live + router (live page + @solidjs/router: createRouter, two routes, preload, useNavigate) 47.32 KB −10 B (−0.0%) +43 B −1359 B 47.49 KB ✅ eager (counted): client.js 27.75 KB; assets.js 0.78 KB, bind.js 1.84 KB, decode.js 6.23 KB, lazy-page.js 0.04 KB, regions.js 0.81 KB, server.js 1.02 KB, serverForms.js 3.30 KB, trace.js 8.97 KB, wire.js 0.94 KB
server: floor (getRequestEvent + isServer) 1.33 KB 0 B 0 B 0 B 1.34 KB ✅
server: renderToString (the server-render floor) 20.57 KB +39 B (+0.2%) +131 B 0 B 20.58 KB ✅

⚠️ Over the brotli cap within the minified allowance (passes)

  • signals: + createStore: over brotli cap by 3 B; minified 45,343 B vs 45,365 B recorded with the cap (−22 B) — 42 B of the 20 B minified allowance left; +0 B minified over this PR's base
  • app: compiled CSR (JSX todo app: spread/merge/omit, events, class/style, keyed For, Show, Loading + lazy, store): over brotli cap by 12 B; minified 80,624 B vs 80,723 B recorded with the cap (−99 B) — 119 B of the 20 B minified allowance left; +0 B minified over this PR's base
  • app: compiled hydrating (the same JSX todo app through hydrate(), compiled hydratable): over brotli cap by 54 B; minified 100,920 B vs 100,961 B recorded with the cap (−41 B) — 61 B of the 20 B minified allowance left; +0 B minified over this PR's base

Bundled with Rolldown (what Vite ships), brotli q11, decimal KB. A scenario fails only when it is over its brotli cap and its minified size is more than 20 B over the minified recorded with the cap; over the cap within that allowance is brotli layout noise and passes with a warning. Caps and their recorded minified in scripts/size/scenarios.js; the floor and page caps in floor-caps.json are frozen (lower only, or Size-Exception: in the PR body). npm run ratchet lowers caps per RC; it never raises one (scripts/size/README.md).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AV6Ya8uSU8gut3V3odqxck

Copy link
Copy Markdown
Member Author

The failing check on 8163bc3 is @solidjs/signals#test › tests/attribution-engine-cost.test.ts › "engine enabled with defaults: a re-run costs within the cap of the idle observe build" (folded 4.27x ... expected 4.27 to be less than 4). It's a wall-clock ratio test in packages/signals, which this PR doesn't touch (the diff is packages/web plus size scripts). The full signals suite passes locally on this branch (301 files, 5217 tests). No fix is needed from this PR; the run on the current head (e05e866) is the re-check.


Generated by Claude Code

@coveralls

coveralls commented Oct 14, 2026 •

Copy link
Copy Markdown

Coverage Report for CI Build 38588871668

Coverage remained the same at 77.515%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 1326
Covered Lines: 1084
Line Coverage: 81.75%
Relevant Branches: 1040
Covered Branches: 750
Branch Coverage: 72.12%
Branches in Coverage %: Yes
Coverage Strength: 27.5 hits per line

💛 - Coveralls

@codspeed

codspeed Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Merging this PR will degrade performance by 12.63%

⚠️ 2 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 2 regressed benchmarks
✅ 193 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
❌ memo + sync render effect only (reference) 26.4 ms 30.4 ms -13.15%
❌ createStore setter: delete + set one root key (#3044 overlay) 1.8 ms 2 ms -12.1%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing claude/project-thread-pe3qnr (9b9b921) with next (f048691)

Open in CodSpeed

ryansolid commented Oct 14, 2026 •

Copy link
Copy Markdown
Member Author

CodSpeed's three regressions look like runner noise. Correction: once next was merged in (9b41d0f), the two packages/signals regressions went away, but link-claim.bench.tsx (renderToString over 1,475 anchors) stayed at -13.75%. That one was real. dropCap was a closure created inside ssr(), so every template instance paid for one extra allocation, even on paths that never escalate. A local A/B against next shows the same slowdown. 00079cd writes the removal inline at the three escalation sites instead, and the local bench is back level with next. The cost is 10 B more in brotli: the renderToString floor goes to 20.56 KB.


Generated by Claude Code

…-call closure

The dropCap closure in ssr() cost ~13% on the link-claim renderToString
bench (one allocation per template instance). Write the removal at the
three escalation sites instead; renderToString floor 20.55 -> 20.56 KB.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AV6Ya8uSU8gut3V3odqxck
Re-measures the renderToString floor on top of #4043: 20,570 B
(72,373 minified), combined cap 20.58 KB as the maintainer accepted.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KRieFbP8sapvTBygyVm3mg

Copy link
Copy Markdown
Member Author

CodSpeed's two flagged regressions are not this PR's: both are @solidjs/signals benches (mount-rows "memo + sync render effect only", and the createStore root-key setter), and this PR changes no signals code (only packages/web/src/server.ts, the frames client, tests and size ledgers). CodSpeed also reports that those comparisons ran on different runtime environments. All other checks are green, so merging.


Generated by Claude Code

@ryansolid
ryansolid merged commit 45691d1 into next Oct 14, 2026
7 of 8 checks passed
ryansolid pushed a commit that referenced this pull request Oct 14, 2026
Re-measures the frames eager cap on top of #4055: 11,540 B brotli /
34,499 B minified.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KRieFbP8sapvTBygyVm3mg
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.

3 participants