fix(signals): re-adopt the queue batch when an action completes - #2950
Merged
ryansolid merged 1 commit intoJul 28, 2026
Merged
Conversation
done() restored the active transition with a bare setActiveTransition, leaving globalQueue._batch as a detached ambient batch until the scheduled flush. Anything registered in that microtask window was stranded with nothing to finalize it: - a completed action's held writes were silently lost when another action resumed in the window — the transition merge moves every list except _pendingNodes, and the batch-adoption pass only sees the queue's batch (INV-7, solidjs#2827 class) - a bare optimistic write never reverted (INV-6) - an affects() mark could leak, leaving isPending stuck true (INV-10) Completing an action now goes through initTransition, the same merge-and-adopt path every other transition-resumption site already uses. With the batch adopted, the next action's initTransition also transfers and re-stamps the restored transition's pending nodes, closing the merge orphan without touching mergeTransitionState. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 82d61b4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
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 |
Merging this PR will not alter performance
Comparing Footnotes
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When an async-generator action completes,
done()restored the active transition with a baresetActiveTransition(ctx), leavingglobalQueue._batchas a detached ambient batch until the scheduled flush ran. Anything registered in that microtask window landed in a batch nothing would ever finalize (the same window as the #2916 race, whose fix only protected that batch's_pendingNodes):mergeTransitionStatemoves every list except_pendingNodes(relying on the batch-adoption pass ininitTransition, which only sees the queue's batch — here the detached ambient one), so the restored transition's held writes were orphaned. Dev invariant INV-7 ("value can never commit", fix(server): reset sync memo child state on re-pull so hydration keys stay aligned #2827 class)._optimisticNodes, which the completing flush never resolves and the end-of-flush reschedule check never sees. Dev invariant INV-6.affects()mark registered in the window could leak forever, leavingisPendingstucktruewith an explicitflush()as a no-op. Dev invariant INV-10.Fix
One line (plus a comment):
done()now goes throughglobalQueue.initTransition(ctx)— the same merge-and-adopt path every other transition-resumption site (async resolution, optimistic replay, zombie recompute) already uses. That adopts the ambient batch into the restored transition, so window registrations land somewhere that settles. With the batch adopted, the next action'sinitTransitionalso transfers and re-stamps the restored transition's pending nodes, which closes the merge orphan without touchingmergeTransitionState.One deliberate semantic alignment: an ordinary write landing after
done()but before the flush now joins the active transaction — the normal rule everywhere else a transaction is active — instead of staying ambient. The #2916 keep-the-ambient-batch branch influshstill passes its race test and remains as insurance.Tests
tests/action-done-window.test.tscovers all three symptoms by polling microtasks until the restored transition is observable and injecting the work exactly in the window. The first two fail deterministically onnextwithout the fix (INV-7 firing); the third symptom needs additional in-flight work to become observable but is guarded here as well. Full@solidjs/signalssuite (1,149 tests) andtest-typespass with the fix.🤖 Generated with Claude Code