From 0812b10100f8601f1d342ab5b7bbe9b152f5b15b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 20 Aug 2024 09:34:30 -0400 Subject: [PATCH] breaking: overhaul proxies, remove `$state.is` (#12916) * chore: use closures for state proxies * use variables * early return * tidy up * move ownership stuff into separate object * put original value directly on STATE_SYMBOL * rename * tidy up * tidy * tweak * fix * remove is_frozen check * remove `$state.is` * avoid mutations * tweak * changesets * changeset * changeset * regenerate * add comment * add note * add test --- .changeset/fifty-actors-agree.md | 5 + .changeset/gorgeous-pans-sort.md | 5 + .changeset/heavy-houses-pay.md | 5 + .changeset/short-starfishes-beg.md | 5 + documentation/docs/03-runes/01-state.md | 22 - .../svelte/messages/client-errors/errors.md | 4 + .../messages/client-warnings/warnings.md | 15 +- .../svelte/messages/compile-errors/script.md | 4 + packages/svelte/src/ambient.d.ts | 21 - packages/svelte/src/compiler/errors.js | 10 + .../2-analyze/visitors/CallExpression.js | 7 - .../phases/2-analyze/visitors/Identifier.js | 4 + .../client/visitors/CallExpression.js | 7 - .../client/visitors/VariableDeclaration.js | 3 +- .../server/visitors/CallExpression.js | 8 - .../svelte/src/internal/client/constants.js | 1 + .../src/internal/client/dev/equality.js | 19 +- .../src/internal/client/dev/ownership.js | 10 +- .../client/dom/elements/bindings/this.js | 6 +- packages/svelte/src/internal/client/errors.js | 16 + packages/svelte/src/internal/client/index.js | 2 +- packages/svelte/src/internal/client/proxy.js | 445 +++++++++--------- .../svelte/src/internal/client/proxy.test.ts | 87 ++++ .../svelte/src/internal/client/types.d.ts | 19 +- .../svelte/src/internal/client/warnings.js | 7 +- packages/svelte/src/internal/shared/utils.js | 1 - packages/svelte/src/utils.js | 1 - .../_config.js | 6 +- .../main.svelte | 0 .../runtime-runes/samples/state-is/_config.js | 7 - .../samples/state-is/main.svelte | 10 - .../samples/state-is/_expected.html | 4 - .../samples/state-is/main.svelte | 10 - packages/svelte/types/index.d.ts | 21 - 34 files changed, 395 insertions(+), 402 deletions(-) create mode 100644 .changeset/fifty-actors-agree.md create mode 100644 .changeset/gorgeous-pans-sort.md create mode 100644 .changeset/heavy-houses-pay.md create mode 100644 .changeset/short-starfishes-beg.md create mode 100644 packages/svelte/src/internal/client/proxy.test.ts rename packages/svelte/tests/runtime-runes/samples/{proxy-shared => proxy-not-shared}/_config.js (91%) rename packages/svelte/tests/runtime-runes/samples/{proxy-shared => proxy-not-shared}/main.svelte (100%) delete mode 100644 packages/svelte/tests/runtime-runes/samples/state-is/_config.js delete mode 100644 packages/svelte/tests/runtime-runes/samples/state-is/main.svelte delete mode 100644 packages/svelte/tests/server-side-rendering/samples/state-is/_expected.html delete mode 100644 packages/svelte/tests/server-side-rendering/samples/state-is/main.svelte diff --git a/.changeset/fifty-actors-agree.md b/.changeset/fifty-actors-agree.md new file mode 100644 index 0000000000..2fd5cc66ef --- /dev/null +++ b/.changeset/fifty-actors-agree.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +breaking: disallow `Object.defineProperty` on state proxies with non-basic descriptors diff --git a/.changeset/gorgeous-pans-sort.md b/.changeset/gorgeous-pans-sort.md new file mode 100644 index 0000000000..8ed57cadbb --- /dev/null +++ b/.changeset/gorgeous-pans-sort.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +breaking: allow frozen objects to be proxied diff --git a/.changeset/heavy-houses-pay.md b/.changeset/heavy-houses-pay.md new file mode 100644 index 0000000000..8c1b2342ac --- /dev/null +++ b/.changeset/heavy-houses-pay.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +breaking: avoid mutations to underlying proxied object with $state diff --git a/.changeset/short-starfishes-beg.md b/.changeset/short-starfishes-beg.md new file mode 100644 index 0000000000..dedbfbfeef --- /dev/null +++ b/.changeset/short-starfishes-beg.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +breaking: remove $state.is rune diff --git a/documentation/docs/03-runes/01-state.md b/documentation/docs/03-runes/01-state.md index 64f5e171c0..7bcff2c968 100644 --- a/documentation/docs/03-runes/01-state.md +++ b/documentation/docs/03-runes/01-state.md @@ -101,28 +101,6 @@ To take a static snapshot of a deeply reactive `$state` proxy, use `$state.snaps This is handy when you want to pass some state to an external library or API that doesn't expect a proxy, such as `structuredClone`. -## `$state.is` - -Sometimes you might need to compare two values, one of which is a reactive `$state(...)` proxy but the other is not. For this you can use `$state.is(a, b)`: - -```svelte - -``` - -This is handy when you might want to check if the object exists within a deeply reactive object/array. - -Under the hood, `$state.is` uses [`Object.is`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is) for comparing the values. - -> Use this as an escape hatch - most of the time you don't need this. Svelte will warn you at dev time if you happen to run into this problem - ## `$derived` Derived state is declared with the `$derived` rune: diff --git a/packages/svelte/messages/client-errors/errors.md b/packages/svelte/messages/client-errors/errors.md index e509c970a2..ee05f6e664 100644 --- a/packages/svelte/messages/client-errors/errors.md +++ b/packages/svelte/messages/client-errors/errors.md @@ -64,6 +64,10 @@ > The `%rune%` rune is only available inside `.svelte` and `.svelte.js/ts` files +## state_descriptors_fixed + +> Property descriptors defined on `$state` objects must contain `value` and always be `enumerable`, `configurable` and `writable`. + ## state_prototype_fixed > Cannot set prototype of `$state` object diff --git a/packages/svelte/messages/client-warnings/warnings.md b/packages/svelte/messages/client-warnings/warnings.md index 3072bc4df1..4266f75184 100644 --- a/packages/svelte/messages/client-warnings/warnings.md +++ b/packages/svelte/messages/client-warnings/warnings.md @@ -44,7 +44,7 @@ ## 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%details% +> Reactive `$state(...)` proxies and the values they proxy have different identities. Because of this, comparisons with `%operator%` will produce unexpected results `$state(...)` creates a [proxy](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy) of the value it is passed. The proxy and the value have different identities, meaning equality checks will always return `false`: @@ -57,15 +57,4 @@ ``` -In the rare case that you need to compare them, you can use `$state.is`, which unwraps proxies: - -```svelte - -``` - -During development, Svelte will warn you when comparing values with proxies. +To resolve this, ensure you're comparing values where both values were created with `$state(...)`, or neither were. Note that `$state.raw(...)` will _not_ create a state proxy. diff --git a/packages/svelte/messages/compile-errors/script.md b/packages/svelte/messages/compile-errors/script.md index 6e15fa06d5..e84174e42a 100644 --- a/packages/svelte/messages/compile-errors/script.md +++ b/packages/svelte/messages/compile-errors/script.md @@ -122,6 +122,10 @@ > Cannot use rune without parentheses +## rune_removed + +> The `%name%` rune has been removed + ## rune_renamed > `%name%` is now `%replacement%` diff --git a/packages/svelte/src/ambient.d.ts b/packages/svelte/src/ambient.d.ts index 82242cb494..6af8ca5156 100644 --- a/packages/svelte/src/ambient.d.ts +++ b/packages/svelte/src/ambient.d.ts @@ -147,27 +147,6 @@ declare namespace $state { */ export function snapshot(state: T): Snapshot; - /** - * Compare two values, one or both of which is a reactive `$state(...)` proxy. - * - * Example: - * ```ts - * - * ``` - * - * https://svelte-5-preview.vercel.app/docs/runes#$state.is - * - */ - export function is(a: any, b: any): boolean; - // prevent intellisense from being unhelpful /** @deprecated */ export const apply: never; diff --git a/packages/svelte/src/compiler/errors.js b/packages/svelte/src/compiler/errors.js index c6c388fd3f..b174f69c20 100644 --- a/packages/svelte/src/compiler/errors.js +++ b/packages/svelte/src/compiler/errors.js @@ -348,6 +348,16 @@ export function rune_missing_parentheses(node) { e(node, "rune_missing_parentheses", "Cannot use rune without parentheses"); } +/** + * The `%name%` rune has been removed + * @param {null | number | NodeLike} node + * @param {string} name + * @returns {never} + */ +export function rune_removed(node, name) { + e(node, "rune_removed", `The \`${name}\` rune has been removed`); +} + /** * `%name%` is now `%replacement%` * @param {null | number | NodeLike} node diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js index 92edd00739..f9999f5252 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/CallExpression.js @@ -140,13 +140,6 @@ export function CallExpression(node, context) { e.rune_invalid_arguments_length(node, rune, 'exactly one argument'); } - break; - - case '$state.is': - if (node.arguments.length !== 2) { - e.rune_invalid_arguments_length(node, rune, 'exactly two arguments'); - } - break; } diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js index 1f3406e92e..79dccd5a7c 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js @@ -61,6 +61,10 @@ export function Identifier(node, context) { e.rune_renamed(parent, '$state.frozen', '$state.raw'); } + if (name === '$state.is') { + e.rune_removed(parent, '$state.is'); + } + e.rune_invalid_name(parent, name); } } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/CallExpression.js index d58dfe73a7..124b5cd269 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/CallExpression.js @@ -24,13 +24,6 @@ export function CallExpression(node, context) { is_ignored(node, 'state_snapshot_uncloneable') && b.true ); - case '$state.is': - return b.call( - '$.is', - /** @type {Expression} */ (context.visit(node.arguments[0])), - /** @type {Expression} */ (context.visit(node.arguments[1])) - ); - case '$effect.root': return b.call( '$.effect_root', diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js index d8106f9d7d..77177e397f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js @@ -28,8 +28,7 @@ export function VariableDeclaration(node, context) { rune === '$effect.tracking' || rune === '$effect.root' || rune === '$inspect' || - rune === '$state.snapshot' || - rune === '$state.is' + rune === '$state.snapshot' ) { if (init != null && is_hoisted_function(init)) { context.state.hoisted.push( diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js index 4b15a772c9..386c6b6ff3 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/CallExpression.js @@ -33,14 +33,6 @@ export function CallExpression(node, context) { ); } - if (rune === '$state.is') { - return b.call( - 'Object.is', - /** @type {Expression} */ (context.visit(node.arguments[0])), - /** @type {Expression} */ (context.visit(node.arguments[1])) - ); - } - if (rune === '$inspect' || rune === '$inspect().with') { return transform_inspect_rune(node, context); } diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index 286396703a..19a412726d 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -20,4 +20,5 @@ export const INSPECT_EFFECT = 1 << 17; export const HEAD_EFFECT = 1 << 18; export const STATE_SYMBOL = Symbol('$state'); +export const STATE_SYMBOL_METADATA = Symbol('$state metadata'); export const LOADING_ATTR_SYMBOL = Symbol(''); diff --git a/packages/svelte/src/internal/client/dev/equality.js b/packages/svelte/src/internal/client/dev/equality.js index 67fc039da1..c1c392ba87 100644 --- a/packages/svelte/src/internal/client/dev/equality.js +++ b/packages/svelte/src/internal/client/dev/equality.js @@ -20,10 +20,7 @@ export function init_array_prototype_warnings() { const test = indexOf.call(get_proxied_value(this), get_proxied_value(item), from_index); if (test !== -1) { - w.state_proxy_equality_mismatch( - 'array.indexOf(...)', - ': `array.findIndex(entry => $state.is(entry, item))`' - ); + w.state_proxy_equality_mismatch('array.indexOf(...)'); } } @@ -45,10 +42,7 @@ export function init_array_prototype_warnings() { ); if (test !== -1) { - w.state_proxy_equality_mismatch( - 'array.lastIndexOf(...)', - ': `array.findLastIndex(entry => $state.is(entry, item))`' - ); + w.state_proxy_equality_mismatch('array.lastIndexOf(...)'); } } @@ -62,10 +56,7 @@ export function init_array_prototype_warnings() { const test = includes.call(get_proxied_value(this), get_proxied_value(item), from_index); if (test) { - w.state_proxy_equality_mismatch( - 'array.includes(...)', - ': `array.some(entry => $state.is(entry, item))`' - ); + w.state_proxy_equality_mismatch('array.includes(...)'); } } @@ -88,7 +79,7 @@ export function init_array_prototype_warnings() { */ 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 ? '===' : '!==', ''); + w.state_proxy_equality_mismatch(equal ? '===' : '!=='); } return (a === b) === equal; @@ -102,7 +93,7 @@ export function strict_equals(a, b, equal = true) { */ export function equals(a, b, equal = true) { if ((a == b) !== (get_proxied_value(a) == get_proxied_value(b))) { - w.state_proxy_equality_mismatch(equal ? '==' : '!=', ''); + w.state_proxy_equality_mismatch(equal ? '==' : '!='); } return (a == b) === equal; diff --git a/packages/svelte/src/internal/client/dev/ownership.js b/packages/svelte/src/internal/client/dev/ownership.js index 62d4a43e94..8aa3b91d4e 100644 --- a/packages/svelte/src/internal/client/dev/ownership.js +++ b/packages/svelte/src/internal/client/dev/ownership.js @@ -1,7 +1,7 @@ /** @import { ProxyMetadata } from '#client' */ /** @typedef {{ file: string, line: number, column: number }} Location */ -import { STATE_SYMBOL } from '../constants.js'; +import { STATE_SYMBOL_METADATA } from '../constants.js'; import { render_effect, user_pre_effect } from '../reactivity/effects.js'; import { dev_current_component_function } from '../runtime.js'; import { get_prototype_of } from '../../shared/utils.js'; @@ -113,7 +113,7 @@ export function mark_module_end(component) { export function add_owner(object, owner, global = false, skip_warning = false) { if (object && !global) { const component = dev_current_component_function; - const metadata = object[STATE_SYMBOL]; + const metadata = object[STATE_SYMBOL_METADATA]; if (metadata && !has_owner(metadata, component)) { let original = get_owner(metadata); @@ -138,8 +138,8 @@ export function add_owner_effect(get_object, Component, skip_warning = false) { } /** - * @param {ProxyMetadata | null} from - * @param {ProxyMetadata} to + * @param {ProxyMetadata | null} from + * @param {ProxyMetadata} to */ export function widen_ownership(from, to) { if (to.owners === null) { @@ -166,7 +166,7 @@ export function widen_ownership(from, to) { * @param {Set} seen */ function add_owner_to_object(object, owner, seen) { - const metadata = /** @type {ProxyMetadata} */ (object?.[STATE_SYMBOL]); + const metadata = /** @type {ProxyMetadata} */ (object?.[STATE_SYMBOL_METADATA]); if (metadata) { // this is a state proxy, add owner directly, if not globally shared diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/this.js b/packages/svelte/src/internal/client/dom/elements/bindings/this.js index 6b5c439526..56b0a56e71 100644 --- a/packages/svelte/src/internal/client/dom/elements/bindings/this.js +++ b/packages/svelte/src/internal/client/dom/elements/bindings/this.js @@ -9,9 +9,9 @@ import { queue_micro_task } from '../../task.js'; * @returns {boolean} */ function is_bound_this(bound_value, element_or_component) { - // Find the original target if the value is proxied. - var proxy_target = bound_value && bound_value[STATE_SYMBOL]?.t; - return bound_value === element_or_component || proxy_target === element_or_component; + return ( + bound_value === element_or_component || bound_value?.[STATE_SYMBOL] === element_or_component + ); } /** diff --git a/packages/svelte/src/internal/client/errors.js b/packages/svelte/src/internal/client/errors.js index d6c04cf414..67b5c0550f 100644 --- a/packages/svelte/src/internal/client/errors.js +++ b/packages/svelte/src/internal/client/errors.js @@ -278,6 +278,22 @@ export function rune_outside_svelte(rune) { } } +/** + * Property descriptors defined on `$state` objects must contain `value` and always be `enumerable`, `configurable` and `writable`. + * @returns {never} + */ +export function state_descriptors_fixed() { + if (DEV) { + const error = new Error(`state_descriptors_fixed\nProperty descriptors defined on \`$state\` objects must contain \`value\` and always be \`enumerable\`, \`configurable\` and \`writable\`.`); + + error.name = 'Svelte error'; + throw error; + } else { + // TODO print a link to the documentation + throw new Error("state_descriptors_fixed"); + } +} + /** * Cannot set prototype of `$state` object * @returns {never} diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 5739e9646c..1ca71708af 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -150,7 +150,7 @@ export { validate_prop_bindings } from './validate.js'; export { raf } from './timing.js'; -export { proxy, is } from './proxy.js'; +export { proxy } from './proxy.js'; export { create_custom_element } from './dom/elements/custom-element.js'; export { child, diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 1827b1efda..333cd20b58 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/packages/svelte/src/internal/client/proxy.js @@ -3,16 +3,14 @@ import { DEV } from 'esm-env'; import { get, current_component_context, untrack, current_effect } from './runtime.js'; import { array_prototype, - define_property, get_descriptor, get_prototype_of, is_array, - is_frozen, object_prototype } from '../shared/utils.js'; import { check_ownership, widen_ownership } from './dev/ownership.js'; import { source, set } from './reactivity/sources.js'; -import { STATE_SYMBOL } from './constants.js'; +import { STATE_SYMBOL, STATE_SYMBOL_METADATA } from './constants.js'; import { UNINITIALIZED } from '../../constants.js'; import * as e from './errors.js'; @@ -24,261 +22,262 @@ import * as e from './errors.js'; * @returns {ProxyStateObject | T} */ export function proxy(value, parent = null, prev) { - if (typeof value === 'object' && value != null && !is_frozen(value)) { - // If we have an existing proxy, return it... - if (STATE_SYMBOL in value) { - const metadata = /** @type {ProxyMetadata} */ (value[STATE_SYMBOL]); - - // ...unless the proxy belonged to a different object, because - // someone copied the state symbol using `Reflect.ownKeys(...)` - if (metadata.t === value || metadata.p === value) { - if (DEV) { - // Since original parent relationship gets lost, we need to copy over ancestor owners - // into current metadata. The object might still exist on both, so we need to widen it. - widen_ownership(metadata, metadata); - metadata.parent = parent; - } + // if non-proxyable, or is already a proxy, return `value` + if (typeof value !== 'object' || value === null || STATE_SYMBOL in value) { + return value; + } - return metadata.p; - } + const prototype = get_prototype_of(value); + + if (prototype !== object_prototype && prototype !== array_prototype) { + return value; + } + + var sources = new Map(); + var is_proxied_array = is_array(value); + var version = source(0); + + /** @type {ProxyMetadata} */ + var metadata; + + if (DEV) { + metadata = { + parent, + owners: null + }; + + if (prev) { + // Reuse owners from previous state; necessary because reassignment is not guaranteed to have correct component context. + // If no previous proxy exists we play it safe and assume ownerless state + // @ts-expect-error + const prev_owners = prev.v?.[STATE_SYMBOL_METADATA]?.owners; + metadata.owners = prev_owners ? new Set(prev_owners) : null; + } else { + metadata.owners = + parent === null + ? current_component_context !== null + ? new Set([current_component_context.function]) + : null + : new Set(); } + } - const prototype = get_prototype_of(value); - - if (prototype === object_prototype || prototype === array_prototype) { - const proxy = new Proxy(value, state_proxy_handler); - - define_property(value, STATE_SYMBOL, { - value: /** @type {ProxyMetadata} */ ({ - s: new Map(), - v: source(0), - a: is_array(value), - p: proxy, - t: value - }), - writable: true, - enumerable: false - }); + return new Proxy(/** @type {any} */ (value), { + defineProperty(_, prop, descriptor) { + if ( + !('value' in descriptor) || + descriptor.configurable === false || + descriptor.enumerable === false || + descriptor.writable === false + ) { + // we disallow non-basic descriptors, because unless they are applied to the + // target object — which we avoid, so that state can be forked — we will run + // afoul of the various invariants + // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy/Proxy/getOwnPropertyDescriptor#invariants + e.state_descriptors_fixed(); + } - if (DEV) { - // @ts-expect-error - value[STATE_SYMBOL].parent = parent; - - if (prev) { - // Reuse owners from previous state; necessary because reassignment is not guaranteed to have correct component context. - // If no previous proxy exists we play it safe and assume ownerless state - // @ts-expect-error - const prev_owners = prev?.v?.[STATE_SYMBOL]?.owners; - // @ts-expect-error - value[STATE_SYMBOL].owners = prev_owners ? new Set(prev_owners) : null; - } else { - // @ts-expect-error - value[STATE_SYMBOL].owners = - parent === null - ? current_component_context !== null - ? new Set([current_component_context.function]) - : null - : new Set(); - } + var s = sources.get(prop); + + if (s === undefined) { + s = source(descriptor.value); + sources.set(prop, s); + } else { + set(s, proxy(descriptor.value, metadata)); } - return proxy; - } - } + return true; + }, - return value; -} + deleteProperty(target, prop) { + var s = sources.get(prop); + var exists = s !== undefined ? s.v !== UNINITIALIZED : prop in target; -/** - * @param {Source} signal - * @param {1 | -1} [d] - */ -function update_version(signal, d = 1) { - set(signal, signal.v + d); -} + if (s !== undefined) { + set(s, UNINITIALIZED); + } -/** @type {ProxyHandler>} */ -const state_proxy_handler = { - defineProperty(target, prop, descriptor) { - if (descriptor.value) { - /** @type {ProxyMetadata} */ - const metadata = target[STATE_SYMBOL]; + if (exists) { + update_version(version); + } - const s = metadata.s.get(prop); - if (s !== undefined) set(s, proxy(descriptor.value, metadata)); - } + return exists; + }, - return Reflect.defineProperty(target, prop, descriptor); - }, - - deleteProperty(target, prop) { - /** @type {ProxyMetadata} */ - const metadata = target[STATE_SYMBOL]; - const s = metadata.s.get(prop); - const is_array = metadata.a; - const boolean = delete target[prop]; - - // If we have mutated an array directly, and the deletion - // was successful we will also need to update the length - // before updating the field or the version. This is to - // ensure any effects observing length can execute before - // effects that listen to the fields – otherwise they will - // operate an an index that no longer exists. - if (is_array && boolean) { - const ls = metadata.s.get('length'); - const length = target.length - 1; - if (ls !== undefined && ls.v !== length) { - set(ls, length); + get(target, prop, receiver) { + if (DEV && prop === STATE_SYMBOL_METADATA) { + return metadata; } - } - if (s !== undefined) set(s, UNINITIALIZED); - if (boolean) { - update_version(metadata.v); - } + if (prop === STATE_SYMBOL) { + return value; + } - return boolean; - }, + var s = sources.get(prop); + var exists = prop in target; - get(target, prop, receiver) { - if (prop === STATE_SYMBOL) { - return Reflect.get(target, STATE_SYMBOL); - } + // create a source, but only if it's an own property and not a prototype property + if (s === undefined && (!exists || get_descriptor(target, prop)?.writable)) { + s = source(proxy(exists ? target[prop] : UNINITIALIZED, metadata)); + sources.set(prop, s); + } - /** @type {ProxyMetadata} */ - const metadata = target[STATE_SYMBOL]; - let s = metadata.s.get(prop); + if (s !== undefined) { + var v = get(s); + return v === UNINITIALIZED ? undefined : v; + } - // create a source, but only if it's an own property and not a prototype property - if (s === undefined && (!(prop in target) || get_descriptor(target, prop)?.writable)) { - s = source(proxy(target[prop], metadata)); - metadata.s.set(prop, s); - } + return Reflect.get(target, prop, receiver); + }, + + getOwnPropertyDescriptor(target, prop) { + var descriptor = Reflect.getOwnPropertyDescriptor(target, prop); + + if (descriptor && 'value' in descriptor) { + var s = sources.get(prop); + if (s) descriptor.value = get(s); + } else if (descriptor === undefined) { + var source = sources.get(prop); + var value = source?.v; + + if (source !== undefined && value !== UNINITIALIZED) { + return { + enumerable: true, + configurable: true, + value, + writable: true + }; + } + } - if (s !== undefined) { - const value = get(s); - return value === UNINITIALIZED ? undefined : value; - } + return descriptor; + }, - return Reflect.get(target, prop, receiver); - }, + has(target, prop) { + if (DEV && prop === STATE_SYMBOL_METADATA) { + return true; + } + + if (prop === STATE_SYMBOL) { + return true; + } - getOwnPropertyDescriptor(target, prop) { - const descriptor = Reflect.getOwnPropertyDescriptor(target, prop); - if (descriptor && 'value' in descriptor) { - /** @type {ProxyMetadata} */ - const metadata = target[STATE_SYMBOL]; - const s = metadata.s.get(prop); + var s = sources.get(prop); + var has = (s !== undefined && s.v !== UNINITIALIZED) || Reflect.has(target, prop); - if (s) { - descriptor.value = get(s); + if ( + s !== undefined || + (current_effect !== null && (!has || get_descriptor(target, prop)?.writable)) + ) { + if (s === undefined) { + s = source(has ? proxy(target[prop], metadata) : UNINITIALIZED); + sources.set(prop, s); + } + + var value = get(s); + if (value === UNINITIALIZED) { + return false; + } } - } - return descriptor; - }, + return has; + }, - has(target, prop) { - if (prop === STATE_SYMBOL) { - return true; - } - /** @type {ProxyMetadata} */ - const metadata = target[STATE_SYMBOL]; - const has = Reflect.has(target, prop); - - let s = metadata.s.get(prop); - if ( - s !== undefined || - (current_effect !== null && (!has || get_descriptor(target, prop)?.writable)) - ) { + set(target, prop, value, receiver) { + var s = sources.get(prop); + var has = prop in target; + + // If we haven't yet created a source for this property, we need to ensure + // we do so otherwise if we read it later, then the write won't be tracked and + // the heuristics of effects will be different vs if we had read the proxied + // object property before writing to that property. if (s === undefined) { - s = source(has ? proxy(target[prop], metadata) : UNINITIALIZED); - metadata.s.set(prop, s); - } - const value = get(s); - if (value === UNINITIALIZED) { - return false; + if (!has || get_descriptor(target, prop)?.writable) { + s = source(undefined); + set(s, proxy(value, metadata)); + sources.set(prop, s); + } + } else { + has = s.v !== UNINITIALIZED; + set(s, proxy(value, metadata)); } - } - return has; - }, - - set(target, prop, value, receiver) { - /** @type {ProxyMetadata} */ - const metadata = target[STATE_SYMBOL]; - let s = metadata.s.get(prop); - // If we haven't yet created a source for this property, we need to ensure - // we do so otherwise if we read it later, then the write won't be tracked and - // the heuristics of effects will be different vs if we had read the proxied - // object property before writing to that property. - if (s === undefined) { - // the read creates a signal - untrack(() => receiver[prop]); - s = metadata.s.get(prop); - } - if (s !== undefined) { - set(s, proxy(value, metadata)); - } - const is_array = metadata.a; - const not_has = !(prop in target); - - if (DEV) { - /** @type {ProxyMetadata | undefined} */ - const prop_metadata = value?.[STATE_SYMBOL]; - if (prop_metadata && prop_metadata?.parent !== metadata) { - widen_ownership(metadata, prop_metadata); + + if (DEV) { + /** @type {ProxyMetadata | undefined} */ + var prop_metadata = value?.[STATE_SYMBOL_METADATA]; + if (prop_metadata && prop_metadata?.parent !== metadata) { + widen_ownership(metadata, prop_metadata); + } + check_ownership(metadata); } - check_ownership(metadata); - } - // variable.length = value -> clear all signals with index >= value - if (is_array && prop === 'length') { - for (let i = value; i < target.length; i += 1) { - const s = metadata.s.get(i + ''); - if (s !== undefined) set(s, UNINITIALIZED); + // variable.length = value -> clear all signals with index >= value + if (is_proxied_array && prop === 'length') { + for (var i = value; i < target.length; i += 1) { + var other_s = sources.get(i + ''); + if (other_s !== undefined) set(other_s, UNINITIALIZED); + } } - } - var descriptor = Reflect.getOwnPropertyDescriptor(target, prop); + var descriptor = Reflect.getOwnPropertyDescriptor(target, prop); - // Set the new value before updating any signals so that any listeners get the new value - if (descriptor?.set) { - descriptor.set.call(receiver, value); - } else { - target[prop] = value; - } + // Set the new value before updating any signals so that any listeners get the new value + if (descriptor?.set) { + descriptor.set.call(receiver, value); + } - if (not_has) { - // If we have mutated an array directly, we might need to - // signal that length has also changed. Do it before updating metadata - // to ensure that iterating over the array as a result of a metadata update - // will not cause the length to be out of sync. - if (is_array) { - const ls = metadata.s.get('length'); - const length = target.length; - if (ls !== undefined && ls.v !== length) { - set(ls, length); + if (!has) { + // If we have mutated an array directly, we might need to + // signal that length has also changed. Do it before updating metadata + // to ensure that iterating over the array as a result of a metadata update + // will not cause the length to be out of sync. + if (is_proxied_array && typeof prop === 'string') { + var ls = sources.get('length'); + + if (ls !== undefined) { + var n = Number(prop); + + if (Number.isInteger(n) && n >= ls.v) { + set(ls, n + 1); + } + } } + + update_version(version); } - update_version(metadata.v); - } - return true; - }, + return true; + }, - ownKeys(target) { - /** @type {ProxyMetadata} */ - const metadata = target[STATE_SYMBOL]; + ownKeys(target) { + get(version); - get(metadata.v); - return Reflect.ownKeys(target); - } -}; + var own_keys = Reflect.ownKeys(target).filter((key) => { + var source = sources.get(key); + return source === undefined || source.v !== UNINITIALIZED; + }); + + for (var [key, source] of sources) { + if (source.v !== UNINITIALIZED && !(key in target)) { + own_keys.push(key); + } + } + + return own_keys; + }, + + setPrototypeOf() { + e.state_prototype_fixed(); + } + }); +} -if (DEV) { - state_proxy_handler.setPrototypeOf = () => { - e.state_prototype_fixed(); - }; +/** + * @param {Source} signal + * @param {1 | -1} [d] + */ +function update_version(signal, d = 1) { + set(signal, signal.v + d); } /** @@ -286,11 +285,9 @@ if (DEV) { */ export function get_proxied_value(value) { if (value !== null && typeof value === 'object' && STATE_SYMBOL in value) { - var metadata = value[STATE_SYMBOL]; - if (metadata) { - return metadata.p; - } + return value[STATE_SYMBOL]; } + return value; } diff --git a/packages/svelte/src/internal/client/proxy.test.ts b/packages/svelte/src/internal/client/proxy.test.ts new file mode 100644 index 0000000000..d73e6bef72 --- /dev/null +++ b/packages/svelte/src/internal/client/proxy.test.ts @@ -0,0 +1,87 @@ +import { proxy } from './proxy'; +import { assert, test } from 'vitest'; + +test('does not mutate the original object', () => { + const original = { x: 1 }; + const state = proxy(original); + + state.x = 2; + + assert.equal(original.x, 1); + assert.equal(state.x, 2); +}); + +test('preserves getters', () => { + let count = 0; + const original = { + count: 0, + get x() { + this.count += 1; + count += 1; + return 42; + } + }; + + const state = proxy(original); + + // eslint-disable-next-line @typescript-eslint/no-unused-expressions + state.x; + // eslint-disable-next-line @typescript-eslint/no-unused-expressions + state.x; + + assert.equal(original.count, 0); + assert.equal(count, 2); + assert.equal(state.count, 2); +}); + +test('defines a property', () => { + const original = { y: 0 }; + const state = proxy(original); + + let value = 0; + + Object.defineProperty(state, 'x', { + value: 1 + }); + Object.defineProperty(state, 'y', { + value: 1 + }); + + assert.equal(state.x, 1); + assert.deepEqual(Object.getOwnPropertyDescriptor(state, 'x'), { + configurable: true, + writable: true, + value: 1, + enumerable: true + }); + + assert.ok(!('x' in original)); + assert.deepEqual(Object.getOwnPropertyDescriptor(original, 'y'), { + configurable: true, + writable: true, + value: 0, + enumerable: true + }); + + assert.throws( + () => + Object.defineProperty(state, 'x', { + get: () => value, + set: (v) => (value = v) + }), + /state_descriptors_fixed/ + ); +}); + +test('does not re-proxy proxies', () => { + const inner = proxy({ count: 0 }); + const outer = proxy({ inner }); + + assert.equal(inner.count, 0); + assert.equal(outer.inner.count, 0); + + inner.count += 1; + + assert.equal(inner.count, 1); + assert.equal(outer.inner.count, 1); +}); diff --git a/packages/svelte/src/internal/client/types.d.ts b/packages/svelte/src/internal/client/types.d.ts index 78b1d8d6fd..9a720a02a2 100644 --- a/packages/svelte/src/internal/client/types.d.ts +++ b/packages/svelte/src/internal/client/types.d.ts @@ -173,25 +173,16 @@ export type TaskCallback = (now: number) => boolean | void; export type TaskEntry = { c: TaskCallback; f: () => void }; -export interface ProxyMetadata> { - /** A map of signals associated to the properties that are reactive */ - s: Map>; - /** A version counter, used within the proxy to signal changes in places where there's no other way to signal an update */ - v: Source; - /** `true` if the proxified object is an array */ - a: boolean; - /** The associated proxy */ - p: ProxyStateObject; - /** The original target this proxy was created for */ - t: T; - /** Dev-only — the components that 'own' this state, if any. `null` means no owners, i.e. everyone can mutate this state. */ +/** Dev-only */ +export interface ProxyMetadata { + /** The components that 'own' this state, if any. `null` means no owners, i.e. everyone can mutate this state. */ owners: null | Set; - /** Dev-only — the parent metadata object */ + /** The parent metadata object */ parent: null | ProxyMetadata; } export type ProxyStateObject> = T & { - [STATE_SYMBOL]: ProxyMetadata; + [STATE_SYMBOL]: T; }; export * from './reactivity/types'; diff --git a/packages/svelte/src/internal/client/warnings.js b/packages/svelte/src/internal/client/warnings.js index b7268ee1d7..36d7345b86 100644 --- a/packages/svelte/src/internal/client/warnings.js +++ b/packages/svelte/src/internal/client/warnings.js @@ -128,13 +128,12 @@ export function ownership_invalid_mutation(component, owner) { } /** - * 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%details% + * Reactive `$state(...)` proxies and the values they proxy have different identities. Because of this, comparisons with `%operator%` will produce unexpected results * @param {string} operator - * @param {string} details */ -export function state_proxy_equality_mismatch(operator, details) { +export function state_proxy_equality_mismatch(operator) { if (DEV) { - console.warn(`%c[svelte] state_proxy_equality_mismatch\n%cReactive \`$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${details}`, bold, normal); + console.warn(`%c[svelte] state_proxy_equality_mismatch\n%cReactive \`$state(...)\` proxies and the values they proxy have different identities. Because of this, comparisons with \`${operator}\` will produce unexpected results`, bold, normal); } else { // TODO print a link to the documentation console.warn("state_proxy_equality_mismatch"); diff --git a/packages/svelte/src/internal/shared/utils.js b/packages/svelte/src/internal/shared/utils.js index c2f0b24c11..d843413e57 100644 --- a/packages/svelte/src/internal/shared/utils.js +++ b/packages/svelte/src/internal/shared/utils.js @@ -3,7 +3,6 @@ export var is_array = Array.isArray; export var array_from = Array.from; export var object_keys = Object.keys; -export var is_frozen = Object.isFrozen; export var define_property = Object.defineProperty; export var get_descriptor = Object.getOwnPropertyDescriptor; export var get_descriptors = Object.getOwnPropertyDescriptors; diff --git a/packages/svelte/src/utils.js b/packages/svelte/src/utils.js index 0cf50bd774..bf241c7947 100644 --- a/packages/svelte/src/utils.js +++ b/packages/svelte/src/utils.js @@ -395,7 +395,6 @@ const RUNES = /** @type {const} */ ([ '$state', '$state.raw', '$state.snapshot', - '$state.is', '$props', '$bindable', '$derived', diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-shared/_config.js b/packages/svelte/tests/runtime-runes/samples/proxy-not-shared/_config.js similarity index 91% rename from packages/svelte/tests/runtime-runes/samples/proxy-shared/_config.js rename to packages/svelte/tests/runtime-runes/samples/proxy-not-shared/_config.js index 682c0467fc..0f63d1c4d0 100644 --- a/packages/svelte/tests/runtime-runes/samples/proxy-shared/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/proxy-not-shared/_config.js @@ -18,7 +18,7 @@ export default test({ target.innerHTML, ` - + ` ); @@ -29,8 +29,8 @@ export default test({ assert.htmlEqual( target.innerHTML, ` - - + + ` ); } diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-shared/main.svelte b/packages/svelte/tests/runtime-runes/samples/proxy-not-shared/main.svelte similarity index 100% rename from packages/svelte/tests/runtime-runes/samples/proxy-shared/main.svelte rename to packages/svelte/tests/runtime-runes/samples/proxy-not-shared/main.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/state-is/_config.js b/packages/svelte/tests/runtime-runes/samples/state-is/_config.js deleted file mode 100644 index 27aabe8213..0000000000 --- a/packages/svelte/tests/runtime-runes/samples/state-is/_config.js +++ /dev/null @@ -1,7 +0,0 @@ -import { test } from '../../test'; - -export default test({ - async test({ assert, logs }) { - assert.deepEqual(logs, [false, true]); - } -}); diff --git a/packages/svelte/tests/runtime-runes/samples/state-is/main.svelte b/packages/svelte/tests/runtime-runes/samples/state-is/main.svelte deleted file mode 100644 index ff23de439a..0000000000 --- a/packages/svelte/tests/runtime-runes/samples/state-is/main.svelte +++ /dev/null @@ -1,10 +0,0 @@ - diff --git a/packages/svelte/tests/server-side-rendering/samples/state-is/_expected.html b/packages/svelte/tests/server-side-rendering/samples/state-is/_expected.html deleted file mode 100644 index f9f47bcf0a..0000000000 --- a/packages/svelte/tests/server-side-rendering/samples/state-is/_expected.html +++ /dev/null @@ -1,4 +0,0 @@ -

true

-

true

-

true

-

true

\ No newline at end of file diff --git a/packages/svelte/tests/server-side-rendering/samples/state-is/main.svelte b/packages/svelte/tests/server-side-rendering/samples/state-is/main.svelte deleted file mode 100644 index 17a859cd87..0000000000 --- a/packages/svelte/tests/server-side-rendering/samples/state-is/main.svelte +++ /dev/null @@ -1,10 +0,0 @@ - - -

{a === obj}

-

{$state.is(a, obj)}

-

{a === b}

-

{$state.is(a, b)}

diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 36f7a97229..5b527db2e4 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -2935,27 +2935,6 @@ declare namespace $state { */ export function snapshot(state: T): Snapshot; - /** - * Compare two values, one or both of which is a reactive `$state(...)` proxy. - * - * Example: - * ```ts - * - * ``` - * - * https://svelte-5-preview.vercel.app/docs/runes#$state.is - * - */ - export function is(a: any, b: any): boolean; - // prevent intellisense from being unhelpful /** @deprecated */ export const apply: never;