fix: abort and reschedule effect processing after state change in user effect (#16280)

* fix: abort and reschedule effect processing after state change in user effect

* failing test

* skip for now

Author: Rich-Harris

.changeset/cuddly-walls-tan.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: abort and reschedule effect processing after state change in user effect

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

@@ -19,6 +19,7 @@ export const INSPECT_EFFECT = 1 << 17;
 export const HEAD_EFFECT = 1 << 18;
 export const EFFECT_PRESERVED = 1 << 19;
 export const EFFECT_IS_UPDATING = 1 << 20;
+export const USER_EFFECT = 1 << 21;
 
 export const STATE_SYMBOL = Symbol('$state');
 export const LEGACY_PROPS = Symbol('legacy props');

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

@@ -9,7 +9,7 @@ import {
 	set_active_effect,
 	set_active_reaction
 } from './runtime.js';
-import { effect, teardown } from './reactivity/effects.js';
+import { create_user_effect, teardown } from './reactivity/effects.js';
 import { legacy_mode_flag } from '../flags/index.js';
 import { FILENAME } from '../../constants.js';
 
@@ -191,7 +191,7 @@ export function pop(component) {
 					var component_effect = component_effects[i];
 					set_active_effect(component_effect.effect);
 					set_active_reaction(component_effect.reaction);
-					effect(component_effect.fn);
+					create_user_effect(component_effect.fn);
 				}
 			} finally {
 				set_active_effect(previous_effect);

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

@@ -33,7 +33,8 @@ import {
 	MAYBE_DIRTY,
 	EFFECT_PRESERVED,
 	BOUNDARY_EFFECT,
-	STALE_REACTION
+	STALE_REACTION,
+	USER_EFFECT
 } from '#client/constants';
 import { set } from './sources.js';
 import * as e from '../errors.js';
@@ -200,11 +201,17 @@ export function user_effect(fn) {
 			reaction: active_reaction
 		});
 	} else {
-		var signal = effect(fn);
-		return signal;
+		return create_user_effect(fn);
 	}
 }
 
+/**
+ * @param {() => void | (() => void)} fn
+ */
+export function create_user_effect(fn) {
+	return create_effect(EFFECT | USER_EFFECT, fn, false);
+}
+
 /**
  * Internal representation of `$effect.pre(...)`
  * @param {() => void | (() => void)} fn
@@ -217,7 +224,7 @@ export function user_pre_effect(fn) {
 			value: '$effect.pre'
 		});
 	}
-	return render_effect(fn);
+	return create_effect(RENDER_EFFECT | USER_EFFECT, fn, true);
 }
 
 /** @param {() => void | (() => void)} fn */

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

@@ -22,7 +22,8 @@ import {
 	ROOT_EFFECT,
 	DISCONNECTED,
 	EFFECT_IS_UPDATING,
-	STALE_REACTION
+	STALE_REACTION,
+	USER_EFFECT
 } from './constants.js';
 import { flush_tasks } from './dom/task.js';
 import { internal_set, old_values } from './reactivity/sources.js';
@@ -581,6 +582,8 @@ function flush_queued_effects(effects) {
 
 		if ((effect.f & (DESTROYED | INERT)) === 0) {
 			if (check_dirtiness(effect)) {
+				var wv = write_version;
+
 				update_effect(effect);
 
 				// Effects with no dependencies or teardown do not get added to the effect tree.
@@ -597,9 +600,19 @@ function flush_queued_effects(effects) {
 						effect.fn = null;
 					}
 				}
+
+				// if state is written in a user effect, abort and re-schedule, lest we run
+				// effects that should be removed as a result of the state change
+				if (write_version > wv && (effect.f & USER_EFFECT) !== 0) {
+					break;
+				}
 			}
 		}
 	}
+
+	for (; i < length; i += 1) {
+		schedule_effect(effects[i]);
+	}
 }
 
 /**

packages/svelte/tests/runtime-runes/samples/effect-order-6/A.svelte

@@ -0,0 +1,11 @@
+<script>
+	import B from './B.svelte';
+
+	let { boolean, closed } = $props();
+
+	$effect(() => {
+		console.log(boolean);
+	});
+</script>
+
+<B {closed} />

packages/svelte/tests/runtime-runes/samples/effect-order-6/B.svelte

@@ -0,0 +1,9 @@
+<script>
+	import { close } from './Child.svelte';
+
+	let { closed } = $props();
+
+	$effect(() => {
+		if (closed) close();
+	});
+</script>

packages/svelte/tests/runtime-runes/samples/effect-order-6/Child.svelte

@@ -0,0 +1,20 @@
+<script module>
+	let object = $state();
+
+	export function open() {
+		object = { boolean: true };
+	}
+
+	export function close() {
+		object = undefined;
+	}
+</script>
+
+<script>
+	let { children } = $props();
+</script>
+
+{#if object?.boolean}
+	<!-- error occurs here, this is executed when the if should already make it falsy -->
+	{@render children(object.boolean)}
+{/if}

packages/svelte/tests/runtime-runes/samples/effect-order-6/_config.js

@@ -0,0 +1,13 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target, logs }) {
+		const [open, close] = target.querySelectorAll('button');
+
+		flushSync(() => open.click());
+		flushSync(() => close.click());
+
+		assert.deepEqual(logs, [true]);
+	}
+});

packages/svelte/tests/runtime-runes/samples/effect-order-6/main.svelte

@@ -0,0 +1,23 @@
+<script>
+	import A from './A.svelte';
+	import Child, { open } from './Child.svelte';
+
+	let closed = $state(false);
+</script>
+
+<button onclick={open}>
+	open
+</button>
+
+<button onclick={() => closed = true}>
+	close
+</button>
+
+<hr>
+
+<Child>
+	{#snippet children(boolean)}
+		<A {closed} {boolean} />
+	{/snippet}
+</Child>
+