From 2d089ebe0ade37357e6bcd6a0bbf055a23066b63 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 24 Jul 2025 00:31:39 -0400 Subject: [PATCH 1/5] don't mark_reactions inside decrement, it can cause infinite loops --- .../src/internal/client/reactivity/batch.js | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index ce413fa1e186..29d4d073f9ac 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -272,9 +272,13 @@ export class Batch { if (is_branch) { effect.f ^= CLEAN; } else if ((flags & EFFECT) !== 0) { - this.#effects.push(effect); + if ((flags & CLEAN) === 0) { + this.#effects.push(effect); + } } else if (async_mode_flag && (flags & RENDER_EFFECT) !== 0) { - this.#render_effects.push(effect); + if ((flags & CLEAN) === 0) { + this.#render_effects.push(effect); + } } else if (is_dirty(effect)) { if ((flags & ASYNC) !== 0) { var effects = effect.b?.pending ? this.#boundary_async_effects : this.#async_effects; @@ -380,8 +384,19 @@ export class Batch { this.#pending -= 1; if (this.#pending === 0) { - for (const source of this.current.keys()) { - mark_reactions(source, DIRTY, false); + for (const e of this.#render_effects) { + set_signal_status(e, DIRTY); + schedule_effect(e); + } + + for (const e of this.#effects) { + set_signal_status(e, DIRTY); + schedule_effect(e); + } + + for (const e of this.#block_effects) { + set_signal_status(e, DIRTY); + schedule_effect(e); } this.#render_effects = []; From a826136d74015bbd91124e121315afca96dacc5f Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 24 Jul 2025 00:33:40 -0400 Subject: [PATCH 2/5] revert mark_reactions changes --- packages/svelte/src/internal/client/reactivity/batch.js | 2 +- .../svelte/src/internal/client/reactivity/sources.js | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 29d4d073f9ac..52def2a71542 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -27,7 +27,7 @@ import * as e from '../errors.js'; import { flush_tasks } from '../dom/task.js'; import { DEV } from 'esm-env'; import { invoke_error_boundary } from '../error-handling.js'; -import { mark_reactions, old_values } from './sources.js'; +import { old_values } from './sources.js'; import { unlink_effect } from './effects.js'; import { unset_context } from './async.js'; diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 3b28c8fdceeb..7b5198542a4d 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -301,10 +301,9 @@ export function increment(source) { /** * @param {Value} signal * @param {number} status should be DIRTY or MAYBE_DIRTY - * @param {boolean} schedule_async * @returns {void} */ -export function mark_reactions(signal, status, schedule_async = true) { +function mark_reactions(signal, status) { var reactions = signal.reactions; if (reactions === null) return; @@ -324,16 +323,14 @@ export function mark_reactions(signal, status, schedule_async = true) { continue; } - var should_schedule = (flags & DIRTY) === 0 && (schedule_async || (flags & ASYNC) === 0); - // don't set a DIRTY reaction to MAYBE_DIRTY - if (should_schedule) { + if ((flags & DIRTY) === 0) { set_signal_status(reaction, status); } if ((flags & DERIVED) !== 0) { mark_reactions(/** @type {Derived} */ (reaction), MAYBE_DIRTY); - } else if (should_schedule) { + } else { schedule_effect(/** @type {Effect} */ (reaction)); } } From 0134d04c16928a6745ef7a0486e43689da0ed111 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 24 Jul 2025 00:51:34 -0400 Subject: [PATCH 3/5] preserve DIRTY/MAYBE_DIRTY status of deferred effects --- .../src/internal/client/reactivity/batch.js | 48 ++++++++++++++----- .../async-effect-conservative/_config.js | 28 +++++++++++ .../async-effect-conservative/main.svelte | 17 +++++++ 3 files changed, 80 insertions(+), 13 deletions(-) create mode 100644 packages/svelte/tests/runtime-runes/samples/async-effect-conservative/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/async-effect-conservative/main.svelte diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 52def2a71542..f1f20164e3ce 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -10,7 +10,8 @@ import { INERT, RENDER_EFFECT, ROOT_EFFECT, - USER_EFFECT + USER_EFFECT, + MAYBE_DIRTY } from '#client/constants'; import { async_mode_flag } from '../../flags/index.js'; import { deferred, define_property } from '../../shared/utils.js'; @@ -146,6 +147,18 @@ export class Batch { */ #block_effects = []; + /** + * Deferred effects (which run after async work has completed) that are DIRTY + * @type {Effect[]} + */ + #dirty_effects = []; + + /** + * Deferred effects that are MAYBE_DIRTY + * @type {Effect[]} + */ + #maybe_dirty_effects = []; + /** * A set of branches that still exist, but will be destroyed when this batch * is committed — we skip over these during `process` @@ -221,10 +234,9 @@ export class Batch { this.#deferred?.resolve(); } else { - // otherwise mark effects clean so they get scheduled on the next run - for (const e of this.#render_effects) set_signal_status(e, CLEAN); - for (const e of this.#effects) set_signal_status(e, CLEAN); - for (const e of this.#block_effects) set_signal_status(e, CLEAN); + this.#defer_effects(this.#render_effects); + this.#defer_effects(this.#effects); + this.#defer_effects(this.#block_effects); } if (current_values) { @@ -307,6 +319,21 @@ export class Batch { } } + /** + * @param {Effect[]} effects + */ + #defer_effects(effects) { + for (const e of effects) { + const target = (e.f & DIRTY) !== 0 ? this.#dirty_effects : this.#maybe_dirty_effects; + target.push(e); + + // mark as clean so they get scheduled if they depend on pending async state + set_signal_status(e, CLEAN); + } + + effects.length = 0; + } + /** * Associate a change to a given source with the current * batch, noting its previous and current values @@ -384,18 +411,13 @@ export class Batch { this.#pending -= 1; if (this.#pending === 0) { - for (const e of this.#render_effects) { + for (const e of this.#dirty_effects) { set_signal_status(e, DIRTY); schedule_effect(e); } - for (const e of this.#effects) { - set_signal_status(e, DIRTY); - schedule_effect(e); - } - - for (const e of this.#block_effects) { - set_signal_status(e, DIRTY); + for (const e of this.#maybe_dirty_effects) { + set_signal_status(e, MAYBE_DIRTY); schedule_effect(e); } diff --git a/packages/svelte/tests/runtime-runes/samples/async-effect-conservative/_config.js b/packages/svelte/tests/runtime-runes/samples/async-effect-conservative/_config.js new file mode 100644 index 000000000000..bab06a203d1c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-effect-conservative/_config.js @@ -0,0 +1,28 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target, logs }) { + await tick(); + + const [increment] = target.querySelectorAll('button'); + + assert.deepEqual(logs, [false]); + assert.htmlEqual(target.innerHTML, '

0

'); + + increment.click(); + await tick(); + assert.deepEqual(logs, [false]); + assert.htmlEqual(target.innerHTML, '

1

'); + + increment.click(); + await tick(); + assert.deepEqual(logs, [false, true]); + assert.htmlEqual(target.innerHTML, '

2

'); + + increment.click(); + await tick(); + assert.deepEqual(logs, [false, true]); + assert.htmlEqual(target.innerHTML, '

3

'); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-effect-conservative/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-effect-conservative/main.svelte new file mode 100644 index 000000000000..5305067a5ad9 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-effect-conservative/main.svelte @@ -0,0 +1,17 @@ + + + + +

{await count}

+ + {#snippet pending()} +

loading...

+ {/snippet} + From 86db0aa3238db453a46d957d1da35dce0fcb87d2 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 24 Jul 2025 00:53:14 -0400 Subject: [PATCH 4/5] changeset --- .changeset/cool-insects-argue.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/cool-insects-argue.md diff --git a/.changeset/cool-insects-argue.md b/.changeset/cool-insects-argue.md new file mode 100644 index 000000000000..ff1c520b7bb0 --- /dev/null +++ b/.changeset/cool-insects-argue.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: preserve dirty status of deferred effects From 51f24a333c4ff650d0724ca7ee099956cfb08fd0 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 24 Jul 2025 01:20:20 -0400 Subject: [PATCH 5/5] tweak --- .../svelte/src/internal/client/reactivity/batch.js | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index f1f20164e3ce..89bad947c7fa 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -283,19 +283,15 @@ export class Batch { if (!skip && effect.fn !== null) { if (is_branch) { effect.f ^= CLEAN; - } else if ((flags & EFFECT) !== 0) { - if ((flags & CLEAN) === 0) { + } else if ((flags & CLEAN) === 0) { + if ((flags & EFFECT) !== 0) { this.#effects.push(effect); - } - } else if (async_mode_flag && (flags & RENDER_EFFECT) !== 0) { - if ((flags & CLEAN) === 0) { + } else if (async_mode_flag && (flags & RENDER_EFFECT) !== 0) { this.#render_effects.push(effect); - } - } else if (is_dirty(effect)) { - if ((flags & ASYNC) !== 0) { + } else if ((flags & ASYNC) !== 0) { var effects = effect.b?.pending ? this.#boundary_async_effects : this.#async_effects; effects.push(effect); - } else { + } else if (is_dirty(effect)) { if ((effect.f & BLOCK_EFFECT) !== 0) this.#block_effects.push(effect); update_effect(effect); }