diff --git a/.changeset/fix-held-conditional-effect.md b/.changeset/fix-held-conditional-effect.md new file mode 100644 index 000000000..25c0db2f9 --- /dev/null +++ b/.changeset/fix-held-conditional-effect.md @@ -0,0 +1,7 @@ +--- +"@solidjs/signals": patch +--- + +Conditional JSX across a held branch change stays coherent with its inputs (#3438). + +An effect's dependencies are the committed frame's until its run applies — the effect twin of the memo rule from #3410. A pass that direct-committed its value but whose run was stashed with a hold (the same flush pended an async memo) used to trim the dependencies its previous run still displayed, so a later mainline write to one of them never reached the effect: `{show() ? count() : "hidden"}` held on `show → false` showed `Count: 1` beside `Panel: 0` while `Show` still read `true`. The trim now waits for the run to apply, so the write re-derives the effect against the committed inputs (`Panel: 1` beside `Count: 1`), and the hold's landing reveals `hidden` with `Show: false`. diff --git a/packages/signals/docs/INTERNALS-ASYNC-STATE.md b/packages/signals/docs/INTERNALS-ASYNC-STATE.md index 1861eac05..232fb2480 100644 --- a/packages/signals/docs/INTERNALS-ASYNC-STATE.md +++ b/packages/signals/docs/INTERNALS-ASYNC-STATE.md @@ -108,7 +108,7 @@ Semantics of the `(_pendingValue, _overrideValue)` pair for an optimistic node - Fallback-caught async holds nothing — in both orders (ruled 2026-09-12, #3375). A collecting boundary consumes the notification, so a reader under a fallback never registers. A reader registered while its boundary showed content (forwarded) stays registered when the boundary's `on` changes and it flips to the fallback; `reporterBlocksSource` therefore walks the reporter's `_queue._parent` chain and treats a reporter behind a collecting pending-type boundary (`_collectionType & STATUS_PENDING && !_initialized`) as not live. If nothing outside the boundary consumes the flight, the hold is over; a reader outside it still holds. The reset itself calls `wakeParked()` so the re-judgement happens in the same drain. - Wake of parked transactions (`wokenTransitions`): the flush judges only the _active_ transaction; a parked one is re-entered by a stamped node's landing (`settleTransition`), a stamped recompute, or an action resuming. A reporter that stops counting for another reason — its boundary reset (above), or its disposal by ambient work (#3372: `disposeChildren(self)` on a node with `_transition` and `STATUS_PENDING`; a pending reader is always queued as a pending node, so the stamp is reliable) — is none of those: `reporterBlocksSource` would prune it at the next check, but no check comes, and the writes held with it stay staged. Such sites record the transaction (deduped) and schedule; the flush re-enters a woken transaction from the `finally` of a full pass — reached from the park exit and the normal exit alike — and only when idle: no `activeTransition` and `!scheduled`, which at that point means an empty dirty heap, no write since the heap ran (every write re-arms it) and, the finalize having reverted them, no optimistic ambient nodes. Entering adopts the ambient batch, and ambient work present at that instant would be held behind flights it never read; a wake in a pass with work just falls to the next. Entries are popped in a loop until one enters: a wake whose transaction completed by other means is a bare return (`initTransition` on `_done`) and must not strand the ones behind it. The fast drain defers to the full path while a wake is outstanding so such dead entries are still consumed. A wake with other live reporters re-parks; the idle pass is its only cost. Known shape: the ambient write that triggered the reset commits in its own pass and the released hold in the idle pass after it — two effect runs in one synchronous drain (`Sum: 1`, `Sum: 2` at the same clock time in the #3375 pin), never a visible tear. - Staged reads enter (A29, #3408): `read()` calls `enterStagedRead` on every selection that returns `_pendingValue` — the fast paths and the slow path — and it enters `el._transition` unless that is null (ambient batch), already active, or the read is a probe (`pendingCheckActive`). The third entry beside `setSignal` on a stamped node and `recompute` of a stamped node; rule text and the `Panel: 1` beside `Count: 0` shape live in the spec. A stale (render) reader never reaches it: the carve-out below serves it the committed value. -- Dependencies are the committed frame's (A30, #3410): `recompute`'s tail trims the previous pass's dependency tail only for a pass that published or changed nothing (`_pendingValue === NOT_PENDING` and no `_error`); a staged pass leaves it linked and `commitPendingNode` trims after a clean pass (`_error == null` — a set `_error` means the last pass threw, kept its full list, and `_depsTail` marks where it stopped). `__OBSERVE__` fan-in counting walks the validated prefix only, so a held tail does not inflate distinct-source counts. Why commit-time and not the pass, and the `Selected: 0` beside `Count: 1` shape: the spec. +- Dependencies are the committed frame's (A30, #3410): `recompute`'s tail trims the previous pass's dependency tail only for a pass that published or changed nothing (`_pendingValue === NOT_PENDING` and no `_error`); a staged pass leaves it linked and `commitPendingNode` trims after a clean pass (`_error == null` — a set `_error` means the last pass threw, kept its full list, and `_depsTail` marks where it stopped); an effect pass that direct-committed but still owes a run (`_modified`, #3438 — the flush may stash that run into a transaction it opens later) leaves it for `runEffect` to trim once the run applies. `__OBSERVE__` fan-in counting walks the validated prefix only, so a held tail does not inflate distinct-source counts. Why commit-time and not the pass, and the `Selected: 0` beside `Count: 1` shape: the spec. - Reveal-hold and its carve-out (#3305, #3334, re-ruled 2026-09-10): a reader landing on a node with `STATUS_PENDING` throws — the throw reaches `GlobalQueue.notify`, which opens a transaction for the reveal if none is active (#3305) and records the source as its reporter (INV-3); the reveal completes when the flight lands. One carve-out, the staged-value rule's twin for flights: a **stale** (render) reader of a node pending in some **other** transaction shows the node's committed value, does not entangle (its own writes stay outside that transaction), is recorded for that transaction's commit replay (`heldFromStale`), and joins the transaction's reporters for the node when it has an entry (#3374) — the reader displays the pre-flight value, so the transaction cannot commit the flight's inputs ahead of its answer just because the reader that opened the entry was disposed (a keyed remount). It is refused — the reader holds — when the committed value would tear against the frame: the node carries `CONFIG_INPUTS_PUBLISHED` (a batch or transaction committed with the node still pending, `commitPendingNode`'s computed branch: the flight's inputs are on screen; cleared when the node next enters pending from a settled state, `notifyStatus`), or the node is routed through a live lane (`GlobalQueue._laneLive` → `resolveLane`, exact rather than sticky: lane-revealed inputs, optimistic or `latest`), or the node is uninitialized (nothing committed to show). The stamp itself is pending-node bookkeeping and decides nothing. Replay hygiene: an effect recorded in `_gatedSubs` that later recomputes _under_ the transaction sees its staged view and is applied by the commit (ownership) — `recompute` drops the stale recording at its start (`activeTransition._gatedSubs.delete`), and a lane's committed-view read re-records during the run, so the lane replay (`laneReadsCommitted`) is untouched. - Settle-time re-entry, lane-routed nodes (#3334): `handleAsync`'s `settleTransition` re-enters `resolveTransition(el)` — for a lane-routed node the transaction that _owns_ the lane. That owner's commit is only the override's confirm/revert; the landing itself is revealed by the lane. If a transaction is _waiting_ on the node (`waitingTransition(el)`), the settle enters that one instead: entering the owner would fold a reveal that only waits on the flight into the owner's action (A18 node corollary, #2912). Every other transaction waiting on the flight then folds in explicitly (`enterWaiting`, #3407 — see the next bullet): each reveal that discovered the flight completes at its landing (A15). - Pass provenance for effects (#3407): a render effect's pass belongs to whatever dirtied it. `recompute` re-enters a stamped node's transaction only for memos (their value _is_ that transaction's work); an effect stamped by a transaction — it observed that transaction's flight — and dirtied by another transaction's write, or by mainline, runs that writer's pass, reads the held flight as a stale reader (committed value, `heldFromStale`) and publishes with the writer. The pass entangles only if it _observes_ a pending flight (the carve-out refused: inputs published, lane-live, uninitialized) — the throw reaches `GlobalQueue.notify`, which registers the writer's transaction as a reporter, and the flight's landing folds it in. Before, `recompute` re-entered an effect's stamp whenever _any_ other transaction was active: a sync `action` write to a signal that merely shared a hole with a held async (`{b()}:{detailsA()}`) merged into the async's transaction and waited (`0:0 → 2:1`, no `1:0`), while the same write made plainly passed through; two independent flights read in one hole settled as one unit. Now both writers publish on their own (`1:0` at the write, `1:1` at the landing; two flights land at their own times). The re-entry's other job — delivering a landing to the transactions waiting on it — moves to the landing itself: `settleTransition` enters every parked transaction whose reporters still observe the node (`enterWaiting`, over `sourceObserved`), including the waiter of a stampless node (a flight started under a batch that committed beneath it, #3305), whose landing used to open a fresh batch that the stamped reader's re-entry folded into the waiter. Pinned: `tests/shared-effect-no-entangle.test.ts`; the reveal-completion pins (`spec-async-semantics` A15, `reveal-carve-out`, `stale-read-uninitialized-cross-transition`) are the regression net for the fold. diff --git a/packages/signals/docs/RULES-INDEX.md b/packages/signals/docs/RULES-INDEX.md index 03bcb36e0..db8c33e51 100644 --- a/packages/signals/docs/RULES-INDEX.md +++ b/packages/signals/docs/RULES-INDEX.md @@ -45,12 +45,12 @@ Status legend: **live** stated and standing · **ruled** carries an explicit rul | id | status | defined | cited in src | cited in tests | statement (at definition) | | --- | ---------- | ---------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| A1 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:211` | — | onCleanup.test.ts×2 transitionEntanglement.test.ts×4 | [ruled 2026-07-06] Effect error interception is compute-phase only — `EffectBundle.error` intercepts compute-phase errors only; effect-phase throws escalate to the nearest error boundary (halt if none… | -| A2 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:219` | — | onCleanup.test.ts×2 | [ruled] Unhandled compute-phase errors in user effects are logged and skipped — Compute-phase errors in _user_ effects without a handler are logged and the run is skipped; the system keeps running. | -| A3 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:227` | — | equals-comparator-errors.test.ts×1 | [ruled] Comparator throws are compute-phase errors — Errors thrown by a user `equals` comparator behave exactly like compute-phase errors (boundary-containable; loud halt without a boundary). | -| A4 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:235` | — | equals-comparator-errors.test.ts×1 | [ruled] A custom `equals` never sees `undefined` prev on first commit — A custom `equals` is never invoked with `undefined` previous value on a node's first commit. | -| A5 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:243` | — | errorHalt.test.ts×1 | [ruled] An error escaping every boundary halts the system — An error escaping every boundary permanently halts the system with `REACTIVITY_HALTED`; later writes log "Update ignored". | -| A6 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:251` | — | enforceLoadingBoundary.test.ts×1 | [ruled] `ASYNC_OUTSIDE_LOADING_BOUNDARY` is warn-only — `ASYNC_OUTSIDE_LOADING_BOUNDARY` is a warn-only diagnostic; an `Errored` above must not swallow it and must not show its fallback for a pending. | +| A1 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:213` | — | onCleanup.test.ts×2 transitionEntanglement.test.ts×4 | [ruled 2026-07-06] Effect error interception is compute-phase only — `EffectBundle.error` intercepts compute-phase errors only; effect-phase throws escalate to the nearest error boundary (halt if none… | +| A2 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:221` | — | onCleanup.test.ts×2 | [ruled] Unhandled compute-phase errors in user effects are logged and skipped — Compute-phase errors in _user_ effects without a handler are logged and the run is skipped; the system keeps running. | +| A3 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:229` | — | equals-comparator-errors.test.ts×1 | [ruled] Comparator throws are compute-phase errors — Errors thrown by a user `equals` comparator behave exactly like compute-phase errors (boundary-containable; loud halt without a boundary). | +| A4 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:237` | — | equals-comparator-errors.test.ts×1 | [ruled] A custom `equals` never sees `undefined` prev on first commit — A custom `equals` is never invoked with `undefined` previous value on a node's first commit. | +| A5 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:245` | — | errorHalt.test.ts×1 | [ruled] An error escaping every boundary halts the system — An error escaping every boundary permanently halts the system with `REACTIVITY_HALTED`; later writes log "Update ignored". | +| A6 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:253` | — | enforceLoadingBoundary.test.ts×1 | [ruled] `ASYNC_OUTSIDE_LOADING_BOUNDARY` is warn-only — `ASYNC_OUTSIDE_LOADING_BOUNDARY` is a warn-only diagnostic; an `Errored` above must not swallow it and must not show its fallback for a pending. | | A7 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:93` | — | spec-async-semantics.test.ts×2 | [ruled] Resolved async never reads `[false, undefined]` — After an async memo resolves, `[isPending(x), latest(x)]` is `[false, resolvedValue]` — never `[false, undefined]`. | | A8 | amended | `docs/SPEC-ASYNC-SEMANTICS.md:101` | — | createMemo.test.ts×1 | [ruled, amended in place 2026-07-07] `isPending(() => latest(x))` follows `x`'s own async only — verdicts are per-channel — (**re-ruled 2026-07-07c** — was "tracks the transition the same as `isPendin… | | A9 | amended | `docs/SPEC-ASYNC-SEMANTICS.md:109` | — | spec-async-semantics.test.ts×3 | [ruled, amended in place 2026-07-07] Store leaves behind a firewall report the firewall's new-question refetch — `isPending` on a store leaf behind a firewall reports the firewall's refetch like any a… | @@ -64,26 +64,26 @@ Status legend: **live** stated and standing · **ruled** carries an explicit rul | A17 | amended | `docs/SPEC-ASYNC-SEMANTICS.md:31` | async.ts×3 constants.ts×2 core.ts×7 invariants.ts×3 optimistic.ts×5 scheduler.ts×2 verdict.ts×1 signals.ts×2 optimistic.ts×1 store.ts×3 | optimistic-undefined-override.test.ts×1 refresh-await.test.ts×1 reveal-gating-contract.test.ts×3 spec-async-semantics.test.ts×10 createOptimisticStore.test.ts×1 treeshake.test.ts×1 until.test.ts×1 | [ruled, amended in place 2026-07-06 (promoted from C4)] An active override is the displayed value until its transaction commits, and the graph's value until its own source answers — \*\*Statement (curre… | | A18 | amended | `docs/SPEC-ASYNC-SEMANTICS.md:41` | async.ts×2 constants.ts×1 core.ts×4 optimistic.ts×4 scheduler.ts×2 types.ts×2 optimistic.ts×1 | createOptimistic.test.ts×1 spec-async-semantics.test.ts×3 flight-owned-transaction.test.ts×1 | [ruled, amended in place 2026-07-07 (promoted from B4)] An override lives exactly as long as its own transaction; a newer truth from the source supersedes it in the graph immediately, on screen at com… | | A19 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:77` | async.ts×1 optimistic.ts×1 | spec-async-semantics.test.ts×3 uninitialized-visibility.test.ts×1 | [ruled 2026-07-07 (promoted from C1)] `isPending(x)` ≡ the observable value is not final (three causes) — (was C1 — **partially reverses an earlier decision**) \*\*Definition: `isPending(x)` ≡ the value… | -| A20 | superseded | `docs/SPEC-ASYNC-SEMANTICS.md:263` | invariants.ts×1 | question-scoped-pending.test.ts×2 spec-async-semantics.test.ts×3 createOptimisticStore.test.ts×1 | [superseded 2026-07-13 by A24] (superseded) Optimistic writes announce a store-wide pending — (**SUPERSEDED 2026-07-13 by A24** — the mask is deleted; optimistic writes are verdict-inert. Kept for the… | -| A21 | superseded | `docs/SPEC-ASYNC-SEMANTICS.md:270` | — | question-scoped-pending.test.ts×3 spec-async-semantics.test.ts×3 | [superseded 2026-07-13 by A24] (superseded) The store-wide mask — (**SUPERSEDED 2026-07-13 by A24** — the store-wide mask is deleted with the mask model; nothing silences a new question. The effective… | +| A20 | superseded | `docs/SPEC-ASYNC-SEMANTICS.md:265` | invariants.ts×1 | question-scoped-pending.test.ts×2 spec-async-semantics.test.ts×3 createOptimisticStore.test.ts×1 | [superseded 2026-07-13 by A24] (superseded) Optimistic writes announce a store-wide pending — (**SUPERSEDED 2026-07-13 by A24** — the mask is deleted; optimistic writes are verdict-inert. Kept for the… | +| A21 | superseded | `docs/SPEC-ASYNC-SEMANTICS.md:272` | — | question-scoped-pending.test.ts×3 spec-async-semantics.test.ts×3 | [superseded 2026-07-13 by A24] (superseded) The store-wide mask — (**SUPERSEDED 2026-07-13 by A24** — the store-wide mask is deleted with the mask model; nothing silences a new question. The effective… | | A22 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:157` | — | spec-async-semantics.test.ts×1 | [ruled 2026-07-08] Pending is per-node; store-wide only for the firewall's own work — \*\*Pending is per-node: store-wide verdicts exist only as the firewall's own in-flight work (A9) and the decree tha… | | A23 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:165` | — | spec-async-semantics.test.ts×1 | [ruled 2026-07-08] The `isPending` probe is reads-only — **The `isPending` probe is reads-only — the thunk's return value is never inspected.** `isPending(() => store)` reads nothing and reports `fals… | | A24 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:85` | — | optimistic-undefined-override.test.ts×1 reveal-gating-contract.test.ts×1 spec-async-semantics.test.ts×2 | [ruled 2026-07-13] Question-scoped pending: pending iff a value change is in flight or an `affects()` mark is live — (**ruled 2026-07-13** — supersedes A20/A21; the converged model from the #2844/#272… | -| A25 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:201` | — | uninitialized-visibility.test.ts×3 | [ruled 2026-07-16] A derived store's seed is a draft, never an observable value — (**ruled 2026-07-16**, #2897) **A derived store's seed is a draft, never an observable value.** The seed exists for th… | +| A25 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:203` | — | uninitialized-visibility.test.ts×3 | [ruled 2026-07-16] A derived store's seed is a draft, never an observable value — (**ruled 2026-07-16**, #2897) **A derived store's seed is a draft, never an observable value.** The seed exists for th… | | A26 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:59` | scheduler.ts×1 | action-await-contract.test.ts×2 | [ruled 2026-07-17] An ambient transaction window is one flush; parking is flush-driven — (**ruled 2026-07-17**, #2913; **enforcement hardened 2026-08-31**, #3141 — parking is flush-driven, and a trans… | -| A27 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:193` | — | loading-value.test.ts×2 | [ruled 2026-08-10] The commit-#0 loading window is loading-class and verdict-quiet — (**ruled 2026-08-10**) **The commit-#0 loading window is loading-class and verdict-quiet.** A node born committed v… | +| A27 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:195` | — | loading-value.test.ts×2 | [ruled 2026-08-10] The commit-#0 loading window is loading-class and verdict-quiet — (**ruled 2026-08-10**) **The commit-#0 loading window is loading-class and verdict-quiet.** A node born committed v… | | A29 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:67` | core.ts×1 | held-conditional-memo.test.ts×1 | [ruled 2026-09-13 (#3408)] A tracked read served a live transaction's staged value enters that transaction — A tracked computation served a node's staged `_pendingValue` — a value a live transaction h… | -| A30 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:183` | core.ts×1 scheduler.ts×1 | held-conditional-memo.test.ts×1 | [ruled 2026-09-13 (#3410)] A memo's dependencies are the committed frame's until the frame is replaced — A pass that _staged_ its value has not replaced the committed frame, so the committed value sti… | +| A30 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:183` | attribution.ts×1 core.ts×1 effect.ts×1 scheduler.ts×1 | held-conditional-effect.test.ts×1 held-conditional-memo.test.ts×1 treeshake.test.ts×1 | [ruled 2026-09-13 (#3410)] A memo's dependencies are the committed frame's until the frame is replaced — A pass that _staged_ its value has not replaced the committed frame, so the committed value sti… | ## V — fixed violations | id | status | defined | cited in src | cited in tests | statement (at definition) | | --- | ------ | ---------------------------------- | ------------ | ------------------------------ | ------------------------------------------------------------------------------ | -| V1 | fixed | `docs/SPEC-ASYNC-SEMANTICS.md:333` | async.ts×1 | spec-async-semantics.test.ts×7 | - **V1 (violated A13) — FIXED.** A _resting_ optimistic node reported | -| V2 | fixed | `docs/SPEC-ASYNC-SEMANTICS.md:343` | async.ts×1 | spec-async-semantics.test.ts×2 | - **V2 (violated A7/A13) — FIXED.** `latest()`'s verdict in the window was | -| V3 | fixed | `docs/SPEC-ASYNC-SEMANTICS.md:349` | — | spec-async-semantics.test.ts×2 | - **V3 (violated A19) — FIXED.** After a reporter-less transition completed, | -| V4 | fixed | `docs/SPEC-ASYNC-SEMANTICS.md:356` | — | spec-async-semantics.test.ts×5 | - \*\*V4 (violated the old A20's three-form algebra) — FIXED, then the rule it | -| V5 | live | `docs/SPEC-ASYNC-SEMANTICS.md:368` | — | spec-async-semantics.test.ts×3 | - \*\*V5 (A17 corollary — found and fixed with the revert-target elimination, | +| V1 | fixed | `docs/SPEC-ASYNC-SEMANTICS.md:335` | async.ts×1 | spec-async-semantics.test.ts×7 | - **V1 (violated A13) — FIXED.** A _resting_ optimistic node reported | +| V2 | fixed | `docs/SPEC-ASYNC-SEMANTICS.md:345` | async.ts×1 | spec-async-semantics.test.ts×2 | - **V2 (violated A7/A13) — FIXED.** `latest()`'s verdict in the window was | +| V3 | fixed | `docs/SPEC-ASYNC-SEMANTICS.md:351` | — | spec-async-semantics.test.ts×2 | - **V3 (violated A19) — FIXED.** After a reporter-less transition completed, | +| V4 | fixed | `docs/SPEC-ASYNC-SEMANTICS.md:358` | — | spec-async-semantics.test.ts×5 | - \*\*V4 (violated the old A20's three-form algebra) — FIXED, then the rule it | +| V5 | live | `docs/SPEC-ASYNC-SEMANTICS.md:370` | — | spec-async-semantics.test.ts×3 | - \*\*V5 (A17 corollary — found and fixed with the revert-target elimination, | ## B — tier B @@ -100,8 +100,8 @@ Status legend: **live** stated and standing · **ruled** carries an explicit rul | id | status | defined | cited in src | cited in tests | statement (at definition) | | --- | ------ | ---------------------------------- | ------------ | -------------------------------------------------- | --------------------------------------------------------------------------- | | C1 | live | `docs/SPEC-ASYNC-SEMANTICS.md:77` | — | onCleanup.test.ts×2 spec-async-semantics.test.ts×1 | PROMOTED → A19 (A19's section carries the ruling). | -| C2 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:299` | — | onCleanup.test.ts×2 | - [x] **C2 — RULED (2026-07-07): reverts do not trump other live lanes.** A | -| C3 | closed | `docs/SPEC-ASYNC-SEMANTICS.md:309` | — | — | - [x] **C3 — CLOSED by A19 (2026-07-07): early completion is by design.** | +| C2 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:301` | — | onCleanup.test.ts×2 | - [x] **C2 — RULED (2026-07-07): reverts do not trump other live lanes.** A | +| C3 | closed | `docs/SPEC-ASYNC-SEMANTICS.md:311` | — | — | - [x] **C3 — CLOSED by A19 (2026-07-07): early completion is by design.** | | C4 | live | `docs/SPEC-ASYNC-SEMANTICS.md:31` | — | spec-async-semantics.test.ts×1 | PROMOTED → A17 (A17's section carries the ruling). | ## INV — invariants diff --git a/packages/signals/docs/SPEC-ASYNC-SEMANTICS.md b/packages/signals/docs/SPEC-ASYNC-SEMANTICS.md index 9515dbb60..1397fd071 100644 --- a/packages/signals/docs/SPEC-ASYNC-SEMANTICS.md +++ b/packages/signals/docs/SPEC-ASYNC-SEMANTICS.md @@ -183,11 +183,13 @@ A resting optimistic node reports pending via exactly the causes a plain async m ### A30. A memo's dependencies are the committed frame's until the frame is replaced **Status:** **ruled** 2026-09-13 (#3410) — maintainer ruling, Cluster 4 triage; the dependency twin of held children (#3404) -**Pinned by:** `tests/held-conditional-memo.test.ts` (#3410: a memo whose held pass stopped reading an input still follows a mainline write to it) -**Mechanism (index, 2026-09-14):** `recompute` trims the previous pass's dependency tail (`trimStaleDeps`) only when the pass published or changed nothing (`_pendingValue === NOT_PENDING`, no `_error`); a pass that staged its value leaves the tail linked and `commitPendingNode` trims it after a clean pass (`_error == null`). `__OBSERVE__` fan-in counting walks the validated prefix only. +**Pinned by:** `tests/held-conditional-memo.test.ts` (#3410: a memo whose held pass stopped reading an input still follows a mainline write to it); `tests/held-conditional-effect.test.ts` (#3438: an effect whose held pass stopped reading an input, its run stashed, still follows a mainline write to it) +**Mechanism (index, 2026-09-14):** `recompute` trims the previous pass's dependency tail (`trimStaleDeps`) only when the pass published or changed nothing (`_pendingValue === NOT_PENDING`, no `_error`) and, for an effect, owes no run (`_modified` clear); a pass that staged its value leaves the tail linked and `commitPendingNode` trims it after a clean pass (`_error == null`); an effect pass that direct-committed and owes a run leaves it for `runEffect` to trim once the run applies (again after a clean pass). `__OBSERVE__` fan-in counting and the attribution engine's subscription diff walk the validated prefix only. A pass that _staged_ its value has not replaced the committed frame, so the committed value still derives from the previous pass's dependencies and a write to one of them must reach the node — and, through the node's `_transition` stamp, join its hold — exactly as an unconditional read would. Before: `selected = fixed() ? 2 : count()` held on `fixed → true` dropped `count` at its held pass, and a mainline `count` write then published `Count: 1` beside the committed `Selected: 0` / `Fixed: false`. Decided at commit rather than at the pass because a plain flush knows nothing at recompute time: the transaction that ends up holding the pass may open later in the same flush (an async memo downstream pends and the batch is adopted). An errored pass (a throw, NotReady included, a comparator throw) keeps its full list as before, and the commit skips the trim by the same `_error`. Cost on the plain path: none — a pass that publishes trims at its tail as before; only a staged pass moves the trim to the same flush's commit. +An effect's frame is the run its value is applied by, not its value slot (#3438). A plain-flush pass direct-commits `_value` and enqueues the run, but the same flush can still become a hold (the async memo downstream pends, the batch is adopted) and stash that run with the transaction — so the committed frame is still what the _last_ run published, and it still derives from the previous pass's dependencies. Before: `{show() ? count() : "hidden"}` (the compiler's insert effect) held on `show → false` dropped `count` at its pass, and the mainline `count` write then published `Count: 1` beside `Panel: 0` while `Show` still read `true`. Now the pass leaves the tail while a run is owed and `runEffect` trims once it applies. The write reaches the effect; a render effect is a mainline reader of the foreign hold (it is served the committed `show`, A15's stale-reader term), so it re-derives `Panel: 1` beside `Count: 1` in the mainline frame, and the hold's reveal re-derives it to `hidden` — unlike a memo, which is served the staged value and joins the hold (A29). Both frames are coherent; the shapes differ because effects render mainline by design. A pass that changed nothing owes no run and trims at its tail as before. + ## Loading window and seeds ### A27. The commit-#0 loading window is loading-class and verdict-quiet diff --git a/packages/signals/src/core/attribution.ts b/packages/signals/src/core/attribution.ts index 94de8a274..ed2540ec0 100644 --- a/packages/signals/src/core/attribution.ts +++ b/packages/signals/src/core/attribution.ts @@ -754,10 +754,20 @@ function markSeen(el: Computed): void { (el as AttributedNode)._devSeenSeq = changeSeq; } -/** Snapshot the node's current dep identities (call before a run replaces them). */ +/** + * Snapshot the dep identities of the node's last pass (call before a run + * replaces them, or after it to read the fresh set). The validated prefix + * [`_deps`..`_depsTail`] is that pass's set; links past the tail are a + * previous pass's, kept linked while the frame they fed is still the + * committed one (A30 — a staged memo pass, or an effect pass whose run is + * still owed) and not part of the subscription diff. + */ function captureDeps(el: Computed): unknown[] { const deps: unknown[] = []; - for (let l = el._deps; l !== null; l = l._nextDep) deps.push(l._dep); + for (let l = el._deps; l !== null; l = l._nextDep) { + deps.push(l._dep); + if (l === el._depsTail) break; + } return deps; } @@ -775,6 +785,7 @@ function checkDepWidth(el: Computed): void { for (let l = el._deps; l !== null; l = l._nextDep) { count++; if (names.length < 12) names.push(nodeName(l._dep)); + if (l === el._depsTail) break; // the validated prefix, as captureDeps } const node = el as AttributedNode; if (count < limit || count < (node._devWideWarnedAt ?? 0) * 1.5) return; diff --git a/packages/signals/src/core/core.ts b/packages/signals/src/core/core.ts index 203e08be8..2fb7f19fe 100644 --- a/packages/signals/src/core/core.ts +++ b/packages/signals/src/core/core.ts @@ -653,7 +653,14 @@ export function recompute(el: Computed, create: boolean = false): void { // changed nothing, trims now. An errored pass (a throw, NotReady included, // or a comparator throw above) keeps its full list as before — `_depsTail` // marks where it stopped — and the commit skips it by the same `_error`. - if (!el._x?._error && el._pendingValue === NOT_PENDING) trimStaleDeps(el); + // An effect's frame is the run its value is applied by, not the value slot + // (#3438): a direct-committed pass that still owes a run (`_modified`) has + // not replaced what the last run published — the same flush may stash that + // run into a transaction it opens later — so its tail waits for `runEffect` + // to trim once the run applies. A pass that changed nothing owes no run + // and trims here. + if (!el._x?._error && el._pendingValue === NOT_PENDING && !(isEffect && (el as any)._modified)) + trimStaleDeps(el); // Attribution hook: fired before the lane restore so `currentOptimisticLane` // still reflects THIS run's posture. The facts distinguish an overlay // recompute (optimistic lane, transition replay, transition-held commit) diff --git a/packages/signals/src/core/effect.ts b/packages/signals/src/core/effect.ts index 727d58ab0..5139e4e06 100644 --- a/packages/signals/src/core/effect.ts +++ b/packages/signals/src/core/effect.ts @@ -21,6 +21,7 @@ import { import { attrHooks } from "./attribution-hooks.js"; import { emitDiagnostic, reportDiagnostic } from "./dev.js"; import { StatusError, unwrapStatusError } from "./error.js"; +import { trimStaleDeps } from "./graph.js"; import { enqueueSub } from "./heap.js"; import { _hitUnhandledAsync, @@ -206,6 +207,9 @@ function runEffect(node: Effect, type: number): void { } return; } + // Captured before the callback: its own throw errors the node below, but + // the compute pass that produced `_value` was clean, so its tail still goes. + const cleanPass = node._x?._error == null; let prevStrictRead: string | false = false; if (__DEV__) { prevStrictRead = setStrictRead("an effect callback"); @@ -238,6 +242,12 @@ function runEffect(node: Effect, type: number): void { } node._prevValue = node._value; node._modified = false; + // The run applied: this is the frame now, so the dependency tail the + // compute pass left linked goes (A30, #3438 — `recompute` defers an + // effect's trim while a run is owed; the twin of `commitPendingNode`'s + // trim for a staged pass). An errored compute kept its full list with + // `_depsTail` marking where it stopped; leave it, as the commit does. + if (cleanPass) trimStaleDeps(node); } // Outside the try (see the rule in attribution-hooks.ts). Reached whether or // not the callback threw — a throw that escapes the catch above halts. diff --git a/packages/signals/tests/held-conditional-effect.test.ts b/packages/signals/tests/held-conditional-effect.test.ts new file mode 100644 index 000000000..99697a577 --- /dev/null +++ b/packages/signals/tests/held-conditional-effect.test.ts @@ -0,0 +1,117 @@ +import { describe, expect, it } from "vitest"; +import { + createMemo, + createRenderEffect, + createRoot, + createSignal, + flush, + getOwner +} from "../src/index.js"; + +let now = 0; +let timers: { at: number; run: () => void }[] = []; +function delay(ms: number, value?: T): Promise { + return new Promise(r => timers.push({ at: now + ms, run: () => r(value as T) })); +} +async function settle() { + for (let r = 0; r < 3; r++) { + for (let i = 0; i < 10; i++) await Promise.resolve(); + flush(); + } +} +async function advanceTo(t: number) { + while (true) { + timers.sort((a, b) => a.at - b.at); + const next = timers[0]; + if (!next || next.at > t) break; + timers.shift(); + now = next.at; + next.run(); + await settle(); + } + now = t; + await settle(); +} +function reset() { + now = 0; + timers = []; +} +function frames(log: string[], when: number[]): string[] { + const byTime = new Map(); + log.forEach((v, i) => (byTime.get(when[i]) ?? byTime.set(when[i], []).get(when[i])!).push(v)); + return [...byTime].map(([t, vs]) => `${t}: ${vs.sort().join(" | ")}`); +} +function text(fn: () => string, log: string[], when: number[]) { + let last: string | undefined; + let node: any; + createRenderEffect( + () => { + node ??= getOwner(); + return fn(); + }, + v => { + if (v !== last) { + last = v; + log.push(v); + when.push(now); + } + } + ); + return () => { + const names: string[] = []; + for (let d = node._deps; d !== null; d = d._nextDep) + names.push(d._dep._name + (d === node._depsTail ? "]" : "")); + return names.join(","); + }; +} + +describe("conditional render effect across a held branch change (#3438)", () => { + it("A30 / #3438 an effect whose held pass trimmed an input still agrees with its committed inputs", async () => { + reset(); + const log: string[] = []; + const when: number[] = []; + let setCount!: (v: number) => void; + let setShow!: (v: boolean) => void; + let deps!: () => string; + createRoot(() => { + const [count, sC] = createSignal(0, { name: "count" }); + const [show, sS] = createSignal(true, { name: "show" }); + setCount = sC; + setShow = sS; + const delayedShow = createMemo(() => delay(1000, show()), undefined, { name: "delayedShow" }); + // Shape the compiler emits for `{show() ? count() : "hidden"}`. + const c = createMemo(() => !!show(), undefined, { name: "c" }); + text(() => `Count: ${count()}`, log, when); + text(() => `Show: ${show()}`, log, when); + deps = text(() => `Panel: ${c() ? count() : "hidden"}`, log, when); + text(() => `Delayed: ${delayedShow()}`, log, when); + }); + flush(); + await settle(); + await advanceTo(2000); + expect(deps()).toBe("computed,count]"); + setShow(false); + await settle(); + // The held pass validated only `c`; count stays linked past the tail. + expect(deps()).toBe("computed],count"); + await advanceTo(2500); + setCount(1); + await settle(); + expect(deps()).toBe("computed,count]"); + await advanceTo(5000); + // The `show` flip (2000) is held by delayedShow. Panel's held pass stopped + // reading count; before, its trim dropped that edge at the pass while the + // run was stashed, so the count write (2500) revealed `Count: 1` beside + // `Panel: 0` (the frame still showing the old count). Now the edge lives + // until the run applies: the write reaches Panel, which — a render effect, + // a mainline reader of the foreign hold — re-derives against the committed + // `show` and publishes `Panel: 1` with `Count: 1`, then lands `hidden` with + // the hold's reveal. + expect(frames(log, when)).toEqual([ + "0: Count: 0 | Panel: 0 | Show: true", + "1000: Delayed: true", + "2500: Count: 1 | Panel: 1", + "3000: Delayed: false | Panel: hidden | Show: false" + ]); + }); +}); diff --git a/packages/signals/tests/treeshake.test.ts b/packages/signals/tests/treeshake.test.ts index f3cd7699c..54ea7a898 100644 --- a/packages/signals/tests/treeshake.test.ts +++ b/packages/signals/tests/treeshake.test.ts @@ -279,6 +279,10 @@ describe("pay-for-use tree-shaking (#2883)", () => { // committed frame's, like its children). Core-retained by necessity: // both sit on read()'s value selection and recompute's tail. Measured at // 23,318 on top of #3434 (23,195 → 23,318). + // NOTE (2026-09-14, no bump): +35 B for the effect arm of A30 (#3438) — + // recompute's tail keeps an effect's dependency tail while a run is owed + // (`_modified`), and runEffect trims it once the run applies. Measured + // at 23,353 post-change. expect(minifiedBytes).toBeLessThan(23_400); }); diff --git a/packages/web/test/conditional-jsx-held-branch-3438.spec.tsx b/packages/web/test/conditional-jsx-held-branch-3438.spec.tsx new file mode 100644 index 000000000..f6885ca01 --- /dev/null +++ b/packages/web/test/conditional-jsx-held-branch-3438.spec.tsx @@ -0,0 +1,99 @@ +/** + * @jsxImportSource @solidjs/web + * @vitest-environment jsdom + */ + +import { describe, expect, test } from "vitest"; +import { createMemo, createSignal, flush } from "solid-js"; +import { render } from "@solidjs/web"; + +const delay = (ms: number, value?: T) => + new Promise(r => setTimeout(r, ms, value as T)); + +function snapshot(div: HTMLElement) { + return Array.from(div.querySelectorAll("p")) + .map(p => p.textContent) + .join(" | "); +} + +// Exact port of the issue's playground (https://s.olid.uk/id/LTuZMI4_S8SJuZxMpv8QSQ), +// timings scaled 1000/500 -> 100/50. +function InlineApp() { + const [count, setCount] = createSignal(0); + const [show, setShow] = createSignal(true); + const delayedShow = createMemo(() => delay(100, show())); + return ( + <> + +

Count: {count()}

+

Show: {String(show())}

+

Panel: {show() ? count() : "hidden"}

+

Delayed show: {String(delayedShow())}

+ + ); +} + +// Control: the same conditional wrapped in createMemo (the issue says this avoids it). +function MemoApp() { + const [count, setCount] = createSignal(0); + const [show, setShow] = createSignal(true); + const delayedShow = createMemo(() => delay(100, show())); + const panel = createMemo(() => (show() ? count() : "hidden")); + return ( + <> + +

