fix: prevent reactive statement reruns (#10736)

- run reactive statements only once per tick, even when they have indirect cyclic dependencies. Made possible by adding an array to the component context, which is filled with identifiers of the reactive statements, and which is cleared after all have run. Reactive statements rerunning before that will bail early if they detect they're still in the list
- part of the solution is to run all reactive statements, and then all render effects, which also fixes #10597

Author: dummdidumm

.changeset/moody-houses-argue.md

@@ -0,0 +1,5 @@
+---
+"svelte": patch
+---
+
+fix: prevent reactive statement reruns when they have indirect cyclic dependencies

packages/svelte/src/compiler/phases/3-transform/client/transform-client.js

@@ -214,6 +214,10 @@ export function client_component(source, analysis, options) {
 		instance.body.push(statement[1]);
 	}
 
+	if (analysis.reactive_statements.size > 0) {
+		instance.body.push(b.stmt(b.call('$.legacy_pre_effect_reset')));
+	}
+
 	/**
 	 * Used to store the group nodes
 	 * @type {import('estree').VariableDeclaration[]}

packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-legacy.js

@@ -157,7 +157,6 @@ export const javascript_visitors_legacy = {
 		}
 
 		const body = serialized_body.body;
-		const new_body = [];
 
 		/** @type {import('estree').Expression[]} */
 		const sequence = [];
@@ -176,18 +175,16 @@ export const javascript_visitors_legacy = {
 			sequence.push(serialized);
 		}
 
-		if (sequence.length > 0) {
-			new_body.push(b.stmt(b.sequence(sequence)));
-		}
-
-		new_body.push(b.stmt(b.call('$.untrack', b.thunk(b.block(body)))));
-
-		serialized_body.body = new_body;
-
 		// these statements will be topologically ordered later
 		state.legacy_reactive_statements.set(
 			node,
-			b.stmt(b.call('$.pre_effect', b.thunk(serialized_body)))
+			b.stmt(
+				b.call(
+					'$.legacy_pre_effect',
+					sequence.length > 0 ? b.thunk(b.sequence(sequence)) : b.thunk(b.block([])),
+					b.thunk(b.block(body))
+				)
+			)
 		);
 
 		return b.empty;

packages/svelte/src/internal/client/reactivity/effects.js

@@ -5,9 +5,13 @@ import {
 	current_effect,
 	destroy_signal,
 	flush_local_render_effects,
-	schedule_effect
+	get,
+	is_runes,
+	schedule_effect,
+	untrack
 } from '../runtime.js';
 import { DIRTY, MANAGED, RENDER_EFFECT, EFFECT, PRE_EFFECT } from '../constants.js';
+import { set } from './sources.js';
 
 /**
  * @param {import('#client').Reaction} target_signal
@@ -156,6 +160,7 @@ export function pre_effect(fn) {
 		);
 	}
 	const sync = current_effect !== null && (current_effect.f & RENDER_EFFECT) !== 0;
+	const runes = is_runes(current_component_context);
 	return create_effect(
 		PRE_EFFECT,
 		() => {
@@ -169,6 +174,47 @@ export function pre_effect(fn) {
 	);
 }
 
+/**
+ * Internal representation of `$: ..`
+ * @param {() => any} deps
+ * @param {() => void | (() => void)} fn
+ * @returns {import('#client').Effect}
+ */
+export function legacy_pre_effect(deps, fn) {
+	const component_context = /** @type {import('#client').ComponentContext} */ (
+		current_component_context
+	);
+	const token = {};
+	return create_effect(
+		PRE_EFFECT,
+		() => {
+			deps();
+			if (component_context.l1.includes(token)) {
+				return;
+			}
+			component_context.l1.push(token);
+			set(component_context.l2, true);
+			return untrack(fn);
+		},
+		true,
+		current_block,
+		true
+	);
+}
+
+export function legacy_pre_effect_reset() {
+	const component_context = /** @type {import('#client').ComponentContext} */ (
+		current_component_context
+	);
+	return render_effect(() => {
+		const x = get(component_context.l2);
+		if (x) {
+			component_context.l1.length = 0;
+			component_context.l2.v = false; // set directly to avoid rerunning this effect
+		}
+	});
+}
+
 /**
  * This effect is used to ensure binding are kept in sync. We use a pre effect to ensure we run before the
  * bindings which are in later effects. However, we don't use a pre_effect directly as we don't want to flush anything.

packages/svelte/src/internal/client/runtime.js

@@ -30,7 +30,7 @@ import {
 } from './constants.js';
 import { flush_tasks } from './dom/task.js';
 import { add_owner } from './dev/ownership.js';
-import { mutate, set } from './reactivity/sources.js';
+import { mutate, set, source } from './reactivity/sources.js';
 
 const IS_EFFECT = EFFECT | PRE_EFFECT | RENDER_EFFECT;
 
@@ -430,11 +430,7 @@ export function execute_effect(signal) {
 		current_effect = previous_effect;
 	}
 	const component_context = signal.x;
-	if (
-		is_runes(component_context) && // Don't rerun pre effects more than once to accomodate for "$: only runs once" behavior
-		(signal.f & PRE_EFFECT) !== 0 &&
-		current_queued_pre_and_render_effects.length > 0
-	) {
+	if ((signal.f & PRE_EFFECT) !== 0 && current_queued_pre_and_render_effects.length > 0) {
 		flush_local_pre_effects(component_context);
 	}
 }
@@ -1163,6 +1159,9 @@ export function push(props, runes = false, fn) {
 		s: props,
 		// runes
 		r: runes,
+		// legacy $:
+		l1: [],
+		l2: source(false),
 		// update_callbacks
 		u: null
 	};

packages/svelte/src/internal/client/types.d.ts

@@ -42,6 +42,10 @@ export type ComponentContext = {
 	c: null | Map<unknown, unknown>;
 	/** runes */
 	r: boolean;
+	/** legacy mode: if `$:` statements are allowed to run (ensures they only run once per render) */
+	l1: any[];
+	/** legacy mode: if `$:` statements are allowed to run (ensures they only run once per render) */
+	l2: Source<boolean>;
 	/** update_callbacks */
 	u: null | {
 		/** afterUpdate callbacks */

packages/svelte/tests/runtime-legacy/samples/reactive-assignment-in-complex-declaration-with-store-2/_config.js

@@ -1,10 +1,12 @@
+import { tick } from 'svelte';
 import { test } from '../../test';
 
 // destructure to store value
 export default test({
 	html: '<h1>2 2 xxx 5 6 9 10 2</h1>',
 	async test({ assert, target, component }) {
-		await component.update();
+		component.update();
+		await tick();
 		assert.htmlEqual(target.innerHTML, '<h1>11 11 yyy 12 13 14 15 11</h1>');
 	}
 });

packages/svelte/tests/runtime-legacy/samples/reactive-assignment-in-complex-declaration-with-store/_config.js

@@ -1,10 +1,12 @@
+import { tick } from 'svelte';
 import { test } from '../../test';
 
 // destructure to store
 export default test({
 	html: '<h1>2 2 xxx 5 6 9 10 2</h1>',
 	async test({ assert, target, component }) {
-		await component.update();
+		component.update();
+		await tick();
 		assert.htmlEqual(target.innerHTML, '<h1>11 11 yyy 12 13 14 15 11</h1>');
 	}
 });

packages/svelte/tests/runtime-legacy/samples/reactive-assignment-prevent-loop/_config.js

@@ -0,0 +1,13 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	html: '<button>1 / 1</button>',
+	async test({ assert, target }) {
+		const button = target.querySelector('button');
+		button?.click();
+		await tick();
+
+		assert.htmlEqual(target.innerHTML, '<button>3 / 2</button>');
+	}
+});

packages/svelte/tests/runtime-legacy/samples/reactive-assignment-prevent-loop/main.svelte

@@ -0,0 +1,9 @@
+<script>
+	let count1 = 0;
+	let count2 = 0;
+	function increaseCount1() { count1++ }
+	$: if(count2 < 10) { increaseCount1() }
+	$: if(count1 < 10) { count2++ }
+</script>
+
+<button on:click={() => count1++}>{count1} / {count2}</button>