fix: compare array contents for equality mismatch detections, not the arrays themselves (#14738)

Rich-Harris · web-flow · commit 36a437c2f9a4 · 2024-12-17T13:04:16.000-05:00
diff --git a/.changeset/tasty-deers-agree.md b/.changeset/tasty-deers-agree.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: compare array contents for equality mismatch detections, not the arrays themselves
diff --git a/packages/svelte/src/internal/client/dev/equality.js b/packages/svelte/src/internal/client/dev/equality.js
@@ -17,10 +17,11 @@ export function init_array_prototype_warnings() {
 		const index = indexOf.call(this, item, from_index);
 
 		if (index === -1) {
-			const test = indexOf.call(get_proxied_value(this), get_proxied_value(item), from_index);
-
-			if (test !== -1) {
-				w.state_proxy_equality_mismatch('array.indexOf(...)');
+			for (let i = from_index ?? 0; i < this.length; i += 1) {
+				if (get_proxied_value(this[i]) === item) {
+					w.state_proxy_equality_mismatch('array.indexOf(...)');
+					break;
+				}
 			}
 		}
 
@@ -33,16 +34,11 @@ export function init_array_prototype_warnings() {
 		const index = lastIndexOf.call(this, item, from_index ?? this.length - 1);
 
 		if (index === -1) {
-			// we need to specify this.length - 1 because it's probably using something like
-			// `arguments` inside so passing undefined is different from not passing anything
-			const test = lastIndexOf.call(
-				get_proxied_value(this),
-				get_proxied_value(item),
-				from_index ?? this.length - 1
-			);
-
-			if (test !== -1) {
-				w.state_proxy_equality_mismatch('array.lastIndexOf(...)');
+			for (let i = 0; i <= (from_index ?? this.length - 1); i += 1) {
+				if (get_proxied_value(this[i]) === item) {
+					w.state_proxy_equality_mismatch('array.lastIndexOf(...)');
+					break;
+				}
 			}
 		}
 
@@ -53,10 +49,11 @@ export function init_array_prototype_warnings() {
 		const has = includes.call(this, item, from_index);
 
 		if (!has) {
-			const test = includes.call(get_proxied_value(this), get_proxied_value(item), from_index);
-
-			if (test) {
-				w.state_proxy_equality_mismatch('array.includes(...)');
+			for (let i = 0; i < this.length; i += 1) {
+				if (get_proxied_value(this[i]) === item) {
+					w.state_proxy_equality_mismatch('array.includes(...)');
+					break;
+				}
 			}
 		}
 
diff --git a/packages/svelte/tests/runtime-runes/samples/state-proxy-equality-mismatch/_config.js b/packages/svelte/tests/runtime-runes/samples/state-proxy-equality-mismatch/_config.js
@@ -0,0 +1,41 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	compileOptions: {
+		dev: true
+	},
+
+	async test({ assert, target, warnings }) {
+		const [btn1, btn2, btn3, btn4, btn5, btn6, clear] = target.querySelectorAll('button');
+
+		flushSync(() => {
+			btn1.click();
+			btn2.click();
+			btn3.click();
+			btn4.click();
+			btn5.click();
+			btn6.click();
+		});
+
+		assert.deepEqual(warnings, [
+			'Reactive `$state(...)` proxies and the values they proxy have different identities. Because of this, comparisons with `array.includes(...)` will produce unexpected results',
+			'Reactive `$state(...)` proxies and the values they proxy have different identities. Because of this, comparisons with `array.indexOf(...)` will produce unexpected results',
+			'Reactive `$state(...)` proxies and the values they proxy have different identities. Because of this, comparisons with `array.lastIndexOf(...)` will produce unexpected results'
+		]);
+
+		flushSync(() => clear.click());
+		warnings.length = 0;
+
+		flushSync(() => {
+			btn1.click();
+			btn2.click();
+			btn3.click();
+			btn4.click();
+			btn5.click();
+			btn6.click();
+		});
+
+		assert.deepEqual(warnings, []);
+	}
+});
diff --git a/packages/svelte/tests/runtime-runes/samples/state-proxy-equality-mismatch/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-proxy-equality-mismatch/main.svelte
@@ -0,0 +1,23 @@
+<script>
+	let primitive = 'foo';
+	let object = {};
+
+	let array = $state([primitive, object]);
+</script>
+
+<button onclick={() => array.includes(primitive)}>array.includes(primitive)</button>
+<button onclick={() => array.includes(object)}>array.includes(object)</button>
+
+<hr />
+
+<button onclick={() => array.indexOf(primitive)}>array.indexOf(primitive)</button>
+<button onclick={() => array.indexOf(object)}>array.indexOf(object)</button>
+
+<hr />
+
+<button onclick={() => array.lastIndexOf(primitive)}>array.lastIndexOf(primitive)</button>
+<button onclick={() => array.lastIndexOf(object)}>array.lastIndexOf(object)</button>
+
+<hr />
+
+<button onclick={() => (array.length = 0)}>clear</button>

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +'svelte': patch
 +---
++
 +fix: compare array contents for equality mismatch detections, not the arrays themselves