From 53417ea8f785f2ec884b095ac1bacd08aa30da86 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 24 Jul 2025 01:48:32 -0400 Subject: [PATCH 1/3] fix: preserve dirty status of deferred effects (#16487) * don't mark_reactions inside decrement, it can cause infinite loops * revert mark_reactions changes * preserve DIRTY/MAYBE_DIRTY status of deferred effects * changeset * tweak --- .changeset/cool-insects-argue.md | 5 ++ .../src/internal/client/reactivity/batch.js | 63 ++++++++++++++----- .../src/internal/client/reactivity/sources.js | 9 +-- .../async-effect-conservative/_config.js | 28 +++++++++ .../async-effect-conservative/main.svelte | 17 +++++ 5 files changed, 101 insertions(+), 21 deletions(-) create mode 100644 .changeset/cool-insects-argue.md 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/.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 diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index ce413fa1e186..89bad947c7fa 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'; @@ -27,7 +28,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'; @@ -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) { @@ -271,15 +283,15 @@ export class Batch { if (!skip && effect.fn !== null) { if (is_branch) { effect.f ^= CLEAN; - } else if ((flags & EFFECT) !== 0) { - this.#effects.push(effect); - } 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 & CLEAN) === 0) { + if ((flags & EFFECT) !== 0) { + this.#effects.push(effect); + } else if (async_mode_flag && (flags & RENDER_EFFECT) !== 0) { + this.#render_effects.push(effect); + } 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); } @@ -303,6 +315,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 @@ -380,8 +407,14 @@ 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.#dirty_effects) { + set_signal_status(e, DIRTY); + schedule_effect(e); + } + + for (const e of this.#maybe_dirty_effects) { + set_signal_status(e, MAYBE_DIRTY); + schedule_effect(e); } this.#render_effects = []; 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)); } } 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 46d3261ed9a7d6acb9908f52e989013795555a56 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Thu, 24 Jul 2025 11:39:54 +0200 Subject: [PATCH 2/3] chore: don't do effect scheduling unnecessarily (#16489) mini-cleanup post #16487 - we don't need to do the work of scheduling an effect that's already dirty which means it already scheduled its root effect to run --- packages/svelte/src/internal/client/reactivity/sources.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 7b5198542a4d..3b2087d56bc9 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -323,14 +323,16 @@ function mark_reactions(signal, status) { continue; } + var not_dirty = (flags & DIRTY) === 0; + // don't set a DIRTY reaction to MAYBE_DIRTY - if ((flags & DIRTY) === 0) { + if (not_dirty) { set_signal_status(reaction, status); } if ((flags & DERIVED) !== 0) { mark_reactions(/** @type {Derived} */ (reaction), MAYBE_DIRTY); - } else { + } else if (not_dirty) { schedule_effect(/** @type {Effect} */ (reaction)); } } From 4e74cd35fedf55e65b60c957041f79ca94dfc197 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 24 Jul 2025 11:43:28 +0200 Subject: [PATCH 3/3] Version Packages (#16488) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .changeset/cool-insects-argue.md | 5 ----- packages/svelte/CHANGELOG.md | 6 ++++++ packages/svelte/package.json | 2 +- packages/svelte/src/version.js | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) delete mode 100644 .changeset/cool-insects-argue.md diff --git a/.changeset/cool-insects-argue.md b/.changeset/cool-insects-argue.md deleted file mode 100644 index ff1c520b7bb0..000000000000 --- a/.changeset/cool-insects-argue.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'svelte': patch ---- - -fix: preserve dirty status of deferred effects diff --git a/packages/svelte/CHANGELOG.md b/packages/svelte/CHANGELOG.md index 8cd385046041..5bffa5f70ef3 100644 --- a/packages/svelte/CHANGELOG.md +++ b/packages/svelte/CHANGELOG.md @@ -1,5 +1,11 @@ # svelte +## 5.36.15 + +### Patch Changes + +- fix: preserve dirty status of deferred effects ([#16487](https://github.com/sveltejs/svelte/pull/16487)) + ## 5.36.14 ### Patch Changes diff --git a/packages/svelte/package.json b/packages/svelte/package.json index 7fe1d161f288..051f82ec3aad 100644 --- a/packages/svelte/package.json +++ b/packages/svelte/package.json @@ -2,7 +2,7 @@ "name": "svelte", "description": "Cybernetically enhanced web apps", "license": "MIT", - "version": "5.36.14", + "version": "5.36.15", "type": "module", "types": "./types/index.d.ts", "engines": { diff --git a/packages/svelte/src/version.js b/packages/svelte/src/version.js index cd9d8b459c32..1d469f29b0f6 100644 --- a/packages/svelte/src/version.js +++ b/packages/svelte/src/version.js @@ -4,5 +4,5 @@ * The current version, as set in package.json. * @type {string} */ -export const VERSION = '5.36.14'; +export const VERSION = '5.36.15'; export const PUBLIC_VERSION = '5';