Count: {count()}

+

Show: {String(show())}

+

Panel: {panel()}

+

Delayed show: {String(delayedShow())}

+ + ); +} + +async function run(App: () => any, label: string) { + const div = document.createElement("div"); + document.body.appendChild(div); + const dispose = render(() => , div); + await delay(150); + flush(); + expect(snapshot(div)).toBe("Count: 0 | Show: true | Panel: 0 | Delayed show: true"); + + div.querySelector("button")!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + const log: string[] = []; + for (let t = 0; t <= 140; t += 20) { + flush(); + log.push(`t=${t}: ${snapshot(div)}`); + await delay(20); + } + process.stderr.write(`ISSUE-3438 ${label}\n${log.join("\n")}\n`); + dispose(); + div.remove(); + + expect(log.at(-1)).toBe("t=140: Count: 1 | Show: false | Panel: hidden | Delayed show: false"); + // While Show still reads true, Panel must agree with Count. + for (const line of log) { + const m = /Count: (\d) \| Show: true \| Panel: (\d)/.exec(line); + if (m) expect(m[2], line).toBe(m[1]); + } +} + +describe("conditional JSX during a held branch change (#3438)", () => { + test("inline `show() ? count() : 'hidden'`", () => run(InlineApp, "inline")); + test("createMemo-wrapped conditional (control)", () => run(MemoApp, "memo")); +}); diff --git a/scripts/size/.size-limit.js b/scripts/size/.size-limit.js index 7dd2a1ff9..2fc4805d8 100644 --- a/scripts/size/.size-limit.js +++ b/scripts/size/.size-limit.js @@ -439,7 +439,11 @@ module.exports = [ // Held children (#3404, 2026-09-13): 15.55 -> 15.60 KB, measured at // 15552 B on the merge with `next` — CONFIG_HELD_CHILDREN set/cleared // around recompute and commitPendingNode. - limit: "15.60 KB", + // Effect arm of A30 (#3438, 2026-09-14): 15.60 -> 15.65 KB, measured at + // 15636 B against `next`'s 15580 (+56 brotli on +35 B minified — the + // `_modified` gate on recompute's trim and runEffect's trim); see the + // core floor note. + limit: "15.65 KB", modifyEsbuildConfig }, { @@ -742,7 +746,10 @@ module.exports = [ // (+28); see the core floor note. // #3426/#3427/#3407 (2026-09-14): 18.80 -> 18.85 KB, measured at 18811 B // against `next`'s 18780 (+31); see the core floor note. - limit: "18.85 KB", + // Effect arm of A30 (#3438, 2026-09-14): 18.85 -> 18.90 KB, measured at + // 18880 B against `next`'s 18816 (+64 brotli on +35 B minified); see the + // core floor note. + limit: "18.90 KB", modifyEsbuildConfig }, { @@ -971,7 +978,9 @@ module.exports = [ // Held-input rules (#3408, #3410; 2026-09-14, on top of #3434): 14.20 -> // 14.25 KB, measured at 14245 B against `next`'s 14169 (+76) — `enterStagedRead` on read()'s value // selections and the deferred dependency trim; see the core floor note. - limit: "14.25 KB", + // Effect arm of A30 (#3438, 2026-09-14): 14.25 -> 14.30 KB, measured at + // 14251 B against `next`'s 14245 (+6); see the core floor note. + limit: "14.30 KB", modifyEsbuildConfig }, {