From 6c37524b11b58437aa9fe0fb13bba743d378ca4d Mon Sep 17 00:00:00 2001 From: Brenley Dueck Date: Tue, 15 Sep 2026 08:34:42 -0500 Subject: [PATCH] fix(signals): isPending() inside a memo reports a hold whose staged node is unstamped (#3457) A memo wrapping isPending(copy) cached false for a whole hold while a direct isPending(copy) render-effect probe read true. `slow` pended first and opened the transaction; `copy` then staged its value straight into the transaction's batch, unstamped until the flush stashed the hold. The wrapper memo, recomputing on the companion flip, read copy's fresh staged value, and heldAwaitingAsync's reporter scan was gated on the missing `_transition` stamp, so the A10 fresh-read pairing rule muted the verdict even though the transaction's async source was still computing. Run the reporter scan for an unstamped node too: with a transaction active, the ambient batch IS that transaction, so the node's staged write belongs to the transaction `t` resolved to. The pairing rule still stands for a landed answer awaiting reveal, and the #3078 action carve-out is untouched. Spec note under A10; rules index regenerated; changeset. Co-Authored-By: Claude Fable 5.1 --- .changeset/ispending-memo-unstamped-hold.md | 7 ++ packages/signals/docs/RULES-INDEX.md | 2 +- packages/signals/docs/SPEC-ASYNC-SEMANTICS.md | 2 +- packages/signals/src/core/verdict.ts | 13 ++- ...ispending-memo-unstamped-hold-3457.test.ts | 85 +++++++++++++++++++ ...spending-memo-unstamped-hold-3457.spec.tsx | 65 ++++++++++++++ 6 files changed, 168 insertions(+), 6 deletions(-) create mode 100644 .changeset/ispending-memo-unstamped-hold.md create mode 100644 packages/signals/tests/ispending-memo-unstamped-hold-3457.test.ts create mode 100644 packages/web/test/ispending-memo-unstamped-hold-3457.spec.tsx diff --git a/.changeset/ispending-memo-unstamped-hold.md b/.changeset/ispending-memo-unstamped-hold.md new file mode 100644 index 000000000..3726f4e89 --- /dev/null +++ b/.changeset/ispending-memo-unstamped-hold.md @@ -0,0 +1,7 @@ +--- +"@solidjs/signals": patch +--- + +A memo wrapping `isPending(x)` agrees with a direct `isPending(x)` read while a sibling async memo holds the write (#3457). + +- The fresh-read pairing rule (A10) only mutes a verdict for a LANDED answer awaiting reveal; while the transaction still has an async source computing, pending is the verdict for every reader. That carve-out was gated on the node's `_transition` stamp, but a sync memo staged AFTER the transaction opened is pushed straight into the transaction's batch and is not stamped until the flush stashes the hold. A wrapper memo recomputing on the companion flip read the memo's fresh staged value mid-flush, was told "not pending", and cached `false` for the whole hold, while the direct render-effect probe (which reads the committed value under the companion lane) reported `true`. The scan now runs for an unstamped node too: the transaction it resolves to is the one that owns its staged write. diff --git a/packages/signals/docs/RULES-INDEX.md b/packages/signals/docs/RULES-INDEX.md index 1248f7082..e2394e677 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:109` | verdict.ts×2 | spec-async-semantics.test.ts×2 visibility-oracle-store.test.ts×1 visibility-oracle.test.ts×2 | [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:117` | — | createMemo.test.ts×1 visibility-oracle-store.test.ts×1 visibility-oracle.test.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:125` | — | spec-async-semantics.test.ts×3 visibility-oracle-store.test.ts×5 | [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:133` | invariants.ts×1 | createMemo.test.ts×1 latest-isPending-consistency.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:133` | invariants.ts×1 verdict.ts×1 | createMemo.test.ts×1 ispending-memo-unstamped-hold-3457.test.ts×2 latest-isPending-consistency.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:51` | — | latest-isPending-consistency.test.ts×1 visibility-oracle-store.test.ts×1 visibility-oracle.test.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:141` | — | 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:149` | async.ts×1 | 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 … | diff --git a/packages/signals/docs/SPEC-ASYNC-SEMANTICS.md b/packages/signals/docs/SPEC-ASYNC-SEMANTICS.md index 441bb080e..0006484f2 100644 --- a/packages/signals/docs/SPEC-ASYNC-SEMANTICS.md +++ b/packages/signals/docs/SPEC-ASYNC-SEMANTICS.md @@ -134,7 +134,7 @@ After an async memo resolves, `[isPending(x), latest(x)]` is `[false, resolvedVa **Status:** **ruled** — #2831 finding 2 **Pinned by:** `tests/latest-isPending-consistency.test.ts` -**Mechanism (index, 2026-09-14):** `_recordFresh` (#2831): a probe that observed the fresh value cannot pair it with `pending`. +**Mechanism (index, 2026-09-14):** `_recordFresh` (#2831): a probe that observed the fresh value cannot pair it with `pending`. The pairing only covers a LANDED answer awaiting reveal: while the transaction still has an async source computing, the fresh value is an input and pending stays the verdict for every reader (`heldAwaitingAsync`, #3028) — including a node staged after the transaction opened, which carries no `_transition` stamp until the flush stashes the hold (#3457). `[isPending(x), x()]` read in one scope is atomic: a reader that observed the fresh value must not see `pending === true` for it. diff --git a/packages/signals/src/core/verdict.ts b/packages/signals/src/core/verdict.ts index dfa45170f..430ecd17a 100644 --- a/packages/signals/src/core/verdict.ts +++ b/packages/signals/src/core/verdict.ts @@ -569,10 +569,15 @@ function heldAwaitingAsync(el: Signal | Computed): boolean { // action (#2831: a reader that saw the new value must not also see // pending); still-computing answers are covered by the reporter scan. if (t._actions.length && !(el as Partial>)._fn) return true; - // A node not yet stamped with a transition only qualifies through the - // action check above; the reporter scan below is for transition-held - // writes whose source async is still computing. - if (!et) return false; + // The reporter scan runs for an unstamped node too (#3457): a node staged + // AFTER the transaction opened is pushed straight into the transaction's + // batch (queuePendingNode, once initTransition adopted it) and only gets + // its `_transition` stamp when the flush stashes the hold, but its staged + // value is already the transaction's, and `t` resolved to that very + // transaction above. Gating on the stamp let a memo whose recompute read + // a sync memo's fresh staged value mid-flush pair "not pending" with it + // (A10) while the transaction's async source was still computing, so a + // memo-wrapped isPending() read false where a direct probe read true. for (const [source, reporters] of t._asyncReporters) { if ( reporters.size && diff --git a/packages/signals/tests/ispending-memo-unstamped-hold-3457.test.ts b/packages/signals/tests/ispending-memo-unstamped-hold-3457.test.ts new file mode 100644 index 000000000..9fcda43d7 --- /dev/null +++ b/packages/signals/tests/ispending-memo-unstamped-hold-3457.test.ts @@ -0,0 +1,85 @@ +/** + * #3457: a memo wrapping isPending(copy) must agree with a direct + * isPending(copy) read while a sibling async memo holds the write. + * + * `slow` pends first and opens the transaction; `copy` then stages its value + * straight into the transaction's batch, unstamped until the flush stashes + * the hold. The wrapper memo recomputes on the companion flip, reads copy's + * fresh staged value, and the A10 pairing rule must not mute the verdict: + * the transaction's async source is still computing (#3028), stamp or not. + */ +import { afterEach, describe, expect, it } from "vitest"; +import { + createMemo, + createRenderEffect, + createRoot, + createSignal, + flush, + isPending +} from "../src/index.js"; + +afterEach(() => flush()); + +const delay = (ms: number, value: T) => new Promise(r => setTimeout(r, ms, value)); + +describe("memo-wrapped isPending agrees with a direct read through a hold (#3457)", () => { + it("A10 / #3457 isPending(copy) inside a memo reports the hold like the direct probe", async () => { + let setCount!: (v: number) => void; + let direct: boolean | undefined; + let viaMemo: boolean | undefined; + let slowShown: number | undefined; + createRoot(() => { + const [count, set] = createSignal(0); + setCount = set; + const slow = createMemo(() => delay(20, count())); + const copy = createMemo(() => count()); + const pending = createMemo(() => isPending(copy)); + createRenderEffect( + () => count(), + () => {} + ); + createRenderEffect( + () => slow(), + v => { + slowShown = v; + } + ); + createRenderEffect( + () => isPending(copy), + v => { + direct = v; + } + ); + createRenderEffect( + () => pending(), + v => { + viaMemo = v; + } + ); + }); + flush(); + await delay(40, 0); + flush(); + expect(slowShown).toBe(0); + expect(direct).toBe(false); + expect(viaMemo).toBe(false); + + setCount(1); + flush(); + // Held: the old value displays, both probes report pending. + expect(slowShown).toBe(0); + expect(direct).toBe(true); + expect(viaMemo).toBe(true); + await delay(5, 0); + flush(); + expect(slowShown).toBe(0); + expect(direct).toBe(true); + expect(viaMemo).toBe(true); + + await delay(60, 0); + flush(); + expect(slowShown).toBe(1); + expect(direct).toBe(false); + expect(viaMemo).toBe(false); + }); +}); diff --git a/packages/web/test/ispending-memo-unstamped-hold-3457.spec.tsx b/packages/web/test/ispending-memo-unstamped-hold-3457.spec.tsx new file mode 100644 index 000000000..eeeca36b1 --- /dev/null +++ b/packages/web/test/ispending-memo-unstamped-hold-3457.spec.tsx @@ -0,0 +1,65 @@ +/** + * @jsxImportSource @solidjs/web + * @vitest-environment jsdom + */ + +// #3457: a memo wrapping isPending(copy) must agree with a direct +// isPending(copy) read in JSX while a sibling async memo holds the write. +import { describe, expect, test } from "vitest"; +import { createMemo, createSignal, flush, isPending } from "solid-js"; +import { render } from "@solidjs/web"; + +const delay = (ms: number, value?: T) => + new Promise(r => setTimeout(r, ms, value as T)); + +function snap(div: HTMLElement) { + return Array.from(div.querySelectorAll("p")) + .map(p => p.textContent) + .join(" | "); +} + +function App() { + const [count, setCount] = createSignal(0); + const slow = createMemo(() => delay(100, count())); + const copy = createMemo(() => count()); + const pending = createMemo(() => isPending(copy)); + return ( + <> + +

Count: {count()}

+

Slow: {slow()}

+

Direct pending: {String(isPending(copy))}

+

Memo pending: {String(pending())}

+ + ); +} + +describe("memo-wrapped isPending agrees with a direct read through a hold (#3457)", () => { + test("A10 / #3457 both probes read true for the whole hold, false after the reveal", async () => { + const div = document.createElement("div"); + document.body.appendChild(div); + const dispose = render(() => , div); + await delay(120); + flush(); + expect(snap(div)).toBe("Count: 0 | Slow: 0 | Direct pending: false | Memo pending: false"); + + div.querySelector("button")!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + const log: string[] = []; + for (let t = 0; t <= 140; t += 20) { + flush(); + log.push(snap(div)); + await delay(20); + } + const held = log.filter(l => l.includes("Slow: 0")); + expect(held.length).toBeGreaterThan(0); + for (const l of held) { + expect(l, l).toBe("Count: 0 | Slow: 0 | Direct pending: true | Memo pending: true"); + } + expect(log[log.length - 1]).toBe( + "Count: 1 | Slow: 1 | Direct pending: false | Memo pending: false" + ); + + dispose(); + div.remove(); + }); +});