Repository navigation
fix(signals): a cleanup write during a landing no longer strands later transitions (#4089) - #4092
Merged
Merged
Conversation
…r transitions (#4089) `settle` removes a landing transaction from `transactions` before `land` commits its nodes. A commit that disposes a re-run memo's old children runs their cleanups, and a cleanup that wrote a node the landing still held joined that transaction (`joinFuture(txOf(el))`): `flushTransaction` became the dead transaction, the next flush parked in it, and every later transition held its writes there forever. The write is the landing's own (`land` commits the node after it), so `land` drops the stale join. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018fTNh1FgQx2WTF5xoxwML1
🦋 Changeset detectedLatest commit: fbcf93d The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 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 |
Size (brotli, eager entry chunk)
|
Coverage Report for CI Build 38706763244Coverage remained the same at 77.548%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Merging this PR will not alter performance
|
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
Fixes #4089.
Before: since rc.14, a signal write from an
onCleanupthat runs while a transition lands could leave the landed transaction as the next flush's transaction. Every later transition then parked in it and never committed. With@solidjs/routerand@tanstack/solid-query, this showed up as a navigation that never finishes.After: the write lands with the transition, and later transitions commit. The issue's repro prints
2 12again, as on rc.13.How:
settleremoves a transaction fromtransactionsbeforelandcommits its nodes. A commit that disposes a re-run memo's old children runs their cleanups. A cleanup that writes a node the landing still holds callsjoinFuture(txOf(el)), which setsflushTransactionto the transaction that just left the list. The write already belongs to the landing, becauselandcommits that node right after. So when its commits are done,landclearsflushTransactionif it still points at the landing transaction:This costs +16 B minified, where the guard in
joinFuturesuggested in the issue costs about +53 B. One rarer case stays uncovered. If a single landing's cleanups first write a node held by another live transaction and then one held by the landing itself, the landing merges into the live transaction. I left that out to keep the core floor's size down.How did you test this change?
packages/signals/tests/cleanup-write-during-landing-4089.test.tsuses the issue's repro. It fails onnext([1, 11]) and passes with the fix ([2, 12]). A baseline case without the cleanup write still gives[2, 10].🤖 Generated with Claude Code
https://claude.ai/code/session_018fTNh1FgQx2WTF5xoxwML1
Generated by Claude Code