fix: $effect.pending() should not be affected by uncommitted forks - #18651
Open
waterWang wants to merge 1 commit into
Open
fix: $effect.pending() should not be affected by uncommitted forks#18651waterWang wants to merge 1 commit into
waterWang wants to merge 1 commit into
Conversation
|
Install the latest version of pnpm add https://pkg.svelte.dev/svelte/c/29f4cc7348036a7338e49e48c66256e1eb9f2a26Open in Note This PR is from a fork. A maintainer must approve approve each commit before it can be built and installed. |
🦋 Changeset detectedLatest commit: 29f4cc7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
dummdidumm
requested changes
Aug 20, 2026
dummdidumm
left a comment
Member
There was a problem hiding this comment.
Thank you, but this solution is incorrect. If the fork commits before the async operation has finished, $effect.pending() will be lower than it should be. Example:
<script>
import { fork } from 'svelte';
let count = $state(0);
function delay(v) {
return new Promise(r => setTimeout(r, 2000,v))
}
</script>
{await delay(count)}
{$effect.pending()}
<button onclick={() => {
const f = fork(() => count++);
setTimeout(() => f.commit(), 1000);
}}>i</button>does not show 0 1 after 1 second, just 1 0 after 2.
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 #18649
$effect.pending()(and boundary pending snippets) should not be affected by async work that happens inside an uncommittedfork(). The fork's work is speculative — it hasn't been committed yet, so it must not surface as pending state. This mirrors how$state.eager()is already unaffected by uncommitted forks.Root cause
In
increment_pending()(packages/svelte/src/internal/client/reactivity/async.js), the boundary's pending count was updated unconditionally:When a promise is created inside a fork (e.g. an async expression that re-evaluates because state changed within the fork),
increment_pending()ran withcurrent_batch.is_fork === trueand still bumped the boundary's pending count — even though the fork had not been committed, and the DOM had not (and might never) be updated.Fix
Capture the
is_forkstate at the timeincrement_pending()runs and skip the boundary update when it's an uncommitted fork:The batch's own pending count is still updated (
batch.increment/decrement), sofork().commit()can still await the async work as before. Only the boundary-visible pending state (which powers$effect.pending()and the{#snippet pending()}block) is skipped while the fork is uncommitted.Tests
async-fork-effect-pending: asserts that$effect.pending()stays0(and no pending snippet is shown) when a fork is started but not committed.runtime-runes+runtime-legacysuites pass: 5966 passed, 0 failed.