diff --git a/.changeset/fix-4544-own-probe.md b/.changeset/fix-4544-own-probe.md new file mode 100644 index 000000000..ffd80fb76 --- /dev/null +++ b/.changeset/fix-4544-own-probe.md @@ -0,0 +1,5 @@ +--- +"@solidjs/signals": patch +--- + +A memo holding a transition's staging keeps that staging when its own pass probes, so the branch the hold opened does not wait on the signed-out world (#4544). diff --git a/.changeset/fix-4544-own-staging.md b/.changeset/fix-4544-own-staging.md new file mode 100644 index 000000000..637c77c29 --- /dev/null +++ b/.changeset/fix-4544-own-staging.md @@ -0,0 +1,5 @@ +--- +"@solidjs/signals": patch +--- + +A memo that already holds a transition's staging stays that transition's work when an inherited verdict posture would have pulled it out (#4544). diff --git a/packages/signals/docs/RULES-DERIVED.md b/packages/signals/docs/RULES-DERIVED.md index 77a090017..f1d727368 100644 --- a/packages/signals/docs/RULES-DERIVED.md +++ b/packages/signals/docs/RULES-DERIVED.md @@ -206,7 +206,7 @@ An armed boundary catches a lane the same way it catches a transition (D-12, D-1 What is rendered from a display-ahead read reveals together with it: an async memo over `latest(x)` holds the `latest` view's reveal until it lands; a combined `isPending(() => [fast(), copy()])` over two async memos, one wrapped in a sync memo, stays `true` until both land, because a memo's value is one shared slot — it computes under its own hold posture, never under the posture of whichever reader pulled it. A node whose fetch a hold is waiting on is that hold's work, not a verdict reader: a pass that re-runs it while its fetch is in flight reads plainly — following the change it is fetching for — even if an earlier pass of it called `isPending` or `latest` (#3884). A memo's posture is what it reads: one that reads a verdict reader's value is a derivation of a display-ahead read, and a verdict reader itself — `createMemo(() => [held(), V()])` with `V = createMemo(() => isPending(x) && latest(e))` shows `V`'s verdict now, beside the screen's `held()`, as the inline probe and a render effect reading both do (#4050: a Provider's children memo over a router shell). **The probe decides the pass's posture in either read order (clarified 2026-10-28):** a pass that probes is a verdict reader whether the probe came before or after a plain read of the same held node — `[x(), isPending(x)]` and `[isPending(x), x()]` both show `[committed, true]` now and re-derive at the landing (A10) — unless the pass is the hold's own work: born in its world (D-27, #4341) or re-run by the write it holds (#3884 above). Those two read plainly; a probe does not rescue them. A pass that read two holds' display-ahead values is one reveal unit of both (A15 lanes corollary, #3335): it waits for either lane's flight, never showing one hold's `latest()` beside a half-computed derivation of the other's. -**A verdict posture outlives the pass that asked (ruled 2026-10-28):** a pass that asked (`isPending`, `latest`) is the hold's verdict reader on every later run of its node until one completes having asked nothing — that run still reads as a verdict reader; the next is an ordinary derivation. On each such run its plain read of a node the hold holds is the screen's (a frame reader, A10; re-derived at the landing as A15's stale reader) and it joins nothing, whoever dirtied it — a sync write, a boundary redraw, a landing. The hold's own work is not in this: a node whose fetch the hold waits on, or a pass born in its world, reads plainly whatever it asked before (the #3884 clause and D-27 #4341 above, unchanged). A boundary's display-ahead swap is this posture handed on: an `on` pass that read `latest`/`isPending` is a verdict reader, and the fallback swap it asks for runs as that reader's work — the output's pass reads nothing of the held frame (a pending tree is linked, not read) and its answer, the fallback, is shown now beside the held frame and stays the screen until the landing, instead of being staged with the write's frame as a plain `on`'s swap is (D-15): `[A] → [A + spinner] → [B + spinner] → [B + comments]`, never `[A] → [B + spinner]`. Only the `on` pass's asking counts: `latest()` inside the fallback is the fallback's own display-ahead, not the swap's — a plain `on` over an action-held input lands the swap with the frame whatever the fallback reads (#4337). Today two memories carry this — `CONFIG_VERDICT` left on the node by the pass that probed, and the node's listing in the hold's verdict lane (`fallback`'s `b._lane`, `recompute`'s head) — and the rule is the posture, not the membership. Pins: `tests/loading-on-rearm-3540.test.ts`, `tests/loading-on-frame-following-3540.test.ts` §4–5, `tests/loading-on-latest-fallback-4337.test.ts`. +**A verdict posture outlives the pass that asked (ruled 2026-10-28):** a pass that asked (`isPending`, `latest`) is the hold's verdict reader on every later run of its node until one completes having asked nothing — that run still reads as a verdict reader; the next is an ordinary derivation. On each such run its plain read of a node the hold holds is the screen's (a frame reader, A10; re-derived at the landing as A15's stale reader) and it joins nothing, whoever dirtied it — a sync write, a boundary redraw, a landing. The hold's own work is not in this: a node whose fetch the hold waits on, or a pass born in its world, reads plainly whatever it asked before (the #3884 clause and D-27 #4341 above, unchanged). A boundary's display-ahead swap is this posture handed on: an `on` pass that read `latest`/`isPending` is a verdict reader, and the fallback swap it asks for runs as that reader's work — the output's pass reads nothing of the held frame (a pending tree is linked, not read) and its answer, the fallback, is shown now beside the held frame and stays the screen until the landing, instead of being staged with the write's frame as a plain `on`'s swap is (D-15): `[A] → [A + spinner] → [B + spinner] → [B + comments]`, never `[A] → [B + spinner]`. Only the `on` pass's asking counts: `latest()` inside the fallback is the fallback's own display-ahead, not the swap's — a plain `on` over an action-held input lands the swap with the frame whatever the fallback reads (#4337). Today two memories carry this — `CONFIG_VERDICT` left on the node by the pass that probed, and the node's listing in the hold's verdict lane (`fallback`'s `b._lane`, `recompute`'s head) — and the rule is the posture, not the membership. Pins: `tests/loading-on-rearm-3540.test.ts`, `tests/loading-on-frame-following-3540.test.ts` §4–5, `tests/loading-on-latest-fallback-4337.test.ts`. A node holding its own staging of a hold is that hold's work, and an inherited posture does not pull it out (2026-11-01, #4544). **Collapses:** [A31](./SPEC-ASYNC-SEMANTICS.md#a31-a-memo-computes-under-its-own-lane-posture-never-its-pullers) (#3442), §20.1 question 3 ("Yeah definitely it is held. Consistent with any optimistic update"), OL-R24/R25/R34, recovery 6.2. **Status:** standing. A memo over a verdict memo: `tests/verdict-memo-under-held-memo-4050.test.ts`, `packages/web/test/held-shell-pending-feedback-4050.spec.tsx` (U-10). **Fixed 2026-11-01:** the verdict shortcut in `propagateStatus` (a probed source settling re-derives its verdict reader) now clears the settled source from the reader's pending sources — left in place, an errored probe target stranded a reader re-parked on a second source at that source's landing; `tests/verdict-shortcut-pending-source-liveness.test.ts`. diff --git a/packages/signals/docs/RULES-INDEX.md b/packages/signals/docs/RULES-INDEX.md index c0d096a34..4db0772c9 100644 --- a/packages/signals/docs/RULES-INDEX.md +++ b/packages/signals/docs/RULES-INDEX.md @@ -54,7 +54,7 @@ Status legend: **live** stated and standing · **ruled** carries an explicit rul | A7 | amended | `docs/SPEC-ASYNC-SEMANTICS.md:167` | seat.ts×1 truth.ts×1 verdict.ts×1 | fuzz-findings-l2.test.ts×1 latest-uninitialized-over-latest-4362.test.ts×2 spec-async-semantics.test.ts×2 visibility-oracle-store.states.ts×1 visibility-oracle.states.ts×1 visibility-oracle.test.ts×1 | [ruled, amended in place] Resolved async never reads `[false, undefined]` — After an async memo resolves, `[isPending(x), latest(x)]` is `[false, resolvedValue]` — never `[false, undefined]`. **Amende… | | A8 | amended | `docs/SPEC-ASYNC-SEMANTICS.md:175` | — | create-deferred-3710.test.ts×1 createMemo.test.ts×1 visibility-oracle-store.states.ts×1 visibility-oracle.states.ts×2 | [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:183` | async.ts×1 projection.ts×1 target.ts×1 | spec-async-semantics.test.ts×3 visibility-oracle-store.states.ts×4 visibility-oracle-store.test.ts×1 | [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… | -| A10 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:191` | async.ts×1 core.ts×3 lanes.ts×1 scheduler.ts×1 seat.ts×1 verdict.ts×3 | boundary-not-born-held-3540.test.ts×1 createMemo.test.ts×1 error-staged-recovery.test.ts×1 fuzz-findings-l2.test.ts×1 ispending-memo-unstamped-hold-3457.test.ts×2 latest-isPending-consistency.test.ts×3 latest-uninitialized-over-latest-4362.test.ts×1 verdict-contract.test.ts×2 verdict-memo-flight-no-reask.test.ts×1 | [ruled] `[isPending(x), x()]` is atomic within one scope — `[isPending(x), x()]` read in one scope is atomic: a reader that observed the fresh value must not see `pending === true` for it. | +| A10 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:191` | async.ts×1 core.ts×5 lanes.ts×1 scheduler.ts×1 seat.ts×1 verdict.ts×6 | boundary-not-born-held-3540.test.ts×1 createMemo.test.ts×1 error-staged-recovery.test.ts×1 fuzz-findings-l2.test.ts×1 ispending-memo-unstamped-hold-3457.test.ts×2 latest-isPending-consistency.test.ts×3 latest-uninitialized-over-latest-4362.test.ts×1 verdict-contract.test.ts×2 verdict-memo-flight-no-reask.test.ts×1 verdict-memo-held-branch-4544.test.ts×1 | [ruled] `[isPending(x), x()]` is atomic within one scope — `[isPending(x), x()]` read in one scope is atomic: a reader that observed the fresh value must not see `pending === true` for it. | | A11 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:103` | — | latest-isPending-consistency.test.ts×1 loading-fallback-in-flush-3540.test.ts×2 visibility-oracle-store.states.ts×2 visibility-oracle.states.ts×1 | [ruled] Sync derivations of held sources are visible through `latest()`/`isPending()` — Sync derivations of transition-held sources are visible through `latest()`/`isPending()` (held sync recompute is… | | A12 | amended | `docs/SPEC-ASYNC-SEMANTICS.md:199` | — | createOptimistic.test.ts×2 spec-async-semantics.test.ts×1 | [ruled, amended in place] Resting optimistic nodes report pending like a plain memo — A resting optimistic node reports pending via exactly the causes a plain async memo does (A19) — a reverting optim… | | A13 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:207` | — | spec-async-semantics.test.ts×7 | [ruled 2026-07-06 (promoted from B1)] Resting optimistic ≡ plain async memo at every checkpoint — (was B1) A resting optimistic node (no active override) is observationally identical to a plain async … | @@ -75,7 +75,7 @@ Status legend: **live** stated and standing · **ruled** carries an explicit rul | A28 | ruled, mechanism landed | `docs/SPEC-ASYNC-SEMANTICS.md:95` | constants.ts×1 core.ts×12 lanes.ts×2 scheduler.ts×3 seat.ts×3 types.ts×1 verdict.ts×2 store.ts×5 | continuation-sibling-reader-4146.test.ts×1 createOptimistic.test.ts×5 fuzz-findings-l2.test.ts×6 held-derivation-not-a-proposal-3612.test.ts×1 issue-3800-repro.test.ts×1 latest-held-till-flush.test.ts×1 latest-memo-inline-parity.test.ts×1 posture-store-parity.test.ts×5 question-scoped-pending.test.ts×3 snapshot-derived-store-rows.test.ts×1 createOptimisticStore.test.ts×10 optimistic-draft-visibility-3665.test.ts×5 optimistic-list-mutation-matrix.harness.ts×1 optimistic-list-mutation-matrix.test.ts×2 shallow.test.ts×1 woken-transaction-adopts-staged-bump.test.ts×1 treeshake.test.ts×2 verdict-contract.test.ts×1 visibility-oracle-store.states.ts×8 visibility-oracle.states.ts×8 | [ruled, mechanism landed 2026-09-15] A write becomes visible at flush — to every channel — (**ruled 2026-09-08**; supersedes the #2922 mid-tick pull) **A write becomes visible at flush — to every chan… | | A29 | amended | `docs/SPEC-ASYNC-SEMANTICS.md:119` | boundaries.ts×1 action.ts×1 constants.ts×1 core.ts×14 effect.ts×1 lanes.ts×1 scheduler.ts×4 signals.ts×1 store.ts×1 | adoption-unchanged-key-read-3706.test.ts×9 body-end-supersession-visibility.test.ts×1 born-held.test.ts×3 boundary-not-born-held-3540.test.ts×4 createProjection.draft-lifetime-3585.test.ts×1 direct-commit-readers-posture.test.ts×1 fallback-born-creator-4428.test.ts×1 fuzz-findings-l2.test.ts×6 held-branch-verdict-reader-4010.test.ts×1 held-conditional-memo.test.ts×1 held-frame-dependencies.test.ts×2 held-frame-verdict-parity.test.ts×1 held-truth-lane-only.test.ts×3 l2-contract.test.ts×1 latest-held-till-flush.test.ts×2 loading-fallback-in-flush-3540.test.ts×6 loading-on-frame-following-3540.test.ts×1 mount-over-foreign-hold-3761.test.ts×1 optimistic-mount-nested-memo-3835.test.ts×1 optimistic-read-lane-not-transaction-3698.test.ts×3 posture-born-held-and-observation.test.ts×1 posture-store-parity.test.ts×6 derived-presence-async-3726.test.ts×3 optimistic-untracked-reads-f3-f5.test.ts×1 store-unchanged-read-independent-write-3688.test.ts×1 tick-scoped-pass-transaction.test.ts×2 treeshake.test.ts×3 verdict-born-into-other-flush-4426.test.ts×1 verdict-mount-first-pass-3851.test.ts×2 verdict-mount-loading-flush-loop.test.ts×2 visibility-oracle-store.states.ts×5 visibility-oracle-store.test.ts×1 visibility-oracle.states.ts×9 visibility-oracle.test.ts×1 write-proposals-3494.test.ts×1 | [ruled, amended in place 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 … | | A30 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:259` | async.ts×3 attribution.ts×1 constants.ts×1 core.ts×1 effect.ts×1 lanes.ts×1 scheduler.ts×5 projection.ts×1 | async-landing-deps-3461.test.ts×3 fuzz-findings-l2.test.ts×3 held-conditional-effect.test.ts×1 held-conditional-memo.test.ts×1 held-frame-dependencies.test.ts×2 ispending-in-boundary-on-3528.test.ts×1 lane-frame-deferred-run-3662.test.ts×1 lane-frame-held-lane-3662.test.ts×1 posture-born-held-and-observation.test.ts×1 projection-peek-loop-4342.test.ts×1 treeshake.test.ts×2 write-proposals-3494.test.ts×2 zombie-rerun-after-commit-3546.test.ts×2 | [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… | -| A31 | live | `docs/SPEC-ASYNC-SEMANTICS.md:133` | boundaries.ts×1 core.ts×1 lanes.ts×1 verdict.ts×2 | fuzz-findings-l2.test.ts×6 ispending-combined-atomic-3442.test.ts×1 latest-pulled-pass-ispending.test.ts×1 probe-pulled-pass-4429.test.ts×1 unowned-memo-ispending-reader-4366.test.ts×1 | [live 2026-09-14 (#3442)] A memo computes under its own lane posture, never its puller's — A memo's value is one shared slot every reader sees, so its pass runs under the lane posture the memo itself … | +| A31 | live | `docs/SPEC-ASYNC-SEMANTICS.md:133` | boundaries.ts×1 core.ts×1 lanes.ts×1 verdict.ts×2 | fuzz-findings-l2.test.ts×6 ispending-combined-atomic-3442.test.ts×1 latest-pulled-pass-ispending.test.ts×1 probe-pulled-pass-4429.test.ts×1 unowned-memo-ispending-reader-4366.test.ts×1 verdict-memo-held-branch-4544.test.ts×1 | [live 2026-09-14 (#3442)] A memo computes under its own lane posture, never its puller's — A memo's value is one shared slot every reader sees, so its pass runs under the lane posture the memo itself … | | A32 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:141` | core.ts×4 lanes.ts×1 seat.ts×1 store.ts×1 | visibility-oracle-store.states.ts×8 visibility-oracle-store.test.ts×1 visibility-oracle.states.ts×9 visibility-oracle.test.ts×1 | [ruled 2026-09-14] Children-forbidden readers see the frame, not the graph — `createTrackedEffect` and `onSettled` callbacks are effect-phase code that runs after the frame is decided. They read the f… | | A33 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:275` | boundaries.ts×1 scheduler.ts×2 | async-chain-supersession.test.ts×2 boundary-not-born-held-3540.test.ts×2 create-deferred-3710.test.ts×1 fuzz-findings-l2.test.ts×4 ispending-in-boundary-on-3528.test.ts×2 loading-reset-collects-forwarded-3459.test.ts×3 | [ruled 2026-09-12 (#3375)] A fallback-caught flight holds no transaction; a Loading reset moves the hold onto the boundary — A `` boundary showing its fallback is the display of everything un… | | A34 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:285` | constants.ts×1 core.ts×6 lanes.ts×2 scheduler.ts×2 store.ts×7 | a34-writes-then-derivations.test.ts×2 continuation-sibling-reader-4146.test.ts×1 createMemo.test.ts×1 derived-write-then-derivation-3733.test.ts×1 finalize-reentry.test.ts×2 fuzz-findings-l2.test.ts×5 held-derivation-not-a-proposal-3612.test.ts×6 optimistic-guess-over-held-slot.test.ts×2 optimistic-list-mutation-matrix.test.ts×1 unchanged-presence-no-hold-3743.test.ts×3 woken-transaction-adopts-staged-bump.test.ts×1 transition-corpse-revival.test.ts×1 treeshake.test.ts×2 visibility-oracle.states.ts×2 write-proposals-3494.test.ts×5 | [ruled 2026-09-16 (#3494)] A write is a proposal: one on a held node entangles its tick; one that nets to the committed value is none — A write proposes a value for a node. **Held, both are suggestion… | diff --git a/packages/signals/src/core/constants.ts b/packages/signals/src/core/constants.ts index 5eb8d994a..9be3aa0a2 100644 --- a/packages/signals/src/core/constants.ts +++ b/packages/signals/src/core/constants.ts @@ -73,6 +73,10 @@ export const REACTIVE_PROBE_UNANSWERED = 1 << 19; * did not is D-25's hold work while its flight is up (verdict.ts * `holdWork`). Per pass. */ export const REACTIVE_WINDOW = 1 << 21; +/** #4544 — this pass already holds its transition's staging. Set once, after + * the head wipe, from `holdStaging` at `recompute`'s head; `frameRead` tests + * it and does not walk deps on the read. Per pass: the tail wipe clears it. */ +export const REACTIVE_HOLD_STAGING = 1 << 20; /** An observation-lifecycle node found unobserved by a settle (#2934) while * a pass of it was already queued — a source's landing re-enqueued the * `_blocked` reader it walks (`settlePendingSource`). The release waits for diff --git a/packages/signals/src/core/core.ts b/packages/signals/src/core/core.ts index 70e842bf6..1cc2932ec 100644 --- a/packages/signals/src/core/core.ts +++ b/packages/signals/src/core/core.ts @@ -52,6 +52,7 @@ import { REACTIVE_IN_HEAP, REACTIVE_LANE_DIRTY, REACTIVE_LANE_READ, + REACTIVE_HOLD_STAGING, REACTIVE_PROBE_UNANSWERED, REACTIVE_SCREEN_READ, REACTIVE_IN_HEAP_HEIGHT, @@ -167,6 +168,7 @@ import { laneStageHook, laneSeamsHook, laneServedHook, + holdStagingHook, holdWorkHook, laneWriteHook, observeFlightHook, @@ -360,6 +362,10 @@ export function recompute(el: Computed, create: boolean = false): void { bumpNotifyEpoch(); const isEffect = (el as any)._type; let released: Computed[] | undefined; + // #4544: the head's sync-write deps, installed for the body after the flag + // wipe. A probe in the body does not recompute the list. + let stagedHold = false; + let syncDeps: object[] | null = null; // Lanes (plan sec. 28): the seat of a pass is its node's. A node carrying a lane's // derived value runs as the lane's whoever dirtied it — a sync write, a // boundary reset, a frame rerun — so its children and its result are the @@ -462,6 +468,13 @@ export function recompute(el: Computed, create: boolean = false): void { // A). // A boundary's structure (CONFIG_FRAME) is the frame like a render // effect: it publishes mainline outside its transaction's flush (#4283). + // #4544: a verdict memo's sync writes from an earlier flush() call. + // Computed even when this node was dropped from the hold — the read + // still has to agree with the branch that write opened. + if (observeFlightHook && el._config & CONFIG_VERDICT && !(el._config & (EFFECT_RENDER << 23))) { + syncDeps = holdStagingHook?.(el) ?? null; + if (syncDeps !== null && syncDeps.length !== 0) stagedHold = true; + } if (el._config & CONFIG_HELD) { const tx = txOf(el); // Held by a blocked lane: the frame joins nothing — a lane never holds @@ -474,30 +487,38 @@ export function recompute(el: Computed, create: boolean = false): void { // A verdict reader born into `tx` (`bornInto`: never committed) is the // hold's own work, whatever it asked before (D-25): it joins like any // held pass, so what it mounts is the hold's (D-13, #4467). + // #4544: T's own staging stays T's work. A verdict posture does not pull + // it out — inherited, or this pass's own probe (D-25: the hold's work + // reads plainly; a probe does not rescue it). Computed once; the bit + // below is what `frameRead` tests. if (tx._lane || el._config & CONFIG_LAGS) { - } else if ( - !(el._config & (CONFIG_VERDICT | (EFFECT_RENDER << 23))) || - (observeFlightHook && !(el._config & (EFFECT_RENDER << 23)) && bornInto(el, tx)) - ) - joinPassTx(tx, freshHook?.(el, el._statusFlags & STATUS_UNINITIALIZED)); - else if ( - tx !== flushTransaction && - !(el._statusFlags & STATUS_UNINITIALIZED || el._config & CONFIG_HELD_FRAME) - ) { - // Published mainline, it is not held: the frame this pass builds is - // replaced on the spot (#3404) — this flush's commit retires the - // committed frame the held pass had parked (`commitPendingNode`), - // after the new frame's first runs, and the held pass's own - // children die below (`disposeChildren`). The landing re-derives a - // fresh held frame for the reveal (`_reruns`). - // Not held any more, whichever frame reader it is: its readers read - // the value this pass publishes — those that read the staging it - // supersedes re-derive, even when it publishes the committed value - // back (no change to notify them by): their held runs carry the - // superseded frame, and the landing would apply them (#4378). - if (el._pendingValue !== NOT_PENDING) insertSubs(el); - el._pendingValue = NOT_PENDING; - el._config &= ~CONFIG_HELD; + } else { + const staging = syncDeps; + if ( + !(el._config & (CONFIG_VERDICT | (EFFECT_RENDER << 23))) || + (observeFlightHook && !(el._config & (EFFECT_RENDER << 23)) && bornInto(el, tx)) || + staging !== null + ) + joinPassTx(tx, freshHook?.(el, el._statusFlags & STATUS_UNINITIALIZED)); + else if ( + tx !== flushTransaction && + !(el._statusFlags & STATUS_UNINITIALIZED || el._config & CONFIG_HELD_FRAME) + ) { + // Published mainline, it is not held: the frame this pass builds is + // replaced on the spot (#3404) — this flush's commit retires the + // committed frame the held pass had parked (`commitPendingNode`), + // after the new frame's first runs, and the held pass's own + // children die below (`disposeChildren`). The landing re-derives a + // fresh held frame for the reveal (`_reruns`). + // Not held any more, whichever frame reader it is: its readers read + // the value this pass publishes — those that read the staging it + // supersedes re-derive, even when it publishes the committed value + // back (no change to notify them by): their held runs carry the + // superseded frame, and the landing would apply them (#4378). + if (el._pendingValue !== NOT_PENDING) insertSubs(el); + el._pendingValue = NOT_PENDING; + el._config &= ~CONFIG_HELD; + } } } // The previous pass's frame. If that pass is still awaiting its commit @@ -577,6 +598,7 @@ export function recompute(el: Computed, create: boolean = false): void { // (REACTIVE_RELEASE_AFTER): the release is this pass's tail's (#4474). const releaseAfter = el._flags & REACTIVE_RELEASE_AFTER; el._flags = REACTIVE_RECOMPUTING_DEPS | (el._flags & (REACTIVE_ZOMBIE | REACTIVE_REASK)); + if (stagedHold) el._flags |= REACTIVE_HOLD_STAGING; el._time = clock; // The pass's previous value: its staging, else a lane node's lane value // when it has one (lanes.ts), else the committed one. @@ -591,6 +613,16 @@ export function recompute(el: Computed, create: boolean = false): void { // L2: did this pass read the future (REACTIVE_JOINED, set by read) — or, // a lane's mount, a staging of this flush (#3851, #4166)? let joined = false; + // #4544: nodes this pass probes, while it holds the transition's staging. + // Nested passes save the outer list. Only a staging pass allocates one. + let prevHoldProbes: object[] | null = null; + let prevStagingDeps: object[] | null = null; + if (stagedHold) { + prevHoldProbes = holdProbes; + holdProbes = null; + prevStagingDeps = stagingDeps; + stagingDeps = syncDeps; + } let prevTracking = tracking; let prevStrictRead: string | false = false; if (__DEV__) { @@ -695,6 +727,10 @@ export function recompute(el: Computed, create: boolean = false): void { settlePendingSource(el, source); } } finally { + if (stagedHold) { + holdProbes = prevHoldProbes; + stagingDeps = prevStagingDeps; + } tracking = prevTracking; if (laneStageHook) laneRoot = prevMount; if (__DEV__) { @@ -1323,6 +1359,18 @@ export function joinPassAt(c: Computed, t: Transaction): void { * the parent's flights is still up — needs none of this: the correction * dissolves the lane, and the refetching pass is the parent's own, reading * the truth as a member (`joinPass`). */ +/** Nodes the running pass probed while `REACTIVE_HOLD_STAGING` is set. + * `frameRead` serves those the screen (A10) and the pass's other held deps + * the staging. Null outside such a pass. */ +let holdProbes: object[] | null = null; +/** Sync writes this pass holds. Plain reads of them see the staging; a dep + * a flight is waiting on is not in the list and stays the screen. */ +let stagingDeps: object[] | null = null; +export function noteHoldProbe(el: Signal | Computed): void { + if (holdProbes === null) holdProbes = [el]; + else holdProbes.push(el); +} + function frameRead(c: Computed, el: Signal | Computed): boolean { // Frame readers: a render effect, a verdict reader (CONFIG_VERDICT — it // reads the screen like one: `[isPending(x), x()]` never pairs the fresh @@ -1339,7 +1387,31 @@ function frameRead(c: Computed, el: Signal | Computed): boolean { // under a hold still enters it: the content memo it reads is born held, // with no committed value to serve. // (Verdict readers and lanes exist only with their engines installed.) - const verdict = observeFlightHook && c._config & CONFIG_VERDICT; + // #4544: a memo holding T's staging is not served T's committed value. + // The bit was set once at this pass's head. A frame still reads the + // screen. Both checks sit behind `observeFlightHook` so a build without + // the verdict engine folds them away. + const asked = observeFlightHook && c._config & CONFIG_VERDICT; + // The bit is the pass. A node the pass probed still reads as the screen + // (A10): `[isPending(x), x()]` stays the committed `x`, even when another + // dep of this pass is the hold's staging. + let staging = !!(asked && c._flags & REACTIVE_HOLD_STAGING && stagingDeps !== null); + if (staging) { + staging = false; + for (let i = 0; i < stagingDeps!.length; i++) + if (stagingDeps![i] === el) { + staging = true; + break; + } + } + if (staging && holdProbes !== null) + for (let i = 0; i < holdProbes.length; i++) + if (holdProbes[i] === el) { + staging = false; + break; + } + if (staging && !(c._config & (EFFECT_RENDER << 23))) return false; + const verdict = !staging && asked; const lane = laneStageHook && passLane !== null; if ( (!lane || @@ -2711,6 +2783,9 @@ export function setSignal(el: Signal | Computed, v: T | ((prev: T) => T isSame(el, el._value, v) ? el._value : v; + // A held node written inside this flush: the pending value is this flush's, + // not a write parked on an earlier one (#4365 vs #4544). The extension + // already exists (`CONFIG_HELD`). // A28 arm, gated on the load the write already pays for (`context`); the // arm itself is a cold helper so setSignal stays within every setter's // inlining budget, ~300 B bytecode. diff --git a/packages/signals/src/core/scheduler.ts b/packages/signals/src/core/scheduler.ts index 916548696..b0c788b03 100644 --- a/packages/signals/src/core/scheduler.ts +++ b/packages/signals/src/core/scheduler.ts @@ -186,14 +186,20 @@ export let holdWorkHook: ((n: Computed, source: Computed) => unknown) /** D-41's staged-error mark (verdict.ts `stageMark`): the settle walk's * reached node `n` carries staged error source `el`'s staging. */ export let stageMarkHook: ((n: Computed, el: Computed) => void) | null = null; +/** #4544 (verdict.ts). The pass's sync joined deps, or an empty list when it + * only holds a flight's source (join, but those reads stay the screen). + * Null: not this hold's staging. Null without the verdict engine. */ +export let holdStagingHook: ((el: Computed) => object[] | null) | null = null; export function installVerdictHooks( observeFlight: NonNullable, holdWork: NonNullable, - stageMark: NonNullable + stageMark: NonNullable, + holdStaging: NonNullable ): void { observeFlightHook = observeFlight; holdWorkHook = holdWork; stageMarkHook = stageMark; + holdStagingHook = holdStaging; } /** External-source bridge (wired by enableExternalSource(); undefined while * no config is active — including after _resetExternalSourceConfig()). */ @@ -303,6 +309,10 @@ export interface Transaction { * transaction's flush the lane's work reads its staging (core.ts * `frameRead`, #4536). Unset until then; not `_`-prefixed, as `folded`. */ over?: Transaction; + /** The `flush()` call that opened it. A sync write parked then is that + * hold's on a later call; one parked in the call still running is the + * screen (#4544, #4365). Unset name is unprefixed, as `folded`. */ + gen: number; /** `affects()` marks declared in it (affects.ts): released at the landing. * Not nodes — a mark holds nothing (`blocked` never sees one). */ _marks: Signal[] | null; @@ -327,7 +337,8 @@ export function newTransaction(lane: boolean, parent: Transaction | null = null) _links: null, _shown: false, _held: false, - _marks: null + _marks: null, + gen: flushGen }; if (!lane) transactions.push(t); return t; @@ -656,6 +667,9 @@ export const dirtyQueue: Heap = { }; export let clock = 0; +/** One per draining `flush()` call. Transactions opened in that call share + * it (`Transaction.gen`). */ +export let flushGen = 0; let scheduled = false; let halted = false; let haltNotified = false; @@ -1609,6 +1623,7 @@ export function flush(fn?: () => T): T | void { // exactly when `flushEnd` below will. Outside every try (see the rule in // attribution-hooks.ts). if (__OBSERVE__ && attrHooks !== null && scheduled) attrHooks.flushStart(); + if (scheduled) flushGen++; while (scheduled) { if (__DEV__ && ++count === 1e5) { rethrowCleanupError(true); diff --git a/packages/signals/src/core/verdict.ts b/packages/signals/src/core/verdict.ts index 93100bf38..06299850f 100644 --- a/packages/signals/src/core/verdict.ts +++ b/packages/signals/src/core/verdict.ts @@ -32,6 +32,7 @@ import { REACTIVE_PROBED, REACTIVE_REASK, REACTIVE_SNAPSHOT_STALE, + REACTIVE_HOLD_STAGING, REACTIVE_PROBE_UNANSWERED, REACTIVE_STAGED_READ, REACTIVE_WINDOW, @@ -46,6 +47,7 @@ import { creatorReads, joinPass, markLateLinker, + noteHoldProbe, prepareComputed, pullComputed, setVerdict, @@ -64,6 +66,7 @@ import { link } from "./graph.js"; import { display, laneValueOf, verdictLane } from "./lanes.js"; import { enqueueSub } from "./heap.js"; import { + flushGen, flushTransaction, globalQueue, GlobalQueue, @@ -429,6 +432,11 @@ function verdictValue(el: Signal | Computed, c: Computed | null): const owner = el as Computed; const owned = !!c && !(c._config & CONFIG_CHILDREN_FORBIDDEN); const tracked = owned && tracking; + // #4544: a pass holding the transition's staging still reads a node it + // probes as the screen (A10). The plain read after the window asks + // `frameRead`, which would otherwise serve the staging. + if ((probing || latestActive) && c !== null && c._flags & REACTIVE_HOLD_STAGING) + noteHoldProbe(el); // A31: the pulled memo's pass is its own, not the probe's — it reads // outside the window, and the probe's read below answers for what the pass // produced. So is a lazy or dormant memo's wake (an unowned memo with no @@ -723,7 +731,66 @@ function stageMark(node: Computed, el: Computed): void { } } -installVerdictHooks(observeFlight, holdWork, stageMark); +/** A held dep whose new value a flight in `t` is waiting on (`A` reads `s` + * and is pending). A plain read of it is the screen (#4365). A sync write + * that only joined `t` is not. */ +function feedsFlight( + n: Signal | Computed, + t: ReturnType, + self: Computed +): boolean { + if (n._statusFlags & STATUS_PENDING && txOf(n) === t) return true; + for (let s = n._subs; s !== null; s = s._nextSub) { + const sub = s._sub; + if ( + sub !== self && + sub._statusFlags & STATUS_PENDING && + sub._config & CONFIG_HELD && + txOf(sub) === t + ) + return true; + } + return false; +} + +/** #4544: deps whose pending value is this hold's sync write, not a + * park-seam `CONFIG_HELD` on a published verdict and not the source of a + * flight the hold waits on. Called once at the pass's head. An own probe + * does not drop a sync write (D-25); the probed node still reads as the + * screen (`noteHoldProbe`, A10). An unanswered probe drops a flight's source + * (#4365) and nothing else. A dep held by a lane is that lane's. + * + * `null`: not staging. `[]`: join, but every held dep feeds a flight so + * plain reads stay the screen. A non-empty list is the sync writes `frameRead` + * serves as staging. */ +function holdStaging(el: Computed): object[] | null { + const unanswered = (el._flags & REACTIVE_PROBE_UNANSWERED) !== 0; + let sync: object[] | null = null; + let flight = false; + for (let d = el._deps; d !== null; d = d._nextDep) { + const n = d._dep; + if (n._pendingValue === NOT_PENDING || !(n._config & CONFIG_HELD)) continue; + const t = txOf(n); + if (t._lane) continue; + // An async memo's held answer, and a recovery sitting on a shown error, + // stay the screen (A10, D-41). A plain signal the hold parked does not. + if (n._config & CONFIG_ASYNC || n._x?._shownError || n._statusFlags & STATUS_ERROR) { + flight = true; + continue; + } + // Opened by an earlier flush() call: the hold's own sync write, whether + // or not this reader still carries a pending value. Opened by the call + // still running: the screen (#4365). + if (t.gen < flushGen && !feedsFlight(n, t, el)) (sync ??= []).push(n); + else flight = true; + } + if (sync) return sync; + // A flight's source joins only while this reader is already in the hold. + // An unanswered probe does not (#4365). + return flight && !unanswered && el._pendingValue !== NOT_PENDING ? [] : null; +} + +installVerdictHooks(observeFlight, holdWork, stageMark, holdStaging); /** A store read inside an untracked `isPending` probe found a live mark on * a record with no node to carry it (store/affects.ts): not final. */ GlobalQueue._witnessMark = () => { diff --git a/packages/signals/tests/verdict-memo-held-branch-4544.test.ts b/packages/signals/tests/verdict-memo-held-branch-4544.test.ts new file mode 100644 index 000000000..b5ee26f4e --- /dev/null +++ b/packages/signals/tests/verdict-memo-held-branch-4544.test.ts @@ -0,0 +1,202 @@ +/** + * #4544. A held update writes `session` and opens a branch that exists only + * for that write. Inside, `params` reads `session` and `chrome`; `chrome` + * reads an unrelated signal `plan`. + * + * `chrome` computes under the probe in its own body (A31 / D-25): `isPending` + * then `plan()` is the screen beside the probe. `params`, holding the hold's + * staging of `session`, is that hold's work. A probe in its own body does not + * pull that staging out (D-25: the hold's work reads plainly). The probed + * node still reads as the screen (A10). The branch the hold opened therefore + * reads `session` as signed in, returns `rows`, and the hold lands — a + * never-resolving signed-out fetch is not what it waits on. + */ +import { expect, it } from "vitest"; +import { + createLoadingBoundary, + createMemo, + createRenderEffect, + createRoot, + createSignal, + flush, + isPending, + latest, + untrack +} from "../src/index.js"; + +const tick = async () => { + for (let i = 0; i < 8; i++) await Promise.resolve(); + flush(); +}; + +type Probe = "plain read" | "isPending" | "latest" | "inline isPending" | "same-node isPending"; +type Params = { plan: boolean; signedIn: boolean; busy?: boolean }; + +function page(probe: Probe) { + const [session, setSession] = createSignal(undefined); + const [plan, setPlan] = createSignal(false); + const [gate, setGate] = createSignal(0); + let release: () => void = () => {}; + let screen = ""; + const branchSaw: Params[] = []; + const chromeSaw: boolean[] = []; + let signedOutIssued = false; + const dispose = createRoot(dispose => { + const held = createMemo(() => { + const g = gate(); + return g === 0 ? g : new Promise(r => (release = () => r(g))); + }); + const out = createLoadingBoundary( + () => { + const chrome = createMemo(() => { + const v = + probe === "isPending" + ? (isPending(plan), plan()) + : probe === "latest" + ? latest(plan) + : plan(); + chromeSaw.push(v); + return v; + }); + const params = createMemo(() => + probe === "same-node isPending" + ? { plan: plan(), signedIn: session() !== undefined, busy: isPending(session) } + : { + plan: probe === "inline isPending" ? (isPending(plan), plan()) : chrome(), + signedIn: session() !== undefined + } + ); + const signedOutOnly = createMemo( + () => { + signedOutIssued = true; + return new Promise(() => {}); + }, + { lazy: true } + ); + const cond = createMemo(() => session() !== undefined); + const show = createMemo(() => { + if (!cond()) return "signed out"; + return untrack(() => { + let text = ""; + createRenderEffect( + () => { + const p = params(); + branchSaw.push(p); + return p.signedIn ? "rows" : signedOutOnly(); + }, + v => { + text = v as string; + } + ); + return () => text; + }); + }); + return createMemo(() => { + const h = held(); + let s: unknown = show(); + while (typeof s === "function") s = s(); + return `${h} ${s}`; + }); + }, + () => "root" + ); + createRenderEffect( + () => { + let v: unknown = out; + while (typeof v === "function") v = v(); + return v; + }, + v => { + screen = v as string; + } + ); + return dispose; + }); + return { + dispose, + screen: () => screen, + branchSaw, + chromeSaw, + setSession, + setPlan, + setGate, + release: () => release(), + signedOutIssued: () => signedOutIssued + }; +} + +for (const probe of ["plain read", "isPending", "latest"] as Probe[]) + it(`${probe} upstream of a held branch: the branch reads the hold's session and the hold lands`, async () => { + const p = page(probe); + try { + flush(); + expect(p.screen()).toBe("0 signed out"); + p.setGate(1); + p.setSession("acc"); + flush(); + // The branch the hold opens, before the unrelated write. + expect(p.branchSaw.at(-1)).toMatchObject({ plan: false, signedIn: true }); + p.setPlan(true); + flush(); + // `chrome`'s own value follows its probe (D-25: a memo computes under + // its own posture). `latest` and a plain read see `plan`'s write; + // `isPending` then `plan()` is the screen beside the probe. + const chrome = p.chromeSaw.at(-1); + expect(p.branchSaw.at(-1)).toMatchObject({ + plan: probe === "isPending" ? false : true, + signedIn: true + }); + expect(chrome).toBe(probe === "isPending" ? false : true); + p.release(); + await tick(); + await tick(); + expect(p.screen()).toBe("1 rows"); + } finally { + p.dispose(); + } + }); + +it("inline isPending(plan): the branch reads the hold's session and the hold lands without the signed-out fetch", async () => { + const p = page("inline isPending"); + try { + flush(); + expect(p.screen()).toBe("0 signed out"); + p.setGate(1); + p.setSession("acc"); + flush(); + p.setPlan(true); + flush(); + // `isPending(plan)` then `plan()` is the screen. `session` is the hold's + // write, so the branch the hold opened stays signed in and never starts + // the signed-out fetch. + expect(p.branchSaw.at(-1)).toMatchObject({ plan: false, signedIn: true }); + expect(p.signedOutIssued()).toBe(false); + p.release(); + await tick(); + await tick(); + expect(p.screen()).toBe("1 rows"); + } finally { + p.dispose(); + } +}); + +it("same-node isPending(session): the branch reads the hold's session and the hold lands", async () => { + const p = page("same-node isPending"); + try { + flush(); + p.setGate(1); + p.setSession("acc"); + flush(); + p.setPlan(true); + flush(); + // The plain read of `session` is the hold's staging. The branch returns + // `rows` rather than waiting on the signed-out fetch. + expect(p.branchSaw.at(-1)).toMatchObject({ signedIn: true, busy: true }); + p.release(); + await tick(); + await tick(); + expect(p.screen()).toBe("1 rows"); + } finally { + p.dispose(); + } +}); diff --git a/packages/web/test/verdict-memo-held-branch-4544.spec.tsx b/packages/web/test/verdict-memo-held-branch-4544.spec.tsx new file mode 100644 index 000000000..28b7b8a9c --- /dev/null +++ b/packages/web/test/verdict-memo-held-branch-4544.spec.tsx @@ -0,0 +1,107 @@ +/** + * @jsxImportSource @solidjs/web + * @vitest-environment jsdom + */ +// #4544. `chrome` owns the probe of an unrelated signal and computes under +// that probe (A31 / D-25). `params`, holding the hold's staging of `session`, +// is that hold's work: a probe in its own body does not pull that staging +// out. The branch the hold opened reads `session` as signed in and the hold +// lands, without waiting on the signed-out fetch. +import { expect, test } from "vitest"; +import { render } from "@solidjs/web"; +import { createMemo, createSignal, flush, isPending, latest, Loading, Show } from "solid-js"; + +const tick = async () => { + for (let i = 0; i < 8; i++) await Promise.resolve(); + flush(); +}; + +type Probe = "plain read" | "isPending" | "latest" | "inline isPending"; +type Params = { plan: boolean; signedIn: boolean }; + +async function arrive(probe: Probe) { + const [session, setSession] = createSignal(undefined); + const [plan, setPlan] = createSignal(false); + const [gate, setGate] = createSignal(0); + let release: () => void = () => {}; + const branchSaw: Params[] = []; + let signedOutIssued = false; + const el = document.createElement("div"); + const dispose = render(() => { + const held = createMemo(() => { + const g = gate(); + return g === 0 ? g : new Promise(r => (release = () => r(g))); + }); + const chrome = createMemo(() => + probe === "isPending" ? (isPending(plan), plan()) : probe === "latest" ? latest(plan) : plan() + ); + const params = createMemo(() => ({ + plan: probe === "inline isPending" ? (isPending(plan), plan()) : chrome(), + signedIn: session() !== undefined + })); + const signedOutOnly = createMemo( + () => { + signedOutIssued = true; + return new Promise(() => {}); + }, + { lazy: true } + ); + return ( + root}> + {held()} + signed out}> + + {(() => { + const p = params(); + branchSaw.push(p); + return p.signedIn ? "rows" : signedOutOnly(); + })()} + + + + ); + }, el); + try { + flush(); + expect(el.textContent).toBe("0signed out"); + setGate(1); + setSession("acc"); + flush(); + setPlan(true); + flush(); + return { el, branchSaw, signedOutIssued, release, dispose }; + } catch (e) { + dispose(); + throw e; + } +} + +for (const probe of ["plain read", "isPending", "latest"] as Probe[]) + test(`${probe}: the branch the hold opens reads the hold's session and the hold lands`, async () => { + const p = await arrive(probe); + try { + expect(p.branchSaw.at(-1)).toMatchObject({ + plan: probe === "isPending" ? false : true, + signedIn: true + }); + p.release(); + await tick(); + await tick(); + expect(p.el.querySelector("b")?.textContent).toBe("rows"); + } finally { + p.dispose(); + } + }); + +test("inline isPending(plan): the branch reads the hold's session and the hold lands without the signed-out fetch", async () => { + const p = await arrive("inline isPending"); + try { + expect(p.branchSaw.at(-1)).toMatchObject({ plan: false, signedIn: true }); + p.release(); + await tick(); + await tick(); + expect(p.el.querySelector("b")?.textContent).toBe("rows"); + } finally { + p.dispose(); + } +});