From 2a1f5ada13e167ed82e44274ea45722bc640900b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 5 Mar 2026 21:36:03 -0500 Subject: [PATCH] perf: avoid re-traversing the effect tree after `$:` assignments (#17848) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If an assignment happens in a `$:` statement, any affected effects are rescheduled while the traversal is ongoing. But this is wasteful — it results in the `flush_effects` loop running another time, even though the affected effects are guaranteed to be visited _later_ in the traversal (unless the thing being updated is a store). This PR fixes it: inside a `legacy_pre_effect`, we temporarily pretend that the branch _containing_ the component with the `$:` statement is the `active_effect`, such that Svelte understands that any marked effects are about to be visited and thus don't need to be scheduled. We deal with the store case by temporarily pretending that there _is_ no `active_effect`. I will be delighted when we can rip all this legacy stuff out of the codebase. ### Before submitting the PR, please make sure you do the following - [x] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs - [x] Prefix your PR title with `feat:`, `fix:`, `chore:`, or `docs:`. - [x] This message body should clearly illustrate what problems it solves. - [ ] Ideally, include a test that fails without this PR but passes with it. - [x] If this PR changes code within `packages/svelte/src`, add a changeset (`npx changeset`). ### Tests and linting - [x] Run the tests with `pnpm test` and lint the project with `pnpm lint` --- .changeset/ninety-kings-attend.md | 5 ++++ .../src/internal/client/reactivity/batch.js | 14 +++++++-- .../src/internal/client/reactivity/effects.js | 17 +++++++++-- .../src/internal/client/reactivity/store.js | 29 ++++++++++++++++--- .../samples/store-reschedule/Child.svelte | 9 ++++++ .../samples/store-reschedule/_config.js | 22 ++++++++++++++ .../samples/store-reschedule/main.svelte | 6 ++++ .../samples/store-reschedule/stores.js | 3 ++ 8 files changed, 97 insertions(+), 8 deletions(-) create mode 100644 .changeset/ninety-kings-attend.md create mode 100644 packages/svelte/tests/runtime-legacy/samples/store-reschedule/Child.svelte create mode 100644 packages/svelte/tests/runtime-legacy/samples/store-reschedule/_config.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/store-reschedule/main.svelte create mode 100644 packages/svelte/tests/runtime-legacy/samples/store-reschedule/stores.js diff --git a/.changeset/ninety-kings-attend.md b/.changeset/ninety-kings-attend.md new file mode 100644 index 0000000000..40913dab67 --- /dev/null +++ b/.changeset/ninety-kings-attend.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +perf: avoid re-traversing the effect tree after `$:` assignments diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index a1cd08bd6a..638aba2fcd 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -22,6 +22,7 @@ import { async_mode_flag } from '../../flags/index.js'; import { deferred, define_property, includes } from '../../shared/utils.js'; import { active_effect, + active_reaction, get, increment_write_version, is_dirty, @@ -36,6 +37,7 @@ import { eager_effect, unlink_effect } from './effects.js'; import { defer_effect } from './utils.js'; import { UNINITIALIZED } from '../../../constants.js'; import { set_signal_status } from './status.js'; +import { legacy_is_updating_store } from './store.js'; /** @type {Set} */ const batches = new Set(); @@ -856,10 +858,18 @@ export function schedule_effect(signal) { // updated an internal source, or because a branch is being unskipped, // bail out or we'll cause a second flush if (collected_effects !== null && effect === active_effect) { + if (async_mode_flag) return; + // in sync mode, render effects run during traversal. in an extreme edge case + // — namely that we're setting a value inside a derived read during traversal — // they can be made dirty after they have already been visited, in which - // case we shouldn't bail out - if (async_mode_flag || (signal.f & RENDER_EFFECT) === 0) { + // case we shouldn't bail out. we also shouldn't bail out if we're + // updating a store inside a `$:`, since this might invalidate + // effects that were already visited + if ( + (active_reaction === null || (active_reaction.f & DERIVED) === 0) && + !legacy_is_updating_store + ) { return; } } diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index b3d37659ea..3118851277 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -10,7 +10,8 @@ import { set_active_reaction, set_is_destroying_effect, untrack, - untracking + untracking, + set_active_effect } from '../runtime.js'; import { DIRTY, @@ -316,7 +317,19 @@ export function legacy_pre_effect(deps, fn) { if (token.ran) return; token.ran = true; - untrack(fn); + + var effect = /** @type {Effect} */ (active_effect); + + // here, we lie: by setting `active_effect` to be the parent branch, any writes + // that happen inside `fn` will _not_ cause an unnecessary reschedule, because + // the affected effects will be children of `active_effect`. this is safe + // because these effects are known to run in the correct order + try { + set_active_effect(effect.parent); + untrack(fn); + } finally { + set_active_effect(effect); + } }); } diff --git a/packages/svelte/src/internal/client/reactivity/store.js b/packages/svelte/src/internal/client/reactivity/store.js index ce082866ce..7124e23db8 100644 --- a/packages/svelte/src/internal/client/reactivity/store.js +++ b/packages/svelte/src/internal/client/reactivity/store.js @@ -8,6 +8,12 @@ import { teardown } from './effects.js'; import { mutable_source, set } from './sources.js'; import { DEV } from 'esm-env'; +/** + * We set this to `true` when updating a store so that we correctly + * schedule effects if the update takes place inside a `$:` effect + */ +export let legacy_is_updating_store = false; + /** * Whether or not the prop currently being read is a store binding, as in * ``. If it is, we treat the prop as mutable even in @@ -102,7 +108,7 @@ export function store_unsub(store, store_name, stores) { * @returns {V} */ export function store_set(store, value) { - store.set(value); + update_with_flag(store, value); return value; } @@ -141,6 +147,21 @@ export function setup_stores() { return [stores, cleanup]; } +/** + * @param {Store} store + * @param {V} value + * @template V + */ +function update_with_flag(store, value) { + legacy_is_updating_store = true; + + try { + store.set(value); + } finally { + legacy_is_updating_store = false; + } +} + /** * Updates a store with a new value. * @param {Store} store the store to update @@ -149,7 +170,7 @@ export function setup_stores() { * @template V */ export function store_mutate(store, expression, new_value) { - store.set(new_value); + update_with_flag(store, new_value); return expression; } @@ -160,7 +181,7 @@ export function store_mutate(store, expression, new_value) { * @returns {number} */ export function update_store(store, store_value, d = 1) { - store.set(store_value + d); + update_with_flag(store, store_value + d); return store_value; } @@ -172,7 +193,7 @@ export function update_store(store, store_value, d = 1) { */ export function update_pre_store(store, store_value, d = 1) { const value = store_value + d; - store.set(value); + update_with_flag(store, value); return value; } diff --git a/packages/svelte/tests/runtime-legacy/samples/store-reschedule/Child.svelte b/packages/svelte/tests/runtime-legacy/samples/store-reschedule/Child.svelte new file mode 100644 index 0000000000..d955a82a88 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/store-reschedule/Child.svelte @@ -0,0 +1,9 @@ + + + diff --git a/packages/svelte/tests/runtime-legacy/samples/store-reschedule/_config.js b/packages/svelte/tests/runtime-legacy/samples/store-reschedule/_config.js new file mode 100644 index 0000000000..1c9ea0d5ea --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/store-reschedule/_config.js @@ -0,0 +1,22 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + html: ``, + + test({ assert, target }) { + const [button1, button2] = target.querySelectorAll('button'); + + flushSync(() => button1.click()); + assert.htmlEqual(target.innerHTML, ``); + + flushSync(() => button1.click()); + assert.htmlEqual(target.innerHTML, ``); + + flushSync(() => button2.click()); + assert.htmlEqual(target.innerHTML, ``); + + flushSync(() => button2.click()); + assert.htmlEqual(target.innerHTML, ``); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/store-reschedule/main.svelte b/packages/svelte/tests/runtime-legacy/samples/store-reschedule/main.svelte new file mode 100644 index 0000000000..55c1438411 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/store-reschedule/main.svelte @@ -0,0 +1,6 @@ + + + + diff --git a/packages/svelte/tests/runtime-legacy/samples/store-reschedule/stores.js b/packages/svelte/tests/runtime-legacy/samples/store-reschedule/stores.js new file mode 100644 index 0000000000..d432d339ec --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/store-reschedule/stores.js @@ -0,0 +1,3 @@ +import { writable } from 'svelte/store'; + +export const count = writable(0);