chore: verify that $effect.root(...) does not re-run (#11020)

Author: Rich-Harris

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

@@ -53,6 +53,7 @@ export function push_effect(effect, parent_effect) {
  */
 function create_effect(type, fn, sync) {
 	var is_root = (type & ROOT_EFFECT) !== 0;
+
 	/** @type {import('#client').Effect} */
 	var effect = {
 		ctx: current_component_context,
@@ -150,9 +151,7 @@ export function user_pre_effect(fn) {
  * @returns {() => void}
  */
 export function effect_root(fn) {
-	// TODO is `untrack` correct here? Should `fn` re-run if its dependencies change?
-	// Should it even be modelled as an effect?
-	const effect = create_effect(ROOT_EFFECT, () => untrack(fn), true);
+	const effect = create_effect(ROOT_EFFECT, fn, true);
 	return () => {
 		destroy_effect(effect);
 	};

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

@@ -245,27 +245,25 @@ function _mount(
 
 	const unmount = effect_root(() => {
 		branch(() => {
-			untrack(() => {
-				if (context) {
-					push({});
-					var ctx = /** @type {import('#client').ComponentContext} */ (current_component_context);
-					ctx.c = context;
-				}
-
-				if (events) {
-					// We can't spread the object or else we'd lose the state proxy stuff, if it is one
-					/** @type {any} */ (props).$$events = events;
-				}
-
-				should_intro = intro;
-				// @ts-expect-error the public typings are not what the actual function looks like
-				component = Component(anchor, props) || {};
-				should_intro = true;
-
-				if (context) {
-					pop();
-				}
-			});
+			if (context) {
+				push({});
+				var ctx = /** @type {import('#client').ComponentContext} */ (current_component_context);
+				ctx.c = context;
+			}
+
+			if (events) {
+				// We can't spread the object or else we'd lose the state proxy stuff, if it is one
+				/** @type {any} */ (props).$$events = events;
+			}
+
+			should_intro = intro;
+			// @ts-expect-error the public typings are not what the actual function looks like
+			component = Component(anchor, props) || {};
+			should_intro = true;
+
+			if (context) {
+				pop();
+			}
 		});
 
 		return () => {

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

@@ -21,7 +21,8 @@ import {
 	INERT,
 	BRANCH_EFFECT,
 	STATE_SYMBOL,
-	BLOCK_EFFECT
+	BLOCK_EFFECT,
+	ROOT_EFFECT
 } from './constants.js';
 import { flush_tasks } from './dom/task.js';
 import { add_owner } from './dev/ownership.js';
@@ -692,7 +693,7 @@ export function get(signal) {
 	// Register the dependency on the current reaction signal.
 	if (
 		current_reaction !== null &&
-		(current_reaction.f & BRANCH_EFFECT) === 0 &&
+		(current_reaction.f & (BRANCH_EFFECT | ROOT_EFFECT)) === 0 &&
 		!current_untracking
 	) {
 		const unowned = (current_reaction.f & UNOWNED) !== 0;
@@ -741,6 +742,7 @@ export function get(signal) {
 			update_derived(/** @type {import('./types.js').Derived} **/ (signal), false);
 		}
 	}
+
 	return signal.v;
 }
 

packages/svelte/tests/runtime-runes/samples/effect-root-2/_config.js

@@ -0,0 +1,31 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+import { log } from './log.js';
+
+export default test({
+	before_test() {
+		log.length = 0;
+	},
+
+	async test({ assert, target }) {
+		const [b1, b2] = target.querySelectorAll('button');
+
+		flushSync(() => {
+			b1.click();
+		});
+
+		assert.deepEqual(log, [0]);
+
+		flushSync(() => {
+			b2.click();
+		});
+
+		assert.deepEqual(log, [0, 'cleanup']);
+
+		flushSync(() => {
+			b1.click();
+		});
+
+		assert.deepEqual(log, [0, 'cleanup']);
+	}
+});

packages/svelte/tests/runtime-runes/samples/effect-root-2/log.js

@@ -0,0 +1,2 @@
+/** @type {any[]} */
+export const log = [];

packages/svelte/tests/runtime-runes/samples/effect-root-2/main.svelte

@@ -0,0 +1,13 @@
+<script>
+	import { log } from './log.js';
+
+	let x = $state(0);
+
+	const cleanup = $effect.root(() => {
+		log.push(x);
+		return () => log.push('cleanup');
+	});
+</script>
+
+<button onclick={() => x++}>{x}</button>
+<button onclick={cleanup}>cleanup</button>

packages/svelte/tests/runtime-runes/samples/effect-root/main.svelte

@@ -11,7 +11,7 @@
 
 		const nested_cleanup = $effect.root(() => {
 			return () => {
-				log.push('cleanup 2')	;
+				log.push('cleanup 2');
 			}
 		});
 
@@ -22,6 +22,6 @@
 	});
 </script>
 
-<button on:click={() => x++}>{x}</button>
-<button on:click={() => y++}>{y}</button>
-<button on:click={() => cleanup()}>cleanup</button>
+<button onclick={() => x++}>{x}</button>
+<button onclick={() => y++}>{y}</button>
+<button onclick={cleanup}>cleanup</button>