mirror of https://github.com/sveltejs/svelte
fix: end a restored reaction context at the end of its synchronous segment (#18694)
When an async expression resumes after a pickled `await`, the thunk returned by `save()` in `reactivity/async.js` calls `restore()` to re-arm `active_reaction` for the rest of the expression, then disarms it with `queue_micro_task(unset_context)`. Any microtask already queued before that one runs inside the restored context. If it writes to a source, `set()` throws `state_unsafe_mutation` in 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 `save` thunk now ends with the synchronous segment it was restored in. A `restored` flag is set by the thunk and consumed on entry to `save` and `track_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_await` on `ExpressionMetadata`), so a trailing await compiles to `$.save` rather than a bare `await`. At the end of the body, `async_thunk` in `3-transform/client/utils.js` wraps the return expression in `$.unsave(...)` when the metadata has a pickled await. If the body throws instead, the context is unset by `async_derived`'s existing `finally`, as before. The queued microtask in `save` is 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:compare` shows 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 `await` shape, and fail on `main`. --------- Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>pull/18702/head
Nic Polumeyv
3 weeks ago
committed by
GitHub
parent
1be496f0a6
commit
f2648b3537
@ -0,0 +1,5 @@
|
||||
---
|
||||
'svelte': patch
|
||||
---
|
||||
|
||||
fix: end a restored reaction context at the end of its synchronous segment
|
||||
@ -0,0 +1,20 @@
|
||||
import { tick } from 'svelte';
|
||||
import { test } from '../../test';
|
||||
|
||||
export default test({
|
||||
mode: ['client'],
|
||||
compileOptions: {
|
||||
dev: true
|
||||
},
|
||||
|
||||
async test({ assert, target, errors }) {
|
||||
await new Promise((resolve) => setTimeout(resolve, 20));
|
||||
await tick();
|
||||
|
||||
assert.deepEqual(
|
||||
errors.filter((error) => error.includes('state_unsafe_mutation')),
|
||||
[]
|
||||
);
|
||||
assert.htmlEqual(target.innerHTML, '<p>pending</p><p>1</p>');
|
||||
}
|
||||
});
|
||||
@ -0,0 +1,14 @@
|
||||
<script>
|
||||
let foreign = $state(0);
|
||||
const input = Promise.resolve({ pending: new Promise(() => {}) });
|
||||
|
||||
setTimeout(() => {
|
||||
foreign += 1;
|
||||
});
|
||||
</script>
|
||||
|
||||
{#await (await input).pending}
|
||||
<p>pending</p>
|
||||
{/await}
|
||||
|
||||
<p>{foreign}</p>
|
||||
@ -0,0 +1,22 @@
|
||||
import { tick } from 'svelte';
|
||||
import { test } from '../../test';
|
||||
|
||||
export default test({
|
||||
mode: ['client'],
|
||||
|
||||
compileOptions: {
|
||||
dev: false
|
||||
},
|
||||
|
||||
async test({ assert, target, errors }) {
|
||||
await tick();
|
||||
await tick();
|
||||
await tick();
|
||||
|
||||
assert.deepEqual(
|
||||
errors.filter((error) => error.includes('state_unsafe_mutation')),
|
||||
[]
|
||||
);
|
||||
assert.htmlEqual(target.innerHTML, '<p>4 1</p>');
|
||||
}
|
||||
});
|
||||
@ -0,0 +1,20 @@
|
||||
<script>
|
||||
let foreign = $state(0);
|
||||
|
||||
const items = Promise.resolve([1, 2, 3]);
|
||||
const one = Promise.resolve(1);
|
||||
|
||||
// lands the write while the derived is suspended on `await one`, after the
|
||||
// context restored for `.length` — in production that await has no dev hook
|
||||
items.then(() => {
|
||||
queueMicrotask(() => {
|
||||
queueMicrotask(() => {
|
||||
foreign += 1;
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
const total = $derived((await items).length + (await one));
|
||||
</script>
|
||||
|
||||
<p>{total} {foreign}</p>
|
||||
@ -0,0 +1,21 @@
|
||||
import { tick } from 'svelte';
|
||||
import { test } from '../../test';
|
||||
|
||||
export default test({
|
||||
mode: ['client'],
|
||||
|
||||
compileOptions: {
|
||||
dev: true
|
||||
},
|
||||
|
||||
async test({ assert, target, errors }) {
|
||||
await tick();
|
||||
await tick();
|
||||
|
||||
assert.deepEqual(
|
||||
errors.filter((error) => error.includes('state_unsafe_mutation')),
|
||||
[]
|
||||
);
|
||||
assert.htmlEqual(target.innerHTML, '<p>3 1</p>');
|
||||
}
|
||||
});
|
||||
@ -0,0 +1,21 @@
|
||||
<script>
|
||||
let foreign = $state(0);
|
||||
|
||||
const items = Promise.resolve([1, 2, 3]);
|
||||
|
||||
// registered before the derived, so it runs first when `items` settles.
|
||||
// Two nested microtasks land the write in the window between the compiled
|
||||
// continuation restoring the derived's context and svelte's queued unset
|
||||
items.then(() => {
|
||||
queueMicrotask(() => {
|
||||
queueMicrotask(() => {
|
||||
foreign += 1;
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// `.length` follows the await, so the compiler pickles it via `$.save`
|
||||
const length = $derived((await items).length);
|
||||
</script>
|
||||
|
||||
<p>{length} {foreign}</p>
|
||||
@ -0,0 +1,20 @@
|
||||
import { tick } from 'svelte';
|
||||
import { test } from '../../test';
|
||||
|
||||
export default test({
|
||||
mode: ['client'],
|
||||
compileOptions: {
|
||||
dev: true
|
||||
},
|
||||
|
||||
async test({ assert, target, errors }) {
|
||||
await tick();
|
||||
await tick();
|
||||
|
||||
assert.deepEqual(
|
||||
errors.filter((error) => error.includes('state_unsafe_mutation')),
|
||||
[]
|
||||
);
|
||||
assert.htmlEqual(target.innerHTML, '<p>failed</p><p>1</p>');
|
||||
}
|
||||
});
|
||||
@ -0,0 +1,26 @@
|
||||
<script>
|
||||
let foreign = $state(0);
|
||||
const input = Promise.resolve({
|
||||
get value() {
|
||||
throw new Error('boom');
|
||||
}
|
||||
});
|
||||
|
||||
input.then(() => {
|
||||
queueMicrotask(() => {
|
||||
queueMicrotask(() => {
|
||||
foreign += 1;
|
||||
});
|
||||
});
|
||||
});
|
||||
</script>
|
||||
|
||||
<svelte:boundary onerror={() => {}}>
|
||||
{#snippet failed()}
|
||||
<p>failed</p>
|
||||
{/snippet}
|
||||
|
||||
<p>{(await input).value}</p>
|
||||
</svelte:boundary>
|
||||
|
||||
<p>{foreign}</p>
|
||||
Loading…
Reference in new issue