perf: avoid re-traversing the effect tree after `$:` assignments (#17848)

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

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+perf: avoid re-traversing the effect tree after `$:` assignments

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<Batch>} */
 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;
 			}
 		}

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);
+		}
 	});
 }
 

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
  * `<Child bind:x={$y} />`. 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<V>} 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<V>} 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;
 }
 

packages/svelte/tests/runtime-legacy/samples/store-reschedule/Child.svelte

@@ -0,0 +1,9 @@
+<script lang="ts">
+	import { count } from './stores';
+
+	let n = 0;
+
+	$: $count = n;
+</script>
+
+<button onclick={() => n += 1}>{$count}</button>

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: `<button>0</button><button>0</button>`,
+
+	test({ assert, target }) {
+		const [button1, button2] = target.querySelectorAll('button');
+
+		flushSync(() => button1.click());
+		assert.htmlEqual(target.innerHTML, `<button>1</button><button>1</button>`);
+
+		flushSync(() => button1.click());
+		assert.htmlEqual(target.innerHTML, `<button>2</button><button>2</button>`);
+
+		flushSync(() => button2.click());
+		assert.htmlEqual(target.innerHTML, `<button>1</button><button>1</button>`);
+
+		flushSync(() => button2.click());
+		assert.htmlEqual(target.innerHTML, `<button>2</button><button>2</button>`);
+	}
+});

packages/svelte/tests/runtime-legacy/samples/store-reschedule/main.svelte

@@ -0,0 +1,6 @@
+<script>
+	import Child from './Child.svelte';
+</script>
+
+<Child />
+<Child />

packages/svelte/tests/runtime-legacy/samples/store-reschedule/stores.js

@@ -0,0 +1,3 @@
+import { writable } from 'svelte/store';
+
+export const count = writable(0);