fix(signals): LOADING_ON_OUTSIDE_HOLD reports only the same-source outside read (#3540) by ryansolid · Pull Request #3584 · solidjs/solid

ryansolid · 2026-09-22T10:14:43Z

Ruling

The DEV diagnostic LOADING_ON_OUTSIDE_HOLD (a <Loading on={...}> boundary re-armed but its fallback never displayed) fired in two shapes. Only the first is sound; this PR removes the second entirely.

Kept — at the change, same source. on re-arms the boundary while the very async source the boundary is waiting on is also read by a live reader outside it, so the frame is held on that source and the fallback can never be seen. Deterministic and structural; data.source names the source and the fix is to move the outside read under the boundary. Reported once, at the re-arm, before anything lands. The message now states that claim directly; latest() in on stays a de-emphasized parenthetical capability (docs: LOADING_ON_OUTSIDE_HOLD recommends the structural fix; latest() in on is a capability #3578 ruling).
Removed — after the fact. The frame was held past the content's landing (by the write's action, or by other pending data), so the staged swap was cleared before display.

Why the after-the-fact rule is unsound

A fallback that never shows in that shape is the outcome of a race the developer does not control: the nested boundary's data may simply be faster than the parent frame's hold. Had the action ended first (or the shell landed first), the fallback would have landed with the commit. A fallback that loses that race is a legitimate outcome, not a defect. The engine cannot distinguish "the action awaited exactly this data" (structural) from "the action awaited something else that happened to be slower" (race), so any rule that reports the hold after the fact reports races as defects.

What was removed

boundaries.ts: devHeldSweep, CollectionQueue._swapUnseen, CollectionQueue._devHeldSweep, the _rearm DEV assignment of _swapUnseen, the _checkSources DEV drop (_disabled._pendingValue !== true), and the now-unused currentTransition import. _settled, _judgeHeld and _output stay — they carry the pre-verdict boundary judgment from fix(signals): a boundary whose fallback is pending reveals landed content without waiting for the fallback's flight (#3540) #3581.
scheduler.ts: the DEV-only parked-finalize walk (devSweepBoundaryChildren) and its call site in finalizePureQueue.
The kept message reads: "on re-armed a Loading boundary, but comments is also read outside it and holds the frame: the fallback can never be seen — the frame waits on the very source the boundary is waiting on. Move the outside read under the boundary so one hold owns the data. (Reading latest() in on shows the fallback now, beside the held frame.)"

Tests

loading-on-frame-following-3540.test.ts: §3 same-source pins tightened on the new message (structure, data.source, latest() last in parentheses, no isPending); §5 "action outlasts the data" flipped to no report; new pin: plain write, frame held by OTHER pending data that outlasts the content (no action) → no report at any step, content reveals at the commit.
loading-on-rearm-reveal-3540.test.ts: both held-action reveal-at-commit tests flipped to [] / warn not called; comments rewritten — reveal-at-commit is by design (membership in the transaction is the criterion, not data lineage) and a never-displayed fallback in a race is not reported.
packages/web/test/loading-on-keyed-boundary-3540.spec.tsx: the held-action after-the-fact pin flipped to no report.
Signals 3,511 passed (baseline 3,510 + 1 new), solid 654, web 852.

Size

Prod core floor unchanged (26,054 B measured with the treeshake harness — boundaries.ts is shed from that fixture and the scheduler branch was __DEV__-folded). A core+createLoadingBoundary fixture: −9 B prod (29,969 → 29,960, the _swapUnseen class field), −647 B dev (40,018 → 39,371). No cap ratchet needed; scripts/size/.size-limit.js untouched.

Docs

documentation/solid-2.0/08-dev-diagnostics.md — LOADING_ON_OUTSIDE_HOLD section: single message, single trigger; the "after the fact" bullet, the action example and the isPending action-shape snippet removed; the race carve-out is now the rule; the summary table row updated.
documentation/solid-2.0/05-async-data.md — the diagnostic no longer claims to name the action shape.
packages/solid/CHEATSHEET.md, packages/solid/src/client/flow.ts (Loading JSDoc), packages/signals/src/boundaries.ts (createLoadingBoundary JSDoc).
Pending changesets loading-on-follows-frame.md and fix-boundary-pending-fallback-reveal.md trimmed so the release notes do not announce a rule that never ships.

Public API changes

No exports, props, options or parameters change. The diagnostic code LOADING_ON_OUTSIDE_HOLD stays; one of its two messages is removed (documented behavior of a diagnostic): the after-the-fact report "the frame was held until its content settled, so the fallback was never displayed…" no longer fires. The remaining message's wording changes (drops the isPending() suggestion; states the deterministic claim). data.source is now always present on the event.

Co-authored-by: Claude via Cursor noreply@cursor.com

changeset-bot · 2026-09-22T10:14:50Z

🦋 Changeset detected

Latest commit: 0366e67

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

This PR includes changesets to release 11 packages

Name	Type
@solidjs/signals	Patch
solid-js	Patch
test-integration	Patch
@solidjs/web	Patch
@solidjs/element	Patch
@solidjs/h	Patch
@solidjs/html	Patch
@solidjs/universal	Patch
@solidjs/babel-plugin	Patch
@solidjs/compiler	Patch
@solidjs/diagnostics	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

coveralls · 2026-09-22T10:22:12Z

Coverage Report for CI Build 35745853461

Coverage remained the same at 72.775%

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


Relevant Lines:	1070
Covered Lines:	824
Line Coverage:	77.01%
Relevant Branches:	818
Covered Branches:	550
Branch Coverage:	67.24%
Branches in Coverage %:	Yes
Coverage Strength:	17.38 hits per line

💛 - Coveralls

codspeed · 2026-09-22T10:35:08Z

Merging this PR will not alter performance

✅ 172 untouched benchmarks
⏩ 3 skipped benchmarks¹

_{Comparing fix/loading-on-outside-hold-same-source (0366e67) with next (9adf007)}

3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

Measured against next (4d7638c), brotli: - signals: + createStore: 16,866 B vs 16,805 (+61 B); cap 16.85 -> 16.90 KB. Minified bundle length unchanged; the delta is the prop mangler's short-name reshuffle after two _ props left CollectionQueue. - app: CSR, observe tier + attribution engine enabled: 27,767 B vs 27,756 (+11 B; next already 6 B over the old cap since #3581); cap 27.75 -> 27.80 KB. - boundaries.ts is -9 B minified on both tiers; scheduler.ts 0 B. - All other scenarios under cap: core floor 9,675 (+4), isPending/latest 12,411 (-20), simple-app 12,489 (+16), hydrating 20,655 (-49), hydrating + stores 30,825 (-55), CSR 15,779 (+36), CSR observe 17,293 (+18), frames 11,384 (0). Co-authored-by: Claude via Cursor <noreply@cursor.com>

…tside read (#3540) The DEV diagnostic fired in two shapes. Only the first is sound: - At the change, same source (kept): `on` re-arms the boundary while the very async source it is waiting on is also read by a live reader outside it, so the frame is held on that source and the fallback can never be seen. Deterministic and structural; `data.source` names the source and the fix is to move the outside read under the boundary. The message now states that claim; `latest()` in `on` stays a parenthetical capability (#3578 ruling). - After the fact (removed): the frame was held — by the write's action, or by other pending data — past the content's landing, so the staged swap was cleared before display. That is a race the developer does not control (the nested boundary's data may simply be faster than the frame's hold) and a fallback that loses it is a legitimate outcome, not a defect. The engine cannot distinguish "the action awaited exactly this data" from "the action awaited something else that happened to be slower", so the rule is unsound. Removed with it: `devHeldSweep`, `CollectionQueue._swapUnseen` and `_devHeldSweep`, the `_checkSources` DEV drop, and the scheduler's DEV-only parked-finalize walk (`devSweepBoundaryChildren`). `_settled`, `_judgeHeld` and `_output` stay — they carry the pre-verdict boundary judgment (#3581). Prod core floor unchanged (boundaries.ts is shed from that fixture; the scheduler branch was DEV-folded); core+Loading −9 B prod, −647 B dev. Tests: the after-the-fact pins flip to "not reported" (frame-following 5., rearm-reveal held-action cases, the web held-action spec); the same-source pins tighten on the new message; one new pin for a frame held by OTHER pending data that outlasts the content with no action — no report, the content reveals at the commit. Docs, CHEATSHEET, `Loading` JSDoc and the two pending changesets that announced the removed rule updated to match. Co-authored-by: Claude via Cursor <noreply@cursor.com>

…ld removal (#3584) Measured against next (4d7638c), brotli: - signals: + createStore: 16,866 B vs 16,805 (+61 B); cap 16.85 -> 16.90 KB. Minified bundle length unchanged; the delta is the prop mangler's short-name reshuffle after two _ props left CollectionQueue. - app: CSR, observe tier + attribution engine enabled: 27,767 B, under the 27.80 KB cap `next` already re-set in #3587 — no change here. - boundaries.ts is -9 B minified on both tiers; scheduler.ts 0 B. - All other scenarios under cap: core floor 9,675 (+4), isPending/latest 12,411 (-20), simple-app 12,489 (+16), hydrating 20,655 (-49), hydrating + stores 30,825 (-55), CSR 15,779 (+36), CSR observe 17,293 (+18), frames 11,384 (0). Co-authored-by: Claude via Cursor <noreply@cursor.com>

signals: + createStore: cap unchanged at 16.90 KB (set by #3584), measured at 16,891 B against next's 16,866 (+25 B; 9 B under the cap) — merge()'s presized source arrays and the $RECORD classification in store/utils.ts. Every other scenario is within its cap: core floor 0 B, isPending/latest +31 B, every-store-family app +32 B, CSR observe +11 B (prop mangler short names / brotli layout), the rest 0 B. The observe + attribution cap was re-set by #3587 on next; this PR adds 0 B there. Co-authored-by: Claude via Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>

…rElement/merge at the writer floor (#3562) * perf(ssr): spread elements — trailing attributes as markup, source literals hoisted, ssrElement/merge fast paths Compilers (Babel + Oxc, byte-identical): - Attributes after an element's last spread are markup, not a source. `<li {...rest} class="row" data-id={id}>` compiles to ssrElement("li", rest, …, _sk$, () => ' class="row"' + ssrElementAttribute("data-id", id)) with a hoisted skip predicate per key set; the string is a literal when every part is static. No later source can override a trailing attribute, so output is unchanged. Child properties, textarea value, reserved namespaces and JSX-valued attributes stay sources. - A spread element's source literal with getters (`<li data-id={id} class="row" {...rest}>`) is marked for hoistProps and compiles to the same hoisted constructor as component props. Runtime (@solidjs/web): - ssrElement takes an optional trailing `attrs`: a string, or a thunk called after the source walk and before the children — where the trailing source's getters were read, so hydration ids keep their order (spec: a server memo in the tail yields the same _hk sequence as the getter and template forms). - ssrElementAttribute(key, value): one attribute by the spread walk's rules, for compiler-emitted tail thunks. - Void-tag test and attribute-name escape cached per name; plain single source read and keyed in place. Runtime (@solidjs/signals): - merge(): presized source arrays; the get trap walks plain sources directly, reading before the `in` check. spread-static-tail server bench, 500 rows (ops/s, Babel): tail {...a} class 5,038 -> 5,057 head class {...a} 3,646 -> 3,661 mixed {...a} class data={id} 3,115 -> 3,790 (hoisted source) -> 4,437 (thunk) 1.42x dynhead data={id} class {...a} 2,821 -> 3,418 1.21x Co-authored-by: Claude via Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com> * perf(web): ssrElement remembers one record per tag and reads every source body directly Per tag name one cached TagInfo — `<tag`, `</tag>`, void, textarea, raw-text — in place of building both strings and testing the name per element (the tag work was 13 of the 22 ns/element over a hand-written writer). The source-kind dispatch leaves the key loop: sourceGet was `s[key]` for every kind ssrElement sees, so each body is read as props[prop]; only the key list depends on kind (Reflect.ownKeys through one trap for a store/foreign proxy, as sourceKeys did, else Object.keys). No second loop, no output change. Isolation (btn-variant inputs vs a yak-shaped writer): 70 -> 60 µs/1000 elements, 1.46x -> 1.18x, 22 -> 9 ns/element. Test: a store and a foreign proxy as the single source are walked through one ownKeys trap and get, never a descriptor trap. Co-authored-by: Claude via Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com> * fix(signals): narrow the merge trap's table state so the package typechecks The inlined read trap kept `view.table` (number | Map | null) in one binding through the branch that reaches `table.get`, so `pnpm build` in signals exited 2 while rollup still emitted JS. Read the state once and give the table its own binding. Co-authored-by: Cursor <cursoragent@cursor.com> * perf(signals): one `$RECORD` read classifies a proxy source — views answer their record, stores answer undefined on the fast path A `$PROXY`-marked source was classified by `$TARGET`, then `$VIEW`, then `$OMIT`: two to four trap hops per source at every merge(), omit(), isStatic and ssrElement. One brand symbol the merge and omit traps answer first and the store trap answers undefined in its symbol branch replaces the three private ones. merge over an omit view 81 → 50 ns, omit over a merge 63 → 36 ns, ssrElement on a view −8%. A branded proxy that predates the key forwards it to a target without it: still a leaf, one read. Co-authored-by: Cursor <cursoragent@cursor.com> * chore(size): audit note for ssrElement/merge at the writer floor (#3562) signals: + createStore: cap unchanged at 16.90 KB (set by #3584), measured at 16,891 B against next's 16,866 (+25 B; 9 B under the cap) — merge()'s presized source arrays and the $RECORD classification in store/utils.ts. Every other scenario is within its cap: core floor 0 B, isPending/latest +31 B, every-store-family app +32 B, CSR observe +11 B (prop mangler short names / brotli layout), the rest 0 B. The observe + attribution cap was re-set by #3587 on next; this PR adds 0 B there. Co-authored-by: Claude via Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Claude via Cursor <noreply@cursor.com> Co-authored-by: Cursor <cursoragent@cursor.com>

ryansolid mentioned this pull request Sep 22, 2026

chore(size): re-set the observe + attribution cap after #3581 #3587

Merged

ryansolid force-pushed the fix/loading-on-outside-hold-same-source branch from 448c00c to 15d81b6 Compare September 22, 2026 15:09

ryansolid and others added 2 commits September 22, 2026 08:10

ryansolid force-pushed the fix/loading-on-outside-hold-same-source branch from 15d81b6 to 0366e67 Compare September 22, 2026 15:13

ryansolid merged commit 28fcc9b into next Sep 22, 2026
7 checks passed

ryansolid deleted the fix/loading-on-outside-hold-same-source branch September 22, 2026 15:33

ryansolid mentioned this pull request Sep 22, 2026

perf(ssr): spread-element tails as markup, hoisted spread sources, ssrElement/merge at the writer floor #3562

Merged

This was referenced Sep 22, 2026

Loading on should behave as a keyed <Show> around the boundary; a boundary mounted under a hold must not be born held (A29) #3540

Closed

feat: Chrome Performance panel tracks for Solid (@solidjs/web/performance-tracks) #3580

Merged

Navigation Menu

Uh oh!

fix(signals): LOADING_ON_OUTSIDE_HOLD reports only the same-source outside read (#3540) - #3584

fix(signals): LOADING_ON_OUTSIDE_HOLD reports only the same-source outside read (#3540)#3584
ryansolid merged 2 commits into
nextfrom
fix/loading-on-outside-hold-same-source

ryansolid commented Sep 22, 2026

Uh oh!

changeset-bot Bot commented Sep 22, 2026 •

edited

Loading

Uh oh!

coveralls commented Sep 22, 2026 •

edited

Loading

Uh oh!

codspeed Bot commented Sep 22, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ryansolid commented Sep 22, 2026

Ruling

Why the after-the-fact rule is unsound

What was removed

Tests

Size

Docs

Public API changes

Uh oh!

changeset-bot Bot commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

coveralls commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 35745853461

Coverage remained the same at 72.775%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

codspeed Bot commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

changeset-bot Bot commented Sep 22, 2026 •

edited

Loading

coveralls commented Sep 22, 2026 •

edited

Loading

codspeed Bot commented Sep 22, 2026 •

edited

Loading