From 66b7e786550d5ee9e83b56fd1ed41caacddb7581 Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Mon, 31 Aug 2026 00:25:28 -0700 Subject: [PATCH 1/2] Close the ambient transaction window when a transaction opens without writes (#3141) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Parking is flush-driven, but a transaction opened with no writes before its first suspension — an action whose first statements only await — scheduled nothing, so activeTransition and the adopted batch stayed armed across the async gap and captured the next unrelated work to arrive: an optimistic store's landing on a still-live lane was held until the stranger action settled, an unowned optimistic write rode that transaction instead of reverting, and deep()/per-key readers split-brained meanwhile. initTransition now guarantees a flush, enforcing the A26 containment ruling: the ambient window is one flush, period. Co-authored-by: Cursor --- ...x-lingering-ambient-transaction-capture.md | 5 + packages/signals/INTERNALS-ASYNC-STATE.md | 1 + packages/signals/SPEC-ASYNC-SEMANTICS.md | 2 +- packages/signals/src/core/scheduler.ts | 9 + .../store/optimistic-ambient-capture.test.ts | 136 +++++++++++++ .../optimistic-observer-invariance.test.ts | 190 ++++++++++++++++++ .../test/optimistic-playground-port.spec.tsx | 110 ++++++++++ 7 files changed, 452 insertions(+), 1 deletion(-) create mode 100644 .changeset/fix-lingering-ambient-transaction-capture.md create mode 100644 packages/signals/tests/store/optimistic-ambient-capture.test.ts create mode 100644 packages/signals/tests/store/optimistic-observer-invariance.test.ts create mode 100644 packages/web/test/optimistic-playground-port.spec.tsx diff --git a/.changeset/fix-lingering-ambient-transaction-capture.md b/.changeset/fix-lingering-ambient-transaction-capture.md new file mode 100644 index 000000000..948519ab1 --- /dev/null +++ b/.changeset/fix-lingering-ambient-transaction-capture.md @@ -0,0 +1,5 @@ +--- +"@solidjs/signals": patch +--- + +Fix unrelated async work being captured by a lingering ambient transaction (#3141). Parking is flush-driven, but a transaction opened without any writes — an action whose first statements only await — scheduled nothing, so `activeTransition` and the adopted batch stayed armed across the async gap. The next unrelated work to arrive was adopted into a transaction it had nothing to do with: an optimistic store's authoritative landing would not render until the stranger action settled, an unowned optimistic write rode that transaction instead of reverting at the flush, and `deep()`/per-key readers disagreed about the committed value in the meantime. `initTransition` now guarantees a flush, so the ambient window closes in one flush regardless of whether the transaction wrote anything — enforcing the A26 containment ruling. diff --git a/packages/signals/INTERNALS-ASYNC-STATE.md b/packages/signals/INTERNALS-ASYNC-STATE.md index 99de6566f..9dab6f2f2 100644 --- a/packages/signals/INTERNALS-ASYNC-STATE.md +++ b/packages/signals/INTERNALS-ASYNC-STATE.md @@ -50,6 +50,7 @@ Semantics of the `(_pendingValue, _overrideValue)` pair for an optimistic node ## 3. Transitions (`scheduler.ts`) - Created by `initTransition` on the first transition-worthy write; at most one `activeTransition` per flush; concurrent ones merge (`mergeTransitionState`, `_done` forwarding pointer). +- `initTransition` ends by scheduling a flush: the ambient window is one flush by definition, but parking is flush-driven, so a transaction opened with no writes (an action that only awaits) would otherwise leave `activeTransition` and the adopted batch armed across the async gap, capturing the next unrelated work to arrive — the A26-rejected behavior (#3141). - `_asyncReporters: Map>` — which computeds are blocked on which async sources. **Populated only from `GlobalQueue.notify` during render-effect status notification** `[ruled — async-registration-invariants rule]`. - `_pendingNodes` — nodes whose `_pendingValue` commits when the transition completes (`commitPendingNodes` → `commitPendingNode`). - `_optimisticNodes` — nodes whose override reverts at completion (`resolveOptimisticNodes`). diff --git a/packages/signals/SPEC-ASYNC-SEMANTICS.md b/packages/signals/SPEC-ASYNC-SEMANTICS.md index 538eb1859..5cb7c088d 100644 --- a/packages/signals/SPEC-ASYNC-SEMANTICS.md +++ b/packages/signals/SPEC-ASYNC-SEMANTICS.md @@ -43,7 +43,7 @@ optimistic lanes. | A24 | (**ruled 2026-07-13** — supersedes A20/A21; the converged model from the #2844/#2728 threads) **Question-scoped pending: a read is pending iff a value change is in flight for it that has not yet revealed, or it carries a live `affects()` mark.** (1) **Same-question motion is silent.** Async whose tracked inputs are value-stable — `refresh()`, polling, an action's confirm refetch — is a _re-ask of the same question_: the shown value still answers it, so `isPending` stays `false` and the fresh value reveals silently. (2) **A new question pends monotonically.** Any tracked input value change in flight (a `setSignal`, an upstream memo's new value, an optimistic write feeding downstream async) pends every read under the source until its answer reveals; **nothing can silence it** — pendingness is additive-only. (3) **Optimistic writes are verdict-inert.** An active override is the displayed value on its own slot: not pending from itself (only a held authoritative _correction_ differing from the override re-opens the verdict; a matching confirm reveals nothing), and it masks nothing — an override displaying over an in-flight new question is the honest mixed state `{ value: guess, pending: true }`. To _downstream_ async the write is a real input change and pends those slots normally. Action affordances still belong in the data (co-written flags — the A20 §1 half that survives). (4) **`affects(target, key?)` is the sole declaration verb** (single optional key since 2026-07-14; the variadic form read as a 1.x path and was dropped). Additive pending on exactly the marked data (store record → every record reachable from it at declaration time, captured proxies included per #2882, siblings untouched; key → that leaf slot; accessor → that source); **store marks — keyed and keyless — cover by raw identity, not by proxy family** (amended 2026-07-17, #2904): a keyed mark registers an identity scope (owning record's raw, narrowed to the key), so reads through any other proxy sharing that backing record — e.g. a derived optimistic store whose projection landed the source store's value — witness the mark and inherit it on nodes born during the window, exactly as keyless scopes do **and on everything derived from it** (re-ruled 2026-07-14: a mark is a synthetic in-flight change on the normal status rails — `isPending(() => derived())` reads `true` during a mark window on `derived`'s inputs, exactly as it would over real in-flight async — while the marked values themselves stay readable; **mark-only pending is value-transparent through derivation too** (amended 2026-07-14, #2886): a read whose owner's pending sources are all mark sentinels never suspends — pendingness reaches readers only through verdicts, so optimistic writes under a whole-store mark keep rendering in live tracked readers; a mark's channel is never a re-ask, so a declared reload's own `refresh()` cannot silence it, and a mark never blocks its own transaction's settlement), live from declaration until its surrounding transaction settles or reverts (ambient marks release at flush end). Four corollaries pinned by the #2893 audit (2026-07-16): **(a)** derivation coverage is transitive and probe-stable — tracked reads of mark-pended owners re-establish the mark on the reader after any mid-window recompute (including the recompute an `isPending()` probe itself triggers), at every derivation depth, for graphs built before or during the window; **(b)** mark propagation is transaction-inert — pended subscribers are not queued as pending nodes, so plain writes to marked data (value-transparency) and to unmarked data sharing a downstream memo commit and render immediately, and concurrent actions don't merge into the marker's transaction through the pend; **(c)** a real error outranks a mark — a node holding `STATUS_ERROR` neither takes a sentinel on propagation nor re-applies collected marks after its recompute, so the user's error is never clobbered by a sentinel `NotReadyError`; **(d)** the pending-source container survives any number of overlapping sources (the singular→Set migration bug stranded mark sentinels forever on the third source — exactly the keyless-store-mark-over-`mapArray` shape). The declared-reload idiom `affects(x); refresh(x)` is how process knowledge enters the verdict when the graph can't see the change yet. Trade accepted knowingly: a re-ask that happens to return different data is silent until it reveals — honest silence over blanket alarm; whoever knows declares. | maintainer ruling 2026-07-13 (#2844/#2728 convergence; cause-scoped pending, per-path masking + UNCHANGED vouching, `background()`, and lane-bounded vouches each rejected on the way) | `tests/question-scoped-pending.test.ts` (scenario matrix: foos bug, list over-lighting, navigation-over-override, poll, reload, iMessage posture; cross-family raw sharing #2904); `tests/affects-propagation.test.ts` (marks through derivation: bare-mark windows on memos, late-mark wake, mid-mark landing hold, settle release, no settlement deadlock, store record/keyed marks reaching derived readers); `tests/affects-audit-2893.test.ts` (audit corollaries: container survival under 3+ sources, transaction-inert propagation, transitive/probe-stable re-establishment, error precedence); re-pinned A13/A14/A20-block in `tests/spec-async-semantics.test.ts`; `tests/createOptimistic.test.ts`, `tests/store/createOptimisticStore.test.ts`, `tests/store/createProjection.async.test.ts`, `tests/latest-isPending-consistency.test.ts`, `tests/createMemo.test.ts`, `tests/createLoadingBoundary.test.ts` (quiet-refresh + declared-reload re-pins); INV-10 (affects-count balance) | | A25 | (**ruled 2026-07-16**, #2897) **A derived store's seed is a draft, never an observable value.** The seed exists for the derive function — self reads while it works its draft are the point — but to every outside consumer the store is _uninitialized_ (A19 exception 1: loading, not pending) until the first resolution lands; for async-iterator derives, until the **first yield** lands (uninitialized only until then — each later yield is a revealed snapshot, readable between yields even while the generator is still running). During that window every outside consumer path throws: tracked reads suspend into loading boundaries via their node's `NotReadyError` as always, and the untracked fall-throughs — property reads, `in` checks, enumeration/spread — throw the same `NotReadyError` from the firewall (in dev strictRead scopes, i.e. component bodies, the more descriptive `PENDING_ASYNC_UNTRACKED_READ` error wins, matching async memos and preventing infinite loops). Returning the seed leaked a value the reader could never observe updating; returning `undefined` would break non-nullable types. Write-path reads (reconcile enumerating during the first landing) are exempt — they _are_ the initialization. This is safeguard parity: memos already behaved this way; store proxies bypassed `read()` and with it every guard. **Write-visibility corollary (ruled 2026-07-17, #2910 follow-up): the seed IS visible to write-path consumers.** A setter's function-form argument — the store setter's draft, `prev` in `set(prev => …)` — reads the raw current state: the seed for an uninitialized derived store, `undefined` for an uninitialized optimistic computed (it has no seed argument), the displayed value once initialized. Same exemption as the derive body: writes need a base, and because every read channel throws during the window, no consumer can _rely_ on the seed — visibility on the write path leaks nothing observable. Absolute writes were never gated. | maintainer rulings 2026-07-16 ("a seed… should never be visible under any case"; "we throw NotReady except in top-level component scope where we throw that other error"; "self reads are fine though — that's the point of seed, but outside isn't") and 2026-07-17 ("if the seed is visible in compute body it probably should be visible on write.. it throws on read so no consumer can rely on it") | `tests/strict-read-pending-store.test.ts` (untracked dev/prod matrix); `tests/store/createProjection.async.test.ts` (seed hidden until first resolution/first yield, supersession keeps it hidden, enumeration throws); `tests/uninitialized-visibility.test.ts` (write-path seed visibility, loading-vs-pending probe #2910) | -| A26 | (**ruled 2026-07-17**, #2913) **`yield` is an action's only transaction-safe suspension point; internal `await` continuations run outside the transaction by platform necessity.** The driver regains control exclusively at yield boundaries (`it.next()` resumes the body synchronously, so `restoreTransition` wraps the segment); a post-`await` continuation is a bare promise job the runtime cannot hook — JavaScript has no ambient async context (TC39 AsyncContext, unshipped), and holding `activeTransition` open across the await window was rejected as strictly worse: unrelated ambient writes interleaving during `await fetch()` (a user click, a timer) would be captured into the action's transaction and held until it settles. Consequences, all accepted: (1) a write to a **fresh** signal between an `await` and the next `yield` escapes and commits ambiently; (2) a signal **already written under the transaction** rejoins it even after an `await` (its `_transition` stamp routes the write back) — containment is write-history-dependent by design, not by accident; (3) the supported idiom is `await` for typed results, then a **bare `yield` before any writes** — re-entry is what `yield` is for, and TypeScript ergonomics are exactly why `await` stays welcome (yield results are untyped; awaited results are typed); (4) calling public `flush()` inside an action body is out of contract — it drains and stashes the transaction mid-step, stranding later same-segment writes; follow the idiom and there is nothing to flush for. The doc block on `action()` teaches the idiom. | maintainer ruling 2026-07-17 ("2913 is not addressable… it is a known thing and it isn't detectable, otherwise we'd have a different solution"; "one reason to not yield the promise is TypeScript — we are set up so you can await typed results and then yield nothing the next line"; flush-in-body ruled out of contract: "if they follow that they shouldn't be flushing there") | `tests/action-await-contract.test.ts` (documented escape, the await-then-bare-yield idiom, yield-the-promise alternative, pre-await stamp rejoin) | +| A26 | (**ruled 2026-07-17**, #2913; **enforcement hardened 2026-08-31**, #3141 — parking is flush-driven, and a transaction opened with no writes before its first suspension scheduled no flush, so `activeTransition` stayed ambient across the await window and captured exactly the unrelated work this ruling rejects: an optimistic store's authoritative landing on a still-live lane was adopted and held until the stranger action settled. `initTransition` now guarantees a flush, so the ambient window closes one flush later regardless of whether the transaction wrote anything) **`yield` is an action's only transaction-safe suspension point; internal `await` continuations run outside the transaction by platform necessity.** The driver regains control exclusively at yield boundaries (`it.next()` resumes the body synchronously, so `restoreTransition` wraps the segment); a post-`await` continuation is a bare promise job the runtime cannot hook — JavaScript has no ambient async context (TC39 AsyncContext, unshipped), and holding `activeTransition` open across the await window was rejected as strictly worse: unrelated ambient writes interleaving during `await fetch()` (a user click, a timer) would be captured into the action's transaction and held until it settles. Consequences, all accepted: (1) a write to a **fresh** signal between an `await` and the next `yield` escapes and commits ambiently; (2) a signal **already written under the transaction** rejoins it even after an `await` (its `_transition` stamp routes the write back) — containment is write-history-dependent by design, not by accident; (3) the supported idiom is `await` for typed results, then a **bare `yield` before any writes** — re-entry is what `yield` is for, and TypeScript ergonomics are exactly why `await` stays welcome (yield results are untyped; awaited results are typed); (4) calling public `flush()` inside an action body is out of contract — it drains and stashes the transaction mid-step, stranding later same-segment writes; follow the idiom and there is nothing to flush for. The doc block on `action()` teaches the idiom. | maintainer ruling 2026-07-17 ("2913 is not addressable… it is a known thing and it isn't detectable, otherwise we'd have a different solution"; "one reason to not yield the promise is TypeScript — we are set up so you can await typed results and then yield nothing the next line"; flush-in-body ruled out of contract: "if they follow that they shouldn't be flushing there") | `tests/action-await-contract.test.ts` (documented escape, the await-then-bare-yield idiom, yield-the-promise alternative, pre-await stamp rejoin); `tests/store/optimistic-ambient-capture.test.ts` (#3141 — the ambient window closes in one flush even for a writeless transaction) | | A27 | (**ruled 2026-08-10**) **The commit-#0 loading window is loading-class and verdict-quiet.** A node born committed via `loadingValue` (memos: `createMemo` / `createSignal(fn)` / `createOptimistic(fn)`) or `seedLoadingValue` (projections: `createProjection` / `createStore(fn)` / `createOptimisticStore(fn)`) starts with the loading value as commit #0 of its lineage instead of `STATUS_UNINITIALIZED`. While its first real answer is in flight, the window is loading-class on every axis: (1) **reads** — every consumer path serves commit #0 (no `NotReadyError`, no Loading-boundary suspension; `latest()` and `resolve()` return it; it is the compute's first `prev`); (2) **transitions** — the window never initiates or extends one (matching boundary-fallback semantics): ambient writes concurrent with the window commit immediately rather than being held, and a loading node mounted inside a live transition does not add to what that transition waits for; (3) **verdict** — `isPending` reads false at the source, upstream, and downstream, in both forms. The quiet ruling is not an exception to A19 but its question scoping applied: commit #0 answers the question **by declaration**, so the first flight is re-ask-shaped — the shown answer still answers the question (A24 family). The alternative was rejected as structurally unavailable: genuine pending is chain-shaped (the shadow of a held commit — held write upstream, in-flight async at the node, propagated non-finality downstream), and the window has no held commit to shadow, so a true verdict could only exist as a point anomaly at the probed node while upstream and downstream read false — and making it propagate would reintroduce exactly the status machinery the window exists to silence (and re-open the server/client split: `isPending` is always false on the server). First-load affordances are therefore the **value channel**'s job — the author encodes provenance (`null`, a `skeleton` flag) into the loading value itself — and `data.skeleton || isPending(data)` covers the two disjoint states. The window closes at the first real landing on any path (sync return, sync-resolved thenable, first iterator yield, async settle); a real error answers reads but does not close it — a retry serves commit #0 again. After close, A19 applies unchanged: refetches are pending-class forever. A25 is unchanged for plain seeds: without `seedLoadingValue` a derived store's seed remains an unobservable draft; `seedLoadingValue` is precisely the author promoting the seed to commit #0 — observable by declaration. | maintainer ruling 2026-08-10 ("the reason I had skeleton or isPending is because isPending would be false in my mind"; "it was false both upstream and downstream") | `tests/loading-value.test.ts` | ## Tier B (inferred — needs verdict) diff --git a/packages/signals/src/core/scheduler.ts b/packages/signals/src/core/scheduler.ts index 1a61ef064..6c2aeb0e9 100644 --- a/packages/signals/src/core/scheduler.ts +++ b/packages/signals/src/core/scheduler.ts @@ -717,6 +717,15 @@ export class GlobalQueue extends Queue { for (const lane of activeLanes) { if (!lane._transition) lane._transition = activeTransition; } + // A transaction's ambient window is one flush. Entering must therefore + // guarantee a flush: a transaction opened with no writes (an action whose + // first statements only await) otherwise leaves activeTransition and the + // adopted batch armed across the async gap, and the next unrelated work + // to arrive — an optimistic store's authoritative landing, a plain async + // settle — is adopted into a transaction it has nothing to do with + // (#3141). The scheduled flush parks the incomplete transaction through + // the normal machinery and detaches the ambient slots first. + schedule(); } } diff --git a/packages/signals/tests/store/optimistic-ambient-capture.test.ts b/packages/signals/tests/store/optimistic-ambient-capture.test.ts new file mode 100644 index 000000000..db835afcd --- /dev/null +++ b/packages/signals/tests/store/optimistic-ambient-capture.test.ts @@ -0,0 +1,136 @@ +/** + * #3141: a transaction's ambient window must be one flush. + * + * `initTransition` used to leave `activeTransition` (and the adopted batch) + * armed indefinitely when the transaction was opened without any writes — an + * action whose first statements only await schedules nothing, so no flush + * ever parked it. The next unrelated work to arrive was adopted into a + * transaction it had nothing to do with: an optimistic store's authoritative + * landing (with its flight still pending, so its live lane was entangled) + * would not render until the stranger action settled seconds later, an + * unowned optimistic write rode the same transaction instead of reverting, + * and deep()/per-key readers split-brained over what "committed" meant + * meanwhile. + * + * The construction here is exact and deterministic: + * - the action is invoked in a bare macrotask and performs no writes before + * its first await, so pre-fix nothing scheduled a flush and the + * transaction stayed ambient; + * - the store generator keeps running after the yield (awaiting a further + * gate), so the landing arrives on a still-live lane — the entanglement + * target. (A generator that ends at the yield resolves its flight and + * escapes the capture, which is why simpler distillations don't fail.) + * No manual flush() between steps — a manual flush parks the transaction and + * masks the window. The scheduler's own flushing must do the right thing. + */ +import { describe, expect, it } from "vitest"; +import { + action, + createOptimisticStore, + createRenderEffect, + createRoot, + deep, + flush +} from "../../src/index.js"; + +function deferred() { + let resolve!: (v: T) => void; + const promise = new Promise(r => (resolve = r)); + return { promise, resolve }; +} + +// A macrotask hop: lets the scheduler's own flush run without forcing one. +const hop = () => new Promise(r => setTimeout(r, 0)); + +describe("#3141: lingering ambient transaction capture", () => { + it("an in-flight store landing renders immediately despite an unrelated open action", async () => { + const stepGate = deferred(); + const endGate = deferred(); + const actGate = deferred(); + const domLog: string[] = []; + const obsLog: string[] = []; + let setState!: (fn: (s: number[]) => void) => void; + let dispose!: () => void; + + const act = action(function* (p: Promise) { + yield p; + }); + + createRoot(d => { + dispose = d; + const [s, ss] = createOptimisticStore( + async function* () { + yield [1, 2, 3]; + await stepGate.promise; + yield [3, 2, 1]; + // The flight stays pending past the yield: the landing arrives on a + // live lane, which is what the lingering transaction entangled. + await endGate.promise; + }, + [1, 2] + ); + setState = ss; + + createRenderEffect( + () => JSON.stringify(s), + v => { + domLog.push(v); + } + ); + createRenderEffect( + () => deep(s), + v => { + obsLog.push(JSON.stringify(v)); + } + ); + }); + + await hop(); + expect(domLog.at(-1)).toBe("[1,2,3]"); + + // Open the transaction in a bare macrotask: no writes precede its first + // await, so nothing else will schedule the flush that parks it. + let acting!: Promise; + setTimeout(() => { + acting = act(actGate.promise); + }, 0); + await hop(); + await hop(); + + // The store's own truth lands mid-action: it must render now — not when + // the unrelated action settles — and both reader families must agree. + stepGate.resolve(); + await hop(); + await hop(); + expect(domLog.at(-1)).toBe("[3,2,1]"); + expect(obsLog.at(-1)).toBe("[3,2,1]"); + + // An unowned optimistic push composes over the CURRENT base and, with no + // transaction of its own, reverts at the flush — it must not be adopted + // by the open action and persist until that action settles. + setState(draft => { + draft.push(1111); + }); + await hop(); + expect(domLog.at(-1)).toBe("[3,2,1]"); + expect(obsLog.at(-1)).toBe("[3,2,1]"); + expect(domLog).not.toContain("[1,2,3,1111]"); // the split-brain composition + + // At no point may the two reader families disagree about the value. The + // observer may legitimately fire more often (its initial run doubles), so + // compare deduped value sequences: pre-fix the DOM showed [1,2,3,1111] + // while deep() reported [3,2,1,1111]. + const dedupe = (log: string[]) => log.filter((v, i) => v !== log[i - 1]); + expect(dedupe(obsLog)).toEqual(dedupe(domLog)); + + actGate.resolve(); + await acting; + await hop(); + expect(domLog.at(-1)).toBe("[3,2,1]"); + + endGate.resolve(); + await hop(); + dispose(); + flush(); + }); +}); diff --git a/packages/signals/tests/store/optimistic-observer-invariance.test.ts b/packages/signals/tests/store/optimistic-observer-invariance.test.ts new file mode 100644 index 000000000..39dafddfb --- /dev/null +++ b/packages/signals/tests/store/optimistic-observer-invariance.test.ts @@ -0,0 +1,190 @@ +/** + * Reproduction for #3141: a generator-backed optimistic store, read by a + * JSON-rendering effect (the DOM stand-in), renders DIFFERENTLY depending on + * whether a second, passive `createRenderEffect(() => deep(s), ...)` observer + * exists — and while an optimistic override is visible, the two readers can + * disagree about the base state underneath the overlay in the same flush. + * + * Timeline mirrored from the report: + * 1. generator yields [1,2,3] + * 2. a long action transition starts + * 3. generator yields [3,2,1] (mid-action) + * 4. an optimistic setter pushes 1111 (mid-action) + * 5. generator draft-mutates push(5), push(666), returns + * 6. the action settles; the override drops + * + * Invariants pinned here: + * - the rendered sequence is identical with and without the deep observer + * - whenever both readers emit, they emit the same value + */ +import { + action, + createOptimisticStore, + createRenderEffect, + createRoot, + deep, + flush +} from "../../src/index.js"; + +afterEach(() => flush()); + +interface Gates { + afterFirstYield: () => void; + afterSecondYield: () => void; + afterPush5: () => void; +} + +function setup(withDeepObserver: boolean) { + const rendered: string[] = []; + const observed: string[] = []; + const gates = {} as Gates; + let setState!: (fn: (s: number[]) => void) => void; + let resolveAction!: () => void; + let start!: () => Promise; + let generatorDone!: Promise; + + const gate1 = new Promise(r => (gates.afterFirstYield = r)); + const gate2 = new Promise(r => (gates.afterSecondYield = r)); + const gate3 = new Promise(r => (gates.afterPush5 = r)); + + const dispose = createRoot(disposer => { + let markDone!: () => void; + generatorDone = new Promise(r => (markDone = r)); + + const [s, ss] = createOptimisticStore( + async function* (draft) { + yield [1, 2, 3]; + await gate1; + yield [3, 2, 1]; + await gate2; + draft.push(5); + await gate3; + draft.push(666); + markDone(); + }, + [1, 2] + ); + setState = ss; + + // The DOM stand-in: renders the whole array, like `{JSON.stringify(s)}`. + createRenderEffect( + () => JSON.stringify(s), + v => { + rendered.push(v); + } + ); + + if (withDeepObserver) { + // The passive observer from the report: subscribes to everything, + // renders nothing. + createRenderEffect( + () => deep(s), + v => { + observed.push(JSON.stringify(v)); + } + ); + } + + start = action(function* () { + yield new Promise(r => (resolveAction = r)); + }); + + return disposer; + }); + + return { + rendered, + observed, + gates, + setState, + dispose, + begin: () => start(), + settleAction: () => resolveAction(), + generatorDone: () => generatorDone + }; +} + +async function drive(t: ReturnType) { + // Step 1: first yield lands. + flush(); + await Promise.resolve(); + flush(); + + // Step 2: long action transition begins. + const acting = t.begin(); + flush(); + + // Step 3: second yield lands mid-action. + t.gates.afterFirstYield(); + await new Promise(r => setTimeout(r, 0)); + flush(); + const atSecondYield = { + rendered: t.rendered.at(-1), + observed: t.observed.at(-1) + }; + + // Step 4: optimistic push mid-action. + t.setState(draft => { + draft.push(1111); + }); + flush(); + const atOverride = { + rendered: t.rendered.at(-1), + observed: t.observed.at(-1) + }; + + // Step 5: generator draft mutations, then it returns. + t.gates.afterSecondYield(); + await new Promise(r => setTimeout(r, 0)); + flush(); + t.gates.afterPush5(); + await new Promise(r => setTimeout(r, 0)); + flush(); + await t.generatorDone(); + flush(); + + // Step 6: the action settles; the override drops. + t.settleAction(); + await acting; + await Promise.resolve(); + flush(); + + return { atSecondYield, atOverride, final: t.rendered.at(-1) }; +} + +describe("optimistic store observer invariance (#3141)", () => { + it("renders the same sequence with and without a passive deep() observer", async () => { + const without = setup(false); + const baseline = await drive(without); + without.dispose(); + flush(); + + const withObserver = setup(true); + const observed = await drive(withObserver); + withObserver.dispose(); + flush(); + + // The mid-action yield must reach the renderer either way. + expect(baseline.atSecondYield.rendered).toBe("[3,2,1]"); + expect(observed.atSecondYield.rendered).toBe("[3,2,1]"); + + // The optimistic override overlays the same base either way. + expect(observed.atOverride.rendered).toBe(baseline.atOverride.rendered); + + // Settling drops the override and lands the generator's final state. + expect(baseline.final).toBe("[3,2,1,5,666]"); + expect(observed.final).toBe("[3,2,1,5,666]"); + }); + + it("simultaneous readers agree on the base under an optimistic override", async () => { + const t = setup(true); + const { atOverride } = await drive(t); + t.dispose(); + flush(); + + // The DOM read and the deep() observer ran in the same flush; they must + // describe the same array. (The report saw [1,2,3,1111] rendered while + // the observer logged [3,2,1,1111].) + expect(atOverride.rendered).toBe(atOverride.observed); + }); +}); diff --git a/packages/web/test/optimistic-playground-port.spec.tsx b/packages/web/test/optimistic-playground-port.spec.tsx new file mode 100644 index 000000000..bcd02aa2a --- /dev/null +++ b/packages/web/test/optimistic-playground-port.spec.tsx @@ -0,0 +1,110 @@ +/** + * @jsxImportSource @solidjs/web + * @vitest-environment jsdom + * + * Faithful port of the #3141 playground program: no manual flush() anywhere, + * real timers (scaled 1 playground second -> 30ms), scheduler auto-flush + * only. The DOM is sampled on a fine timer into a deduped timeline. + */ +import { describe, expect, test } from "vitest"; +import { action, createOptimisticStore, createRenderEffect, deep, Loading } from "solid-js"; +import { render } from "../src/index.js"; + +const S = 30; // 1 playground second +const sleep = (ms: number) => new Promise(r => setTimeout(r, ms)); + +async function run(enableBlock: boolean) { + const container = document.createElement("div"); + const observed: string[] = []; + + const act = action(function* (ms: number) { + yield sleep(ms); + }); + + function App() { + const [s, ss] = createOptimisticStore( + async function* (draft) { + yield [1, 2, 3]; + await sleep(2 * S); + yield [3, 2, 1]; + await sleep(2 * S); // offset 4s + draft.push(5); + await sleep(1 * S); // offset 5s + draft.push(666); + await sleep(1 * S); // offset 6s + }, + [1, 2] + ); + + setTimeout(() => { + act(10 * S); // 10s transition + setTimeout(() => { + ss(draft => { + draft.push(1111); + }); + }, 3 * S); // offset 3s + }, 0); + + if (enableBlock) { + createRenderEffect( + () => deep(s), + v => { + observed.push(JSON.stringify(v)); + } + ); + } + + return ( +
+ {JSON.stringify(s)} +
+ ); + } + + const dispose = render(() => , container); + + const timeline: Array<{ at: number; dom: string }> = []; + const start = Date.now(); + const deadline = 12 * S; + let last: string | undefined; + while (Date.now() - start < deadline) { + const dom = container.textContent ?? ""; + if (dom !== last) { + timeline.push({ at: Date.now() - start, dom }); + last = dom; + } + await sleep(2); + } + + dispose(); + return { timeline, observed }; +} + +describe("playground port (#3141)", () => { + test("the deep() observer does not change the rendered timeline", async () => { + const baseline = await run(false); + const withObserver = await run(true); + + // The sampler may or may not catch the momentary 1111 flash in any given + // run, so exact timeline equality between the two runs would flake. + // Assert order-based invariants that only the bug violates instead. + const authoritative = ["[1,2,3]", "[3,2,1]", "[3,2,1,5,666]"]; + const authoritativeOrder = (doms: string[]) => doms.filter(d => authoritative.includes(d)); + + for (const { timeline } of [baseline, withObserver]) { + const doms = timeline.map(e => e.dom); + // Truth progresses through every landing, in order, ending settled — + // pre-fix the [3,2,1] landing never rendered on its own (it was held + // by the unrelated action and only surfaced at 10s or under the flash). + expect(authoritativeOrder(doms)).toEqual(authoritative); + // The split-brain signature: the optimistic push composed over the + // STALE base while deep() already saw the fresh one. + expect(doms).not.toContain("[1,2,3,1111]"); + // The unowned push must not persist until the action settles: whatever + // transients were sampled, the run ends on the settled truth. + expect(doms.at(-1)).toBe("[3,2,1,5,666]"); + } + expect(withObserver.observed).not.toContain("[1,2,3,1111]"); + expect(withObserver.observed.at(-1)).toBe("[3,2,1,5,666]"); + }, 20000); +}); From 0d3e2d1272d6a7ef6dd04d98a4af9817c3b060e9 Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Mon, 31 Aug 2026 00:26:08 -0700 Subject: [PATCH 2/2] Pin transition commit-on-handled-error behavior from the #3107 router thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A source write entangled with downstream async that rejects must still commit — the error boundary handling the rejection is the transition resolving, not it failing. This pin came out of the router navigation investigation and was never landed; tests only, no changeset. Co-authored-by: Cursor --- .../router-transition-error-commit.test.ts | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 packages/signals/tests/router-transition-error-commit.test.ts diff --git a/packages/signals/tests/router-transition-error-commit.test.ts b/packages/signals/tests/router-transition-error-commit.test.ts new file mode 100644 index 000000000..50b81cfaa --- /dev/null +++ b/packages/signals/tests/router-transition-error-commit.test.ts @@ -0,0 +1,61 @@ +import { + createEffect, + createErrorBoundary, + createMemo, + createRenderEffect, + createRoot, + createSignal, + flush +} from "../src/index.js"; + +afterEach(() => flush()); + +it("commits a source write and its effect when a downstream async error is handled", async () => { + const error = new Error("lazy route failed"); + let reject!: (error: unknown) => void; + const [route, setRoute] = createSignal("/", { ownedWrite: true }); + let rendered = ""; + let external = "/"; + + createRoot(() => { + const lazyRoute = createMemo(() => { + if (route() === "/") return "home"; + return new Promise((_, fail) => (reject = fail)); + }); + const boundary = createErrorBoundary( + () => lazyRoute(), + caught => { + expect(caught()).toBe(error); + return "error"; + } + ); + createRenderEffect(boundary, value => { + rendered = value; + }); + createEffect( + route, + value => { + external = value; + }, + { defer: true } + ); + }); + + flush(); + expect(rendered).toBe("home"); + + setRoute("/plugins"); + flush(); + expect(route()).toBe("/"); + expect(rendered).toBe("home"); + expect(external).toBe("/"); + + reject(error); + await Promise.resolve(); + await Promise.resolve(); + flush(); + + expect(rendered).toBe("error"); + expect(route()).toBe("/plugins"); + expect(external).toBe("/plugins"); +});