From 489ccc0a6dd52dd3f21e6135e60f3f86410df601 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 13 Oct 2025 16:35:29 +0200 Subject: [PATCH 1/3] fix: unset context on stale promises When a stale promise is rejected in `async_derived`, and the promise eventually resolves, `d.resolve` will be noop and `d.promise.then(handler, ...)` will never run. That in turns means any restored context (via `(await save(..))()`) will never be unset. We have to handle this case and unset the context to prevent errors such as false-positive state mutation errors --- .changeset/major-beans-fry.md | 5 +++ .../internal/client/reactivity/deriveds.js | 22 +++++++++-- .../samples/async-resolve-stale/_config.js | 26 +++++++++++++ .../samples/async-resolve-stale/main.svelte | 38 +++++++++++++++++++ 4 files changed, 87 insertions(+), 4 deletions(-) create mode 100644 .changeset/major-beans-fry.md create mode 100644 packages/svelte/tests/runtime-runes/samples/async-resolve-stale/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/async-resolve-stale/main.svelte diff --git a/.changeset/major-beans-fry.md b/.changeset/major-beans-fry.md new file mode 100644 index 000000000000..8f35683cd623 --- /dev/null +++ b/.changeset/major-beans-fry.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: unset context on stale promises diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 5d5976a6c115..8a1f4666ec35 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -113,20 +113,30 @@ export function async_derived(fn, location) { // only suspend in async deriveds created on initialisation var should_suspend = !active_reaction; - /** @type {Map>>} */ + /** @type {Map> & { rejected?: boolean }>} */ var deferreds = new Map(); async_effect(() => { if (DEV) current_async_effect = active_effect; - /** @type {ReturnType>} */ + /** @type {ReturnType> & { rejected?: boolean }} */ var d = deferred(); promise = d.promise; try { // If this code is changed at some point, make sure to still access the then property // of fn() to read any signals it might access, so that we track them as dependencies. - Promise.resolve(fn()).then(d.resolve, d.reject); + Promise.resolve(fn()).then((v) => { + if (d.rejected) { + // If we rejected this stale promise, d.resolve + // is a noop (d.promise.then(handler) below will never run). + // In this case we need to unset the restored context here + // to avoid leaking it (and e.g. cause false-positive mutation errors). + unset_context(); + } else { + d.resolve(v); + } + }, d.reject); } catch (error) { d.reject(error); } @@ -141,7 +151,11 @@ export function async_derived(fn, location) { if (!pending) { batch.increment(); - deferreds.get(batch)?.reject(STALE_REACTION); + var previous_deferred = deferreds.get(batch); + if (previous_deferred) { + previous_deferred.rejected = true; + previous_deferred.reject(STALE_REACTION); + } deferreds.set(batch, d); } } diff --git a/packages/svelte/tests/runtime-runes/samples/async-resolve-stale/_config.js b/packages/svelte/tests/runtime-runes/samples/async-resolve-stale/_config.js new file mode 100644 index 000000000000..bccf12562ad3 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-resolve-stale/_config.js @@ -0,0 +1,26 @@ +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + // We gotta wait a bit more in this test because of the macrotasks in App.svelte + function macrotask(t = 3) { + return new Promise((r) => setTimeout(r, t)); + } + + await macrotask(); + assert.htmlEqual(target.innerHTML, ' 1 | '); + + const [input] = target.querySelectorAll('input'); + + input.value = '1'; + input.dispatchEvent(new Event('input', { bubbles: true })); + await macrotask(); + assert.htmlEqual(target.innerHTML, ' 1 | '); + + input.value = '12'; + input.dispatchEvent(new Event('input', { bubbles: true })); + await macrotask(6); + // TODO this is wrong (separate bug), this should be 3 | 12 + assert.htmlEqual(target.innerHTML, ' 5 | 12'); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-resolve-stale/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-resolve-stale/main.svelte new file mode 100644 index 000000000000..945d52ba5dc9 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-resolve-stale/main.svelte @@ -0,0 +1,38 @@ + + + + +{count} | {x} \ No newline at end of file From e1e04d0315e62e6b73dda4a624549d7e4b93460d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 13 Oct 2025 16:23:07 -0400 Subject: [PATCH 2/3] fix: unset context on stale promises (slightly different approach) (#16936) * slightly different approach to #16935 * move unset_context call --- .../internal/client/reactivity/deriveds.js | 26 +++++-------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/deriveds.js b/packages/svelte/src/internal/client/reactivity/deriveds.js index 8a1f4666ec35..076a91923680 100644 --- a/packages/svelte/src/internal/client/reactivity/deriveds.js +++ b/packages/svelte/src/internal/client/reactivity/deriveds.js @@ -113,32 +113,24 @@ export function async_derived(fn, location) { // only suspend in async deriveds created on initialisation var should_suspend = !active_reaction; - /** @type {Map> & { rejected?: boolean }>} */ + /** @type {Map>>} */ var deferreds = new Map(); async_effect(() => { if (DEV) current_async_effect = active_effect; - /** @type {ReturnType> & { rejected?: boolean }} */ + /** @type {ReturnType>} */ var d = deferred(); promise = d.promise; try { // If this code is changed at some point, make sure to still access the then property // of fn() to read any signals it might access, so that we track them as dependencies. - Promise.resolve(fn()).then((v) => { - if (d.rejected) { - // If we rejected this stale promise, d.resolve - // is a noop (d.promise.then(handler) below will never run). - // In this case we need to unset the restored context here - // to avoid leaking it (and e.g. cause false-positive mutation errors). - unset_context(); - } else { - d.resolve(v); - } - }, d.reject); + // We call `unset_context` to undo any `save` calls that happen inside `fn()` + Promise.resolve(fn()).then(d.resolve, d.reject).then(unset_context); } catch (error) { d.reject(error); + unset_context(); } if (DEV) current_async_effect = null; @@ -151,11 +143,7 @@ export function async_derived(fn, location) { if (!pending) { batch.increment(); - var previous_deferred = deferreds.get(batch); - if (previous_deferred) { - previous_deferred.rejected = true; - previous_deferred.reject(STALE_REACTION); - } + deferreds.get(batch)?.reject(STALE_REACTION); deferreds.set(batch, d); } } @@ -199,8 +187,6 @@ export function async_derived(fn, location) { boundary.update_pending_count(-1); if (!pending) batch.decrement(); } - - unset_context(); }; d.promise.then(handler, (e) => handler(null, e || 'unknown')); From 2c0304ffec984a301d4110aaff82de21b5b5e92d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 13 Oct 2025 16:26:57 -0400 Subject: [PATCH 3/3] get rid of logs --- .../samples/async-resolve-stale/main.svelte | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/svelte/tests/runtime-runes/samples/async-resolve-stale/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-resolve-stale/main.svelte index 945d52ba5dc9..dc4a157928a3 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-resolve-stale/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/async-resolve-stale/main.svelte @@ -7,26 +7,22 @@ const r = Promise.withResolvers(); if (prev || v === '') { - console.log('hello', !!prev) Promise.resolve().then(async () => { - console.log('count++') count++; r.resolve(v); await new Promise(r => setTimeout(r, 0)); // TODO with a microtask like below it still throws a mutation error // await Promise.resolve(); prev?.resolve(); - }) + }); } else { - console.log('other') prev = Promise.withResolvers(); prev.promise.then(() => { - console.log('other coun++') count++; r.resolve(v) - }) + }); } - + return r.promise; } @@ -35,4 +31,4 @@ -{count} | {x} \ No newline at end of file +{count} | {x}