diff --git a/.changeset/little-ligers-exist.md b/.changeset/little-ligers-exist.md new file mode 100644 index 0000000000..bf161684d2 --- /dev/null +++ b/.changeset/little-ligers-exist.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +feat: provide $state warnings for accidental equality diff --git a/packages/svelte/messages/client-warnings/warnings.md b/packages/svelte/messages/client-warnings/warnings.md index 1f77457f94..3131ab98f4 100644 --- a/packages/svelte/messages/client-warnings/warnings.md +++ b/packages/svelte/messages/client-warnings/warnings.md @@ -19,3 +19,7 @@ > 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 > %component% mutated a value owned by %owner%. This is strongly discouraged. Consider passing values to child components with `bind:`, or use a callback instead + +## state_proxy_equality_mismatch + +> 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 diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js index e1229a2bed..63eec24151 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/javascript-runes.js @@ -451,6 +451,31 @@ export const javascript_visitors_runes = { } context.next(); + }, + BinaryExpression(node, { state, visit, next }) { + const operator = node.operator; + + if (state.options.dev) { + if (operator === '===' || operator === '!==') { + return b.call( + '$.strict_equals', + /** @type {import('estree').Expression} */ (visit(node.left)), + /** @type {import('estree').Expression} */ (visit(node.right)), + operator === '!==' && b.literal(false) + ); + } + + if (operator === '==' || operator === '!=') { + return b.call( + '$.equals', + /** @type {import('estree').Expression} */ (visit(node.left)), + /** @type {import('estree').Expression} */ (visit(node.right)), + operator === '!=' && b.literal(false) + ); + } + } + + next(); } }; diff --git a/packages/svelte/src/internal/client/dev/equality.js b/packages/svelte/src/internal/client/dev/equality.js new file mode 100644 index 0000000000..bd03a84ac6 --- /dev/null +++ b/packages/svelte/src/internal/client/dev/equality.js @@ -0,0 +1,92 @@ +import * as w from '../warnings.js'; +import { get_proxied_value } from '../proxy.js'; + +export function init_array_prototype_warnings() { + const array_prototype = Array.prototype; + const { indexOf, lastIndexOf, includes } = array_prototype; + + array_prototype.indexOf = function (item, from_index) { + 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(...)'); + + // eslint-disable-next-line no-console + console.trace(); + } + } + + return index; + }; + + array_prototype.lastIndexOf = function (item, from_index) { + const index = lastIndexOf.call(this, item, from_index); + + if (index === -1) { + const test = lastIndexOf.call(get_proxied_value(this), get_proxied_value(item), from_index); + + if (test !== -1) { + w.state_proxy_equality_mismatch('array.lastIndexOf(...)'); + + // eslint-disable-next-line no-console + console.trace(); + } + } + + return index; + }; + + array_prototype.includes = function (item, from_index) { + 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(...)'); + + // eslint-disable-next-line no-console + console.trace(); + } + } + + return has; + }; +} + +/** + * @param {any} a + * @param {any} b + * @param {boolean} equal + * @returns {boolean} + */ +export function strict_equals(a, b, equal = true) { + if ((a === b) !== (get_proxied_value(a) === get_proxied_value(b))) { + w.state_proxy_equality_mismatch(equal ? '===' : '!=='); + + // eslint-disable-next-line no-console + console.trace(); + } + + return (a === b) === equal; +} + +/** + * @param {any} a + * @param {any} b + * @param {boolean} equal + * @returns {boolean} + */ +export function equals(a, b, equal = true) { + if ((a == b) !== (get_proxied_value(a) == get_proxied_value(b))) { + w.state_proxy_equality_mismatch(equal ? '==' : '!='); + + // eslint-disable-next-line no-console + console.trace(); + } + + return (a == b) === equal; +} diff --git a/packages/svelte/src/internal/client/dom/operations.js b/packages/svelte/src/internal/client/dom/operations.js index 3c3ca4f7fd..b5c4d2613a 100644 --- a/packages/svelte/src/internal/client/dom/operations.js +++ b/packages/svelte/src/internal/client/dom/operations.js @@ -1,6 +1,7 @@ import { hydrate_anchor, hydrate_nodes, hydrating } from './hydration.js'; import { get_descriptor } from '../utils.js'; import { DEV } from 'esm-env'; +import { init_array_prototype_warnings } from '../dev/equality.js'; // We cache the Node and Element prototype methods, so that we can avoid doing // expensive prototype chain lookups. @@ -74,6 +75,8 @@ export function init_operations() { if (DEV) { // @ts-expect-error element_prototype.__svelte_meta = null; + + init_array_prototype_warnings(); } first_child_get = /** @type {(this: Node) => ChildNode | null} */ ( diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 8453696e69..6ea507bf8e 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -161,3 +161,4 @@ export { validate_store, validate_void_dynamic_element } from '../shared/validate.js'; +export { strict_equals, equals } from './dev/equality.js'; diff --git a/packages/svelte/src/internal/client/warnings.js b/packages/svelte/src/internal/client/warnings.js index 1dd532164c..b28b7d2059 100644 --- a/packages/svelte/src/internal/client/warnings.js +++ b/packages/svelte/src/internal/client/warnings.js @@ -71,4 +71,17 @@ export function ownership_invalid_mutation(component, owner) { // TODO print a link to the documentation console.warn("ownership_invalid_mutation"); } +} + +/** + * 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 + * @param {string} operator + */ +export function state_proxy_equality_mismatch(operator) { + if (DEV) { + 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); + } else { + // TODO print a link to the documentation + console.warn("state_proxy_equality_mismatch"); + } } \ No newline at end of file