Skip to content

Commit

Permalink
fix: compare array contents for equality mismatch detections, not the…
Browse files Browse the repository at this point in the history
… arrays themselves (#14738)
  • Loading branch information
Rich-Harris authored Dec 17, 2024
1 parent d7e4bd2 commit 36a437c
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 18 deletions.
5 changes: 5 additions & 0 deletions .changeset/tasty-deers-agree.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: compare array contents for equality mismatch detections, not the arrays themselves
33 changes: 15 additions & 18 deletions packages/svelte/src/internal/client/dev/equality.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}

Expand All @@ -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;
}
}
}

Expand All @@ -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;
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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, []);
}
});
Original file line number Diff line number Diff line change
@@ -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>

0 comments on commit 36a437c

Please sign in to comment.