Skip to content

fix(signals): move-and-clear _optimisticNodes on transition merge#2892

Merged
ryansolid merged 1 commit into
solidjs:nextfrom
brenelz:fix/merge-transition-optimistic-dup
Jul 16, 2026
Merged

fix(signals): move-and-clear _optimisticNodes on transition merge#2892
ryansolid merged 1 commit into
solidjs:nextfrom
brenelz:fix/merge-transition-optimistic-dup

Conversation

@brenelz

@brenelz brenelz commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

mergeTransitionState copied outgoing._optimisticNodes into the target without clearing the outgoing array. Since the global queue's arrays alias the active transition's after initTransition, the adoption pass in initTransition re-pushed the same entries into the target right after the merge — duplicating every node, compounding across repeated merges. Mirror the move-and-clear pattern already used for _affectsNodes.

Repro: two overlapping actions with optimistic writes; when the first resumes and writes into a signal owned by the second's transition, the merged transition held the first action's optimistic node twice.

mergeTransitionState copied outgoing._optimisticNodes into the target
without clearing the outgoing array. Since the global queue's arrays
alias the active transition's after initTransition, the adoption pass
in initTransition re-pushed the same entries into the target right
after the merge — duplicating every node, compounding across repeated
merges. Mirror the move-and-clear pattern already used for
_affectsNodes.

Repro: two overlapping actions with optimistic writes; when the first
resumes and writes into a signal owned by the second's transition, the
merged transition held the first action's optimistic node twice.

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

changeset-bot Bot commented Jul 16, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 270b1b5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@codspeed-hq

codspeed-hq Bot commented Jul 16, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 7 untouched benchmarks
⏩ 113 skipped benchmarks1


Comparing brenelz:fix/merge-transition-optimistic-dup (270b1b5) with next (ca82433)

Open in CodSpeed

Footnotes

  1. 113 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 ryansolid merged commit bc82de2 into solidjs:next Jul 16, 2026
5 checks passed
ryansolid added a commit that referenced this pull request Jul 16, 2026
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