fix: end a restored reaction context at the end of its synchronous segment - #18694
Open
Nic-Polumeyv wants to merge 4 commits into
Open
fix: end a restored reaction context at the end of its synchronous segment#18694Nic-Polumeyv wants to merge 4 commits into
Nic-Polumeyv wants to merge 4 commits into
Conversation
|
Install the latest version of pnpm add https://pkg.svelte.dev/svelte/c/23298d5e711d9ddf18dfcec509b7e3709322863bOpen in |
🦋 Changeset detectedLatest commit: 23298d5 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 |
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.
When an async expression resumes after a pickled
await, the thunk returned bysave()inreactivity/async.jscallsrestore()to re-armactive_reactionfor the rest of the expression, then disarms it withqueue_micro_task(unset_context). Any microtask already queued before that one runs inside the restored context. If it writes to a source,set()throwsstate_unsafe_mutationin production, since the guard is not dev-only. #18453 introduced the queued disarm and noted this case in review as unavoidable. SvelteKit hits it in practice: its fetch continuations write to internal$state(sveltejs/kit#16914), and a user's$derived((await q()).length)resuming in the same tick makes that write throw and drops the update signal.The context restored by a
savethunk now ends with the synchronous segment it was restored in. Arestoredflag is set by the thunk and consumed on entry tosaveandtrack_reactivity_loss, so every suspension ends it; once an expression contains a pickled await, the analysis pickles every later await in it too (has_pickled_awaitonExpressionMetadata), so a trailing await compiles to$.saverather than a bareawait. At the end of the body,async_thunkin3-transform/client/utils.jswraps the return expression in$.unsave(...)when the metadata has a pickled await. If the body throws instead, the context is unset byasync_derived's existingfinally, as before. The queued microtask insaveis removed.Output is unchanged for expressions that pickle nothing (
$derived(await a)compiles byte for byte the same). Expressions with a pickled await gain one$.unsave(call per body, and their trailing await becomes a$.save, 4 to 6 bytes gzipped in the added tests. At runtime a boolean write replaces a queued microtask per resume.bench:compareshows no difference outside run-to-run noise.Two runtime tests reproduce the throw without any library involved, one in dev and one with the prod
awaitshape, and fail onmain.