fix: prevent reactive statement reruns

.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>