Skip to content

fix(signals): keep a mainline effect value out of its parked transaction (#3412) - #3420

Merged
ryansolid merged 2 commits into
solidjs:nextfrom
brenelz:investigate/loading-reset-stale-3412
Sep 14, 2026
Merged

ryansolid merged 2 commits into
solidjs:nextfrom
brenelz:investigate/loading-reset-stale-3412

Conversation

@brenelz

@brenelz brenelz commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Fixes #3412

Problem

With a Loading on={count()} boundary and an unconditional details() reader outside it, flipping show to false while details is still pending leaves show() ? details() : "hidden" displaying the old value until the unrelated async settles.

Cause

The count write opens a transaction that stays parked (Details and Panel read details() outside any boundary), and both effects are stamped with it. On the later ambient show write, Panel recomputes mainline, reads show as false, and publishes "hidden" into _value with mainline ownership — which is what the flush's "everything computed mainline applies now" rule expects.

recompute then forces a second run of the effect inside its own parked transaction to refresh the staged view. That re-run only writes _pendingValue, but it also re-stamped _valueTransition to the parked transaction, claiming ownership of the value the mainline run produced.

During finalization the boundary sweep sees copy settled and writes _disabled = false. That signal is stamped with the parked transaction (its on reset staged the fallback flip there), so the write legitimately re-enters it mid-flush. With the transaction active and Panel's ownership pointing at it, the runEffect ownership gate leaves Panel queued and the next pass parks it. Show applies because it was never stamped.

Without on, nothing re-enters the transaction in that flush, so the misattributed ownership is harmless and Panel applies by accident — which is why the minimization looks like a boundary bug.

Fix

Save _valueTransition before the forced in-transaction re-run and restore it afterward. The contested-effect bookkeeping is untouched, so the transaction still re-derives the effect against its own committed world at commit.

Tests

  • packages/signals/tests/effect-mainline-ownership-3412.test.ts — the four-variant matrix (boundary / on / unconditional reader) at the signals level; only the reporter's shape failed before the fix.
  • packages/web/test/loading-on-outside-reader-3412.spec.tsx — the same matrix rendered through @solidjs/web.
  • Full signals suite (1775 passed) and web suite (740 passed) are green with the fix.

🤖 Generated with Claude Code

…ion (solidjs#3412)

An effect stamped by a held transaction that recomputes on an unrelated
write, and no longer reads the held source, publishes its value mainline.
The forced re-run inside its own transaction (which only refreshes the
staged view) re-stamped `_valueTransition`, so when finalization re-entered
the transaction — a Loading `on` reset writing its `_disabled` signal — the
effect phase parked the mainline value with it. Restore the owner after the
re-run.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Sep 14, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: b28a7ea

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

This PR includes changesets to release 11 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

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 Sep 14, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 160 untouched benchmarks


Comparing brenelz:investigate/loading-reset-stale-3412 (b28a7ea) with next (7f6332a)

Open in CodSpeed

…p save/restore (+8 B)

Co-authored-by: Cursor <cursoragent@cursor.com>
@ryansolid
ryansolid merged commit 50323b4 into solidjs:next Sep 14, 2026
7 checks passed
ryansolid added a commit to brenelz/solid that referenced this pull request Sep 14, 2026
Resolve recompute's tail against solidjs#3420 (solidjs#3412): keep the held-children
bookkeeping and the _valueTransition save/restore around the forced
in-transaction re-run. Size ledger notes merged; caps unchanged.

Co-authored-by: Cursor <cursoragent@cursor.com>
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