Skip to content

Commit 7ef686f

Browse files
trueadmRich-Harris
trueadm
and
Rich-Harris
authored
feat: provide $state warnings for accidental equality (#11610)
* feat: provide $state warnings for accidental equality * tune * tune * tune * adjust test * fix treeshaking * fix bugs * fix bugs * refactor * revert test changes * tune * tune * tune * tune * fix up * fix * remove if(DEV) stuff * use console.trace, like we do for ownership warnings * tweak * tweak message, simplify logic --------- Co-authored-by: Rich Harris <[email protected]>
1 parent f488a6e commit 7ef686f

File tree

7 files changed

+143
-0
lines changed

7 files changed

+143
-0
lines changed

‎.changeset/little-ligers-exist.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"svelte": patch
3+
---
4+
5+
feat: provide $state warnings for accidental equality

‎packages/svelte/messages/client-warnings/warnings.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,7 @@
1919
> Mutating a value outside the component that created it is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead
2020
2121
> %component% mutated a value owned by %owner%. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead
22+
23+
## state_proxy_equality_mismatch
24+
25+
> Reactive `$state(...)` proxies and the values they proxy have different identities. Because of this, comparisons with `%operator%` will produce unexpected results. Consider using `$state.is(a, b)` instead

‎packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,31 @@ export const javascript_visitors_runes = {
451451
}
452452

453453
context.next();
454+
},
455+
BinaryExpression(node, { state, visit, next }) {
456+
const operator = node.operator;
457+
458+
if (state.options.dev) {
459+
if (operator === '===' || operator === '!==') {
460+
return b.call(
461+
'$.strict_equals',
462+
/** @type {import('estree').Expression} */ (visit(node.left)),
463+
/** @type {import('estree').Expression} */ (visit(node.right)),
464+
operator === '!==' && b.literal(false)
465+
);
466+
}
467+
468+
if (operator === '==' || operator === '!=') {
469+
return b.call(
470+
'$.equals',
471+
/** @type {import('estree').Expression} */ (visit(node.left)),
472+
/** @type {import('estree').Expression} */ (visit(node.right)),
473+
operator === '!=' && b.literal(false)
474+
);
475+
}
476+
}
477+
478+
next();
454479
}
455480
};
456481

‎packages/svelte/src/internal/client/dev/equality.js‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
import * as w from '../warnings.js';
2+
import { get_proxied_value } from '../proxy.js';
3+
4+
export function init_array_prototype_warnings() {
5+
const array_prototype = Array.prototype;
6+
const { indexOf, lastIndexOf, includes } = array_prototype;
7+
8+
array_prototype.indexOf = function (item, from_index) {
9+
const index = indexOf.call(this, item, from_index);
10+
11+
if (index === -1) {
12+
const test = indexOf.call(get_proxied_value(this), get_proxied_value(item), from_index);
13+
14+
if (test !== -1) {
15+
w.state_proxy_equality_mismatch('array.indexOf(...)');
16+
17+
// eslint-disable-next-line no-console
18+
console.trace();
19+
}
20+
}
21+
22+
return index;
23+
};
24+
25+
array_prototype.lastIndexOf = function (item, from_index) {
26+
const index = lastIndexOf.call(this, item, from_index);
27+
28+
if (index === -1) {
29+
const test = lastIndexOf.call(get_proxied_value(this), get_proxied_value(item), from_index);
30+
31+
if (test !== -1) {
32+
w.state_proxy_equality_mismatch('array.lastIndexOf(...)');
33+
34+
// eslint-disable-next-line no-console
35+
console.trace();
36+
}
37+
}
38+
39+
return index;
40+
};
41+
42+
array_prototype.includes = function (item, from_index) {
43+
const has = includes.call(this, item, from_index);
44+
45+
if (!has) {
46+
const test = includes.call(get_proxied_value(this), get_proxied_value(item), from_index);
47+
48+
if (test) {
49+
w.state_proxy_equality_mismatch('array.includes(...)');
50+
51+
// eslint-disable-next-line no-console
52+
console.trace();
53+
}
54+
}
55+
56+
return has;
57+
};
58+
}
59+
60+
/**
61+
* @param {any} a
62+
* @param {any} b
63+
* @param {boolean} equal
64+
* @returns {boolean}
65+
*/
66+
export function strict_equals(a, b, equal = true) {
67+
if ((a === b) !== (get_proxied_value(a) === get_proxied_value(b))) {
68+
w.state_proxy_equality_mismatch(equal ? '===' : '!==');
69+
70+
// eslint-disable-next-line no-console
71+
console.trace();
72+
}
73+
74+
return (a === b) === equal;
75+
}
76+
77+
/**
78+
* @param {any} a
79+
* @param {any} b
80+
* @param {boolean} equal
81+
* @returns {boolean}
82+
*/
83+
export function equals(a, b, equal = true) {
84+
if ((a == b) !== (get_proxied_value(a) == get_proxied_value(b))) {
85+
w.state_proxy_equality_mismatch(equal ? '==' : '!=');
86+
87+
// eslint-disable-next-line no-console
88+
console.trace();
89+
}
90+
91+
return (a == b) === equal;
92+
}

‎packages/svelte/src/internal/client/dom/operations.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { hydrate_anchor, hydrate_nodes, hydrating } from './hydration.js';
22
import { get_descriptor } from '../utils.js';
33
import { DEV } from 'esm-env';
4+
import { init_array_prototype_warnings } from '../dev/equality.js';
45

56
// We cache the Node and Element prototype methods, so that we can avoid doing
67
// expensive prototype chain lookups.
@@ -74,6 +75,8 @@ export function init_operations() {
7475
if (DEV) {
7576
// @ts-expect-error
7677
element_prototype.__svelte_meta = null;
78+
79+
init_array_prototype_warnings();
7780
}
7881

7982
first_child_get = /** @type {(this: Node) => ChildNode | null} */ (

‎packages/svelte/src/internal/client/index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,3 +161,4 @@ export {
161161
validate_store,
162162
validate_void_dynamic_element
163163
} from '../shared/validate.js';
164+
export { strict_equals, equals } from './dev/equality.js';

‎packages/svelte/src/internal/client/warnings.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,4 +71,17 @@ export function ownership_invalid_mutation(component, owner) {
7171
// TODO print a link to the documentation
7272
console.warn("ownership_invalid_mutation");
7373
}
74+
}
75+
76+
/**
77+
* Reactive `$state(...)` proxies and the values they proxy have different identities. Because of this, comparisons with `%operator%` will produce unexpected results. Consider using `$state.is(a, b)` instead
78+
* @param {string} operator
79+
*/
80+
export function state_proxy_equality_mismatch(operator) {
81+
if (DEV) {
82+
console.warn(`%c[svelte] ${"state_proxy_equality_mismatch"}\n%c${`Reactive \`$state(...)\` proxies and the values they proxy have different identities. Because of this, comparisons with \`${operator}\` will produce unexpected results. Consider using \`$state.is(a, b)\` instead`}`, bold, normal);
83+
} else {
84+
// TODO print a link to the documentation
85+
console.warn("state_proxy_equality_mismatch");
86+
}
7487
}

0 commit comments

Comments
 (0)