fix: ensure if block is executed in correct order (#10053)

* fix: ensure if block is executed in correct order

* alternative approach

* improve algo

* optimize

* lint

Author: trueadm

.changeset/unlucky-trees-lick.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: ensure if block is executed in correct order

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

@@ -349,7 +349,21 @@ function execute_signal_fn(signal) {
 
 		if (current_dependencies !== null) {
 			let i;
-			remove_consumer(signal, current_dependencies_index, false);
+			if (dependencies !== null) {
+				const dep_length = dependencies.length;
+				// If we have more than 16 elements in the array then use a Set for faster performance
+				// TODO: evaluate if we should always just use a Set or not here?
+				const current_dependencies_set = dep_length > 16 ? new Set(current_dependencies) : null;
+				for (i = current_dependencies_index; i < dep_length; i++) {
+					const dependency = dependencies[i];
+					if (
+						(current_dependencies_set !== null && !current_dependencies_set.has(dependency)) ||
+						!current_dependencies.includes(dependency)
+					) {
+						remove_consumer(signal, dependency, false);
+					}
+				}
+			}
 
 			if (dependencies !== null && current_dependencies_index > 0) {
 				dependencies.length = current_dependencies_index + current_dependencies.length;
@@ -365,16 +379,17 @@ function execute_signal_fn(signal) {
 			if (!current_skip_consumer) {
 				for (i = current_dependencies_index; i < dependencies.length; i++) {
 					const dependency = dependencies[i];
+					const consumers = dependency.c;
 
-					if (dependency.c === null) {
+					if (consumers === null) {
 						dependency.c = [signal];
-					} else {
-						dependency.c.push(signal);
+					} else if (consumers[consumers.length - 1] !== signal) {
+						consumers.push(signal);
 					}
 				}
 			}
 		} else if (dependencies !== null && current_dependencies_index < dependencies.length) {
-			remove_consumer(signal, current_dependencies_index, false);
+			remove_consumers(signal, current_dependencies_index, false);
 			dependencies.length = current_dependencies_index;
 		}
 		return res;
@@ -390,43 +405,54 @@ function execute_signal_fn(signal) {
 	}
 }
 
+/**
+ * @template V
+ * @param {import('./types.js').ComputationSignal<V>} signal
+ * @param {import('./types.js').Signal<V>} dependency
+ * @param {boolean} remove_unowned
+ * @returns {void}
+ */
+function remove_consumer(signal, dependency, remove_unowned) {
+	const consumers = dependency.c;
+	let consumers_length = 0;
+	if (consumers !== null) {
+		consumers_length = consumers.length - 1;
+		const index = consumers.indexOf(signal);
+		if (index !== -1) {
+			if (consumers_length === 0) {
+				dependency.c = null;
+			} else {
+				// Swap with last element and then remove.
+				consumers[index] = consumers[consumers_length];
+				consumers.pop();
+			}
+		}
+	}
+	if (remove_unowned && consumers_length === 0 && (dependency.f & UNOWNED) !== 0) {
+		// If the signal is unowned then we need to make sure to change it to dirty.
+		set_signal_status(dependency, DIRTY);
+		remove_consumers(
+			/** @type {import('./types.js').ComputationSignal<V>} **/ (dependency),
+			0,
+			true
+		);
+	}
+}
+
 /**
  * @template V
  * @param {import('./types.js').ComputationSignal<V>} signal
  * @param {number} start_index
  * @param {boolean} remove_unowned
  * @returns {void}
  */
-function remove_consumer(signal, start_index, remove_unowned) {
+function remove_consumers(signal, start_index, remove_unowned) {
 	const dependencies = signal.d;
 	if (dependencies !== null) {
 		let i;
 		for (i = start_index; i < dependencies.length; i++) {
 			const dependency = dependencies[i];
-			const consumers = dependency.c;
-			let consumers_length = 0;
-			if (consumers !== null) {
-				consumers_length = consumers.length - 1;
-				const index = consumers.indexOf(signal);
-				if (index !== -1) {
-					if (consumers_length === 0) {
-						dependency.c = null;
-					} else {
-						// Swap with last element and then remove.
-						consumers[index] = consumers[consumers_length];
-						consumers.pop();
-					}
-				}
-			}
-			if (remove_unowned && consumers_length === 0 && (dependency.f & UNOWNED) !== 0) {
-				// If the signal is unowned then we need to make sure to change it to dirty.
-				set_signal_status(dependency, DIRTY);
-				remove_consumer(
-					/** @type {import('./types.js').ComputationSignal<V>} **/ (dependency),
-					0,
-					true
-				);
-			}
+			remove_consumer(signal, dependency, remove_unowned);
 		}
 	}
 }
@@ -446,7 +472,7 @@ function destroy_references(signal) {
 			if ((reference.f & IS_EFFECT) !== 0) {
 				destroy_signal(reference);
 			} else {
-				remove_consumer(reference, 0, true);
+				remove_consumers(reference, 0, true);
 				reference.d = null;
 			}
 		}
@@ -841,10 +867,16 @@ export function get(signal) {
 			!(unowned && current_effect !== null)
 		) {
 			current_dependencies_index++;
-		} else if (current_dependencies === null) {
-			current_dependencies = [signal];
-		} else if (signal !== current_dependencies[current_dependencies.length - 1]) {
-			current_dependencies.push(signal);
+		} else if (
+			dependencies === null ||
+			current_dependencies_index === 0 ||
+			dependencies[current_dependencies_index - 1] !== signal
+		) {
+			if (current_dependencies === null) {
+				current_dependencies = [signal];
+			} else if (signal !== current_dependencies[current_dependencies.length - 1]) {
+				current_dependencies.push(signal);
+			}
 		}
 		if (
 			current_untracked_writes !== null &&
@@ -1098,7 +1130,7 @@ export function destroy_signal(signal) {
 	const destroy = signal.y;
 	const flags = signal.f;
 	destroy_references(signal);
-	remove_consumer(signal, 0, true);
+	remove_consumers(signal, 0, true);
 	signal.i =
 		signal.r =
 		signal.y =

packages/svelte/tests/runtime-runes/samples/block-dependency-sequence/Component.svelte

@@ -0,0 +1,9 @@
+<script>
+	const { item } = $props();
+</script>
+
+<div>
+	{#if item}
+		{item.length}
+	{/if}
+</div>

packages/svelte/tests/runtime-runes/samples/block-dependency-sequence/_config.js

@@ -0,0 +1,26 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target, component }) {
+		const [b1, b2] = target.querySelectorAll('button');
+		assert.htmlEqual(
+			target.innerHTML,
+			'<div>5</div><div>5</div><div>3</div><button>set null</button><button>set object</button'
+		);
+		flushSync(() => {
+			b2.click();
+		});
+		assert.htmlEqual(
+			target.innerHTML,
+			'<div>5</div><div>5</div><div>3</div><button>set null</button><button>set object</button'
+		);
+		flushSync(() => {
+			b1.click();
+		});
+		assert.htmlEqual(
+			target.innerHTML,
+			'<div>5</div><div></div><div>3</div><button>set null</button><button>set object</button'
+		);
+	}
+});

packages/svelte/tests/runtime-runes/samples/block-dependency-sequence/main.svelte

@@ -0,0 +1,12 @@
+<script>
+	import Component from './Component.svelte';
+
+	let items = $state(['hello', 'world', 'bye']);
+</script>
+
+{#each items as item}
+	<Component {item} />
+{/each}
+
+<button onclick={() => (items[1] = null)}> set null </button>
+<button onclick={() => (items[1] = 'hello')}> set object </button>