fix: simplify set calls for proxyable values (#15548)

* chore: simplify set calls for proxyable values

* changeset

Author: Rich-Harris

.changeset/nine-laws-rush.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: simplify set calls for proxyable values

packages/svelte/src/compiler/phases/3-transform/client/types.d.ts

@@ -30,7 +30,7 @@ export interface ClientTransformState extends TransformState {
 			/** turn `foo` into e.g. `$.get(foo)` */
 			read: (id: Identifier) => Expression;
 			/** turn `foo = bar` into e.g. `$.set(foo, bar)` */
-			assign?: (node: Identifier, value: Expression) => Expression;
+			assign?: (node: Identifier, value: Expression, proxy?: boolean) => Expression;
 			/** turn `foo.bar = baz` into e.g. `$.mutate(foo, $.get(foo).bar = baz);` */
 			mutate?: (node: Identifier, mutation: AssignmentExpression | UpdateExpression) => Expression;
 			/** turn `foo++` into e.g. `$.update(foo)` */

packages/svelte/src/compiler/phases/3-transform/client/utils.js

@@ -45,14 +45,6 @@ export function build_getter(node, state) {
 	return node;
 }
 
-/**
- * @param {Expression} value
- * @param {Expression} previous
- */
-export function build_proxy_reassignment(value, previous) {
-	return dev ? b.call('$.proxy', value, b.null, previous) : b.call('$.proxy', value);
-}
-
 /**
  * @param {FunctionDeclaration | FunctionExpression | ArrowFunctionExpression} node
  * @param {ComponentContext} context

packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js

@@ -1,5 +1,4 @@
-/** @import { Location } from 'locate-character' */
-/** @import { AssignmentExpression, AssignmentOperator, Expression, Identifier, Literal, MemberExpression, Pattern } from 'estree' */
+/** @import { AssignmentExpression, AssignmentOperator, Expression, Identifier, Pattern } from 'estree' */
 /** @import { AST } from '#compiler' */
 /** @import { Context } from '../types.js' */
 import * as b from '../../../../utils/builders.js';
@@ -8,8 +7,8 @@ import {
 	get_attribute_expression,
 	is_event_attribute
 } from '../../../../utils/ast.js';
-import { dev, filename, is_ignored, locate_node, locator } from '../../../../state.js';
-import { build_proxy_reassignment, should_proxy } from '../utils.js';
+import { dev, is_ignored, locate_node } from '../../../../state.js';
+import { should_proxy } from '../utils.js';
 import { visit_assignment_expression } from '../../shared/assignments.js';
 
 /**
@@ -65,21 +64,12 @@ function build_assignment(operator, left, right, context) {
 				context.visit(build_assignment_value(operator, left, right))
 			);
 
-			if (
+			const needs_proxy =
 				private_state.kind === 'state' &&
 				is_non_coercive_operator(operator) &&
-				should_proxy(value, context.state.scope)
-			) {
-				value = build_proxy_reassignment(value, b.member(b.this, private_state.id));
-			}
-
-			if (context.state.in_constructor) {
-				// inside the constructor, we can assign to `this.#foo.v` rather than using `$.set`,
-				// since nothing is tracking the signal at this point
-				return b.assignment(operator, /** @type {Pattern} */ (context.visit(left)), value);
-			}
+				should_proxy(value, context.state.scope);
 
-			return b.call('$.set', left, value);
+			return b.call('$.set', left, value, needs_proxy && b.true);
 		}
 	}
 
@@ -113,20 +103,18 @@ function build_assignment(operator, left, right, context) {
 			context.visit(build_assignment_value(operator, left, right))
 		);
 
-		if (
+		return transform.assign(
+			object,
+			value,
 			!is_primitive &&
-			binding.kind !== 'prop' &&
-			binding.kind !== 'bindable_prop' &&
-			binding.kind !== 'raw_state' &&
-			binding.kind !== 'store_sub' &&
-			context.state.analysis.runes &&
-			should_proxy(right, context.state.scope) &&
-			is_non_coercive_operator(operator)
-		) {
-			value = build_proxy_reassignment(value, object);
-		}
-
-		return transform.assign(object, value);
+				binding.kind !== 'prop' &&
+				binding.kind !== 'bindable_prop' &&
+				binding.kind !== 'raw_state' &&
+				binding.kind !== 'store_sub' &&
+				context.state.analysis.runes &&
+				should_proxy(right, context.state.scope) &&
+				is_non_coercive_operator(operator)
+		);
 	}
 
 	// mutation

packages/svelte/src/compiler/phases/3-transform/client/visitors/ClassBody.js

@@ -5,7 +5,7 @@ import { dev, is_ignored } from '../../../../state.js';
 import * as b from '../../../../utils/builders.js';
 import { regex_invalid_identifier_chars } from '../../../patterns.js';
 import { get_rune } from '../../../scope.js';
-import { build_proxy_reassignment, should_proxy } from '../utils.js';
+import { should_proxy } from '../utils.js';
 
 /**
  * @param {ClassBody} node
@@ -142,29 +142,20 @@ export function ClassBody(node, context) {
 					// get foo() { return this.#foo; }
 					body.push(b.method('get', definition.key, [], [b.return(b.call('$.get', member))]));
 
-					if (field.kind === 'state') {
+					if (field.kind === 'state' || field.kind === 'raw_state') {
 						// set foo(value) { this.#foo = value; }
 						const value = b.id('value');
-						const prev = b.member(b.this, field.id);
 
 						body.push(
 							b.method(
 								'set',
 								definition.key,
 								[value],
-								[b.stmt(b.call('$.set', member, build_proxy_reassignment(value, prev)))]
+								[b.stmt(b.call('$.set', member, value, field.kind === 'state' && b.true))]
 							)
 						);
 					}
 
-					if (field.kind === 'raw_state') {
-						// set foo(value) { this.#foo = value; }
-						const value = b.id('value');
-						body.push(
-							b.method('set', definition.key, [value], [b.stmt(b.call('$.set', member, value))])
-						);
-					}
-
 					if (dev && (field.kind === 'derived' || field.kind === 'derived_by')) {
 						body.push(
 							b.method(

packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js

@@ -24,8 +24,8 @@ export function add_state_transformers(context) {
 		) {
 			context.state.transform[name] = {
 				read: binding.declaration_kind === 'var' ? (node) => b.call('$.safe_get', node) : get_value,
-				assign: (node, value) => {
-					let call = b.call('$.set', node, value);
+				assign: (node, value, proxy = false) => {
+					let call = b.call('$.set', node, value, proxy && b.true);
 
 					if (context.state.scope.get(`$${node.name}`)?.kind === 'store_sub') {
 						call = b.call('$.store_unsub', call, b.literal(`$${node.name}`), b.id('$$stores'));

packages/svelte/src/internal/client/reactivity/sources.js

@@ -33,6 +33,7 @@ import * as e from '../errors.js';
 import { legacy_mode_flag, tracing_mode_flag } from '../../flags/index.js';
 import { get_stack } from '../dev/tracing.js';
 import { component_context, is_runes } from '../context.js';
+import { proxy } from '../proxy.js';
 
 export let inspect_effects = new Set();
 export const old_values = new Map();
@@ -143,9 +144,10 @@ export function mutate(source, value) {
  * @template V
  * @param {Source<V>} source
  * @param {V} value
+ * @param {boolean} [should_proxy]
  * @returns {V}
  */
-export function set(source, value) {
+export function set(source, value, should_proxy = false) {
 	if (
 		active_reaction !== null &&
 		!untracking &&
@@ -158,7 +160,9 @@ export function set(source, value) {
 		e.state_unsafe_mutation();
 	}
 
-	return internal_set(source, value);
+	let new_value = should_proxy ? proxy(value, null, source) : value;
+
+	return internal_set(source, new_value);
 }
 
 /**

packages/svelte/tests/snapshot/samples/bind-component-snippet/_expected/client/index.svelte.js

@@ -23,7 +23,7 @@ export default function Bind_component_snippet($$anchor) {
 			return $.get(value);
 		},
 		set value($$value) {
-			$.set(value, $.proxy($$value));
+			$.set(value, $$value, true);
 		}
 	});
 

packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/client/index.svelte.js

@@ -12,14 +12,14 @@ export default function Class_state_field_constructor_assignment($$anchor, $$pro
 		}
 
 		set a(value) {
-			$.set(this.#a, $.proxy(value));
+			$.set(this.#a, value, true);
 		}
 
 		#b = $.state();
 
 		constructor() {
 			this.a = 1;
-			this.#b.v = 2;
+			$.set(this.#b, 2);
 		}
 	}
 

packages/svelte/tests/snapshot/samples/destructured-assignments/_expected/client/index.svelte.js

@@ -8,8 +8,8 @@ let d = 4;
 
 export function update(array) {
 	(
-		$.set(a, $.proxy(array[0])),
-		$.set(b, $.proxy(array[1]))
+		$.set(a, array[0], true),
+		$.set(b, array[1], true)
 	);
 
 	[c, d] = array;

packages/svelte/tests/snapshot/samples/function-prop-no-getter/_expected/client/index.svelte.js

@@ -13,7 +13,7 @@ export default function Function_prop_no_getter($$anchor) {
 	Button($$anchor, {
 		onmousedown: () => $.set(count, $.get(count) + 1),
 		onmouseup,
-		onmouseenter: () => $.set(count, $.proxy(plusOne($.get(count)))),
+		onmouseenter: () => $.set(count, plusOne($.get(count)), true),
 		children: ($$anchor, $$slotProps) => {
 			$.next();