Skip to content

fix(signals): derived store sync landing wakes parked readers - #3732

Closed
brenelz wants to merge 2 commits into
solidjs:nextfrom
brenelz:claude/issue-3726
Closed

brenelz wants to merge 2 commits into
solidjs:nextfrom
brenelz:claude/issue-3726

Conversation

@brenelz

@brenelz brenelz commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Fixes #3726

What was broken

A derived createStore whose first run returned a pending promise, and whose rerun after a source write returned a value synchronously, left some readers pending forever. A reader whose store node the landing did not change ("length" in store, Object.keys, an unchanged length) rendered blank inside ``, while a sibling read of the source updated.

Root cause

The projection computed kept STATUS_UNINITIALIZED until the flush commit, because recompute clears it only on a creation pass. The settle walk that releases readers parked on a superseded flight skips uninitialized nodes, and unchanged-node readers get no value notification to fall back on.

Change

After the synchronous commit, runProjectionComputedNext clears STATUS_UNINITIALIZED when the node's own flight was pending and no transaction holds the landing, the same way the async landing path does after its setter commit. A landing held by a transaction (a live action, a blocked transition, or another pending source) keeps the flag, so the seed stays invisible until that transaction commits.

Verification

  • cd packages/signals && npx vitest run tests/store/derived-presence-async-3726.test.ts: 3 passed. Two of the cases fail on next without the fix.
  • packages/signals: 4794 passed. packages/solid: 817 passed. packages/web: default, hydrate and server suites pass, except one server spec that also fails on next.

Open

  • The fix could instead live in core recompute, which would cover every computed type.
  • When a transaction holds the landing, readers parked on the superseded flight still do not wake after the commit. This happens on next too.

…s#3726)

A derived store whose first run returned a pending promise and whose rerun
landed synchronously kept STATUS_UNINITIALIZED on the projection computed
until the flush commit, because recompute only clears the flag on a
creation pass. The recompute-side settle walk (solidjs#3181) requires the node to
be initialized, so it skipped the projection, and readers subscribed to a
store node the landing left unchanged (`"length" in store` on an array
seed, `Object.keys`, an unchanged `length`) had no value notification to
fall back on. They stayed pending, blank inside a loading boundary, while
a sibling read of the source updated.

The projection's sync commit through the setter now retires the flag the
way asyncWrite does after its setter landing, so the walk releases those
readers in the same flush. The retirement is gated on the previous run
having been pending, which leaves a born-held creation pass unchanged.
The projection computed retires STATUS_UNINITIALIZED after a synchronous
landing only when the pass is mainline or its transaction is parked by
nothing but the node's own flight, so a landing staged by a live action
or beside another pending source keeps the seed invisible until that
transaction commits. The gate is keyed on the node's own pending-source
entry instead of any STATUS_PENDING, so a first run parked on an
upstream async source no longer trips it. The tests assert in the flush
that lands the sync value, register a length reader, and pin the
transaction-held case.
@changeset-bot

changeset-bot Bot commented Oct 1, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 37ac480

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

This PR includes changesets to release 12 packages
Name Type
@solidjs/signals Patch
test-integration Patch
@solidjs/web Patch
@solidjs/babel-plugin Patch
@solidjs/compiler Patch
@solidjs/diagnostics Patch
@solidjs/element Patch
@solidjs/h Patch
@solidjs/html Patch
solid-js Patch
@solidjs/universal Patch
todos-server-example Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codspeed

codspeed Bot commented Oct 1, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 185 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing brenelz:claude/issue-3726 (37ac480) with next (309b087)

Open in CodSpeed

Footnotes

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

@ryansolid

Copy link
Copy Markdown
Member

Thanks for this, @brenelz. The repro for #3726 and the notes on what's still open are really helpful.

I'm going to put this on hold rather than merge it. The reactive system in @solidjs/signals is being rewritten, and I don't want to land more changes in code that's about to be replaced. That also settles your open question about moving the fix into core recompute: it'll be handled in the new core. The cases in derived-presence-async-3726.test.ts will carry over as cases the rewrite has to pass. So will the case you flagged where a transaction holds the landing and parked readers don't wake.

I'll leave the PR open for now and come back to it once the rewrite lands.

— Claude via Cursor

ryansolid added a commit that referenced this pull request Oct 5, 2026
…ked readers (supersedes #3732) (#3789)

* test(signals): pin #3726 on L2 — derived store sync landing wakes parked readers

Port of #3732 (brenelz) onto the hold model. The PR patched
`store/next/projection.ts` to retire STATUS_UNINITIALIZED after a
synchronous landing so core's #3181 settle walk would release readers
parked on the superseded first flight: readers whose store node the
landing left unchanged (`"length" in store`, `Object.keys`, an unchanged
`length`) had no value notification to fall back on and stayed blank
inside a loading boundary while a sibling read of the source updated.

On L2 the core already holds, with no store-side patch: a reader of a
derive with a flight up is parked on the derive itself (`pullFamily`
links it, as a reader of a memo with a flight), and the derive's first
commit (uninitialized → a value) is a value change for every subscriber
(`recompute`: `wasUninitialized` → `insertSubs`) — the walk's
uninitialized exemption is moot. The PR's three tests pass unchanged;
its superseded source change is dropped (`src/store/next/` is gone).

Added: the report's sequence verbatim (the source write and the
superseded promise's resolution in one timer callback, under a loading
boundary, microtask-driven) in signals and as a `<Loading>` web spec; a
verdict probe parked on the first flight; and the case the PR listed as
open — a landing a transaction holds: the parked readers re-run as the
transaction's work and reveal at its commit, under a boundary and out
(red on the PR's base with and without its fix; green on L2).

Tests only — no changeset. RULES-INDEX
regenerated (A19/A25/A29 gain the test's citations).

Co-authored-by: Brenley Dueck <brenleydueck@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

* test(signals): pin GabbeV's board shape from #3726 on L2

The #3726 comment's playground, fetched: an async card source behind a
memo (`createMemo(() => ready() ?? pending)`), a derived
`createStore(() => source(), [])`, `createOptimisticStore` over it and a
keyed `<For>`, remounted under a `<Loading>` that has already revealed;
a count outside the view. Before, Activity → Board left "All cards: 1"
beside an empty lane: the remounted readers were parked on the derive's
first flight under the boundary's hold, and the held synchronous landing
never woke them — the case PR #3732 listed as open (red on its base with
and without its fix). Same root-cause family as the report, not a
distinct bug.

On L2 the revealed boundary holds the view switch (A29: a boundary
already showing content holds like any reader) and the lane reveals
with its rows as one frame — the count and the list never disagree.
Pinned as the playground itself (web spec) and a signals reduction.

The report's own playground, fetched too: the `Source:` hole is outside
the `<Loading>` and the promise is created once — the first web test
and the signals "report's playground" test now follow it exactly.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Brenley Dueck <brenleydueck@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@ryansolid

Copy link
Copy Markdown
Member

Thanks for this, @brenelz — superseded by #3789 (a036c226c), now merged into next.

Your three tests landed verbatim with Co-authored-by credit. The fix itself is subsumed by L2: a derive with a flight up is read tracked through pullFamily, so the parked reader is already linked to the firewall node and re-runs in the landing's flush — no separate wake path is needed.

The held-landing case this PR left open is now green and pinned alongside your tests, as is GabbeV's board shape from the issue thread.

Closing in favour of #3789.

— Claude via Cursor

@ryansolid ryansolid closed this Oct 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants