From 05bd922f7f8028255910d158376c67b257bce0a7 Mon Sep 17 00:00:00 2001
From: Dominic Gannaway <trueadm@users.noreply.github.com>
Date: Fri, 12 Jan 2024 19:57:06 +0000
Subject: [PATCH] fix: improve nested effect heuristics (#10171)

---
 .changeset/angry-books-jam.md                 |  5 +++
 .../svelte/src/internal/client/runtime.js     | 32 +++++++++++++++----
 .../samples/nested-effect-conflict/_config.js | 28 ++++++++++++++++
 .../samples/nested-effect-conflict/log.js     |  2 ++
 .../nested-effect-conflict/main.svelte        | 21 ++++++++++++
 5 files changed, 81 insertions(+), 7 deletions(-)
 create mode 100644 .changeset/angry-books-jam.md
 create mode 100644 packages/svelte/tests/runtime-runes/samples/nested-effect-conflict/_config.js
 create mode 100644 packages/svelte/tests/runtime-runes/samples/nested-effect-conflict/log.js
 create mode 100644 packages/svelte/tests/runtime-runes/samples/nested-effect-conflict/main.svelte

diff --git a/.changeset/angry-books-jam.md b/.changeset/angry-books-jam.md
new file mode 100644
index 0000000000..6552a54e31
--- /dev/null
+++ b/.changeset/angry-books-jam.md
@@ -0,0 +1,5 @@
+---
+"svelte": patch
+---
+
+fix: improve nested effect heuristics
diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js
index 460a15d564..034e9e3c0a 100644
--- a/packages/svelte/src/internal/client/runtime.js
+++ b/packages/svelte/src/internal/client/runtime.js
@@ -624,6 +624,13 @@ export function schedule_effect(signal, sync) {
 		}
 		if ((flags & EFFECT) !== 0) {
 			current_queued_effects.push(signal);
+			// Prevent any nested user effects from potentially triggering
+			// before this effect is scheduled. We know they will be destroyed
+			// so we can make them inert to avoid having to find them in the
+			// queue and remove them.
+			if ((flags & MANAGED) === 0) {
+				mark_subtree_children_inert(signal, true);
+			}
 		} else {
 			current_queued_pre_and_render_effects.push(signal);
 		}
@@ -1017,6 +1024,23 @@ export function mutate_store(store, expression, new_value) {
 /**
  * @param {import('./types.js').ComputationSignal} signal
  * @param {boolean} inert
+ * @param {Set<import('./types.js').Block>} [visited_blocks]
+ * @returns {void}
+ */
+function mark_subtree_children_inert(signal, inert, visited_blocks) {
+	const references = signal.r;
+	if (references !== null) {
+		let i;
+		for (i = 0; i < references.length; i++) {
+			mark_subtree_inert(references[i], inert, visited_blocks);
+		}
+	}
+}
+
+/**
+ * @param {import('./types.js').ComputationSignal} signal
+ * @param {boolean} inert
+ * @param {Set<import('./types.js').Block>} [visited_blocks]
  * @returns {void}
  */
 export function mark_subtree_inert(signal, inert, visited_blocks = new Set()) {
@@ -1055,13 +1079,7 @@ export function mark_subtree_inert(signal, inert, visited_blocks = new Set()) {
 			}
 		}
 	}
-	const references = signal.r;
-	if (references !== null) {
-		let i;
-		for (i = 0; i < references.length; i++) {
-			mark_subtree_inert(references[i], inert, visited_blocks);
-		}
-	}
+	mark_subtree_children_inert(signal, inert, visited_blocks);
 }
 
 /**
diff --git a/packages/svelte/tests/runtime-runes/samples/nested-effect-conflict/_config.js b/packages/svelte/tests/runtime-runes/samples/nested-effect-conflict/_config.js
new file mode 100644
index 0000000000..6de94097f2
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/nested-effect-conflict/_config.js
@@ -0,0 +1,28 @@
+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] = target.querySelectorAll('button');
+
+		flushSync(() => {
+			b1?.click();
+		});
+
+		await Promise.resolve();
+		assert.deepEqual(log, [
+			'top level',
+			'inner',
+			0,
+			'destroy inner',
+			undefined,
+			'destroy outer',
+			undefined
+		]);
+	}
+});
diff --git a/packages/svelte/tests/runtime-runes/samples/nested-effect-conflict/log.js b/packages/svelte/tests/runtime-runes/samples/nested-effect-conflict/log.js
new file mode 100644
index 0000000000..d3df521f4d
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/nested-effect-conflict/log.js
@@ -0,0 +1,2 @@
+/** @type {any[]} */
+export const log = [];
diff --git a/packages/svelte/tests/runtime-runes/samples/nested-effect-conflict/main.svelte b/packages/svelte/tests/runtime-runes/samples/nested-effect-conflict/main.svelte
new file mode 100644
index 0000000000..a7c6625dc1
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/nested-effect-conflict/main.svelte
@@ -0,0 +1,21 @@
+<script>
+	import { log } from './log.js';
+	let c = $state({ a: 0 });
+
+	$effect(() => {
+		log.push('top level')
+			$effect(() => {
+					if (c) {
+							$effect(() => {
+								log.push('inner',c.a);
+									return () => log.push('destroy inner', c?.a);
+							});
+					}
+					return () => log.push('destroy outer', c?.a);
+			})
+	});
+</script>
+
+<button onclick={() => {
+	c.a = 1; c = null
+}}>toggle</button>