Skip to content

breaking: overhaul proxies, remove $state.is #12916

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 22 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fifty-actors-agree.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

breaking: disallow `Object.defineProperty` on state proxies with non-basic descriptors
5 changes: 5 additions & 0 deletions .changeset/gorgeous-pans-sort.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

breaking: allow frozen objects to be proxied
5 changes: 5 additions & 0 deletions .changeset/heavy-houses-pay.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

breaking: avoid mutations to underlying proxied object with $state
5 changes: 5 additions & 0 deletions .changeset/short-starfishes-beg.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

breaking: remove $state.is rune
22 changes: 0 additions & 22 deletions documentation/docs/03-runes/01-state.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
<script>
let foo = $state({});
let bar = {};

foo.bar = bar;

console.log(foo.bar === bar); // false — `foo.bar` is a reactive proxy
console.log($state.is(foo.bar, bar)); // true
</script>
```

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:
Expand Down
4 changes: 4 additions & 0 deletions packages/svelte/messages/client-errors/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 2 additions & 13 deletions packages/svelte/messages/client-warnings/warnings.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`:

Expand All @@ -57,15 +57,4 @@
</script>
```

In the rare case that you need to compare them, you can use `$state.is`, which unwraps proxies:

```svelte
<script>
let value = { foo: 'bar' };
let proxy = $state(value);

$state.is(value, proxy); // true
</script>
```

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.
4 changes: 4 additions & 0 deletions packages/svelte/messages/compile-errors/script.md
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@

> Cannot use rune without parentheses

## rune_removed

> The `%name%` rune has been removed

## rune_renamed

> `%name%` is now `%replacement%`
Expand Down
21 changes: 0 additions & 21 deletions packages/svelte/src/ambient.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,27 +147,6 @@ declare namespace $state {
*/
export function snapshot<T>(state: T): Snapshot<T>;

/**
* Compare two values, one or both of which is a reactive `$state(...)` proxy.
*
* Example:
* ```ts
* <script>
* let foo = $state({});
* let bar = {};
*
* foo.bar = bar;
*
* console.log(foo.bar === bar); // false — `foo.bar` is a reactive proxy
* console.log($state.is(foo.bar, bar)); // true
* </script>
* ```
*
* 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;
Expand Down
10 changes: 10 additions & 0 deletions packages/svelte/src/compiler/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
1 change: 1 addition & 0 deletions packages/svelte/src/internal/client/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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('');
19 changes: 5 additions & 14 deletions packages/svelte/src/internal/client/dev/equality.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(...)');
}
}

Expand All @@ -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(...)');
}
}

Expand All @@ -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(...)');
}
}

Expand All @@ -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;
Expand All @@ -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;
Expand Down
10 changes: 5 additions & 5 deletions packages/svelte/src/internal/client/dev/ownership.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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);

Expand All @@ -138,8 +138,8 @@ export function add_owner_effect(get_object, Component, skip_warning = false) {
}

/**
* @param {ProxyMetadata<any> | null} from
* @param {ProxyMetadata<any>} to
* @param {ProxyMetadata | null} from
* @param {ProxyMetadata} to
*/
export function widen_ownership(from, to) {
if (to.owners === null) {
Expand All @@ -166,7 +166,7 @@ export function widen_ownership(from, to) {
* @param {Set<any>} 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
}

/**
Expand Down
16 changes: 16 additions & 0 deletions packages/svelte/src/internal/client/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
2 changes: 1 addition & 1 deletion packages/svelte/src/internal/client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading
Loading