fix: always use set for private identifiers (#14378)

* fix: always use set for private identifiers

* we can simplify this further - no need to check the value was transformed, since the outcome of not returning immediately is the same but with extra steps

* add explanatory note

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>

.changeset/tiny-trainers-sleep.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: always use set for private identifiers

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

@@ -32,31 +32,26 @@ function build_assignment(operator, left, right, context) {
 	if (
 		context.state.analysis.runes &&
 		left.type === 'MemberExpression' &&
-		left.object.type === 'ThisExpression'
+		left.property.type === 'PrivateIdentifier'
 	) {
-		if (left.property.type === 'PrivateIdentifier') {
+		const private_state = context.state.private_state.get(left.property.name);
-			const private_state = context.state.private_state.get(left.property.name);
+
-
+		if (private_state !== undefined) {
-			if (private_state !== undefined) {
+			let value = /** @type {Expression} */ (
-				let transformed = false;
+				context.visit(build_assignment_value(operator, left, right))
-				let value = /** @type {Expression} */ (
+			);
-					context.visit(build_assignment_value(operator, left, right))
+
-				);
+			if (private_state.kind !== 'raw_state' && should_proxy(value, context.state.scope)) {
-
+				value = build_proxy_reassignment(value, b.member(b.this, private_state.id));
-				if (should_proxy(value, context.state.scope)) {
-					transformed = true;
-					value =
-						private_state.kind === 'raw_state'
-							? value
-							: build_proxy_reassignment(value, b.member(b.this, private_state.id));
-				}
-
-				if (!context.state.in_constructor) {
-					return b.call('$.set', left, value);
-				} else if (transformed) {
-					return b.assignment(operator, /** @type {Pattern} */ (context.visit(left)), value);
-				}
 			}
+
+			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);
+			}
+
+			return b.call('$.set', left, value);
 		}
 	}
 

packages/svelte/tests/runtime-runes/samples/private-identifiers-not-this/_config.js

@@ -0,0 +1,17 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	html: `<p>42</p><p>1337</p><button></button>`,
+	async test({ assert, target, instance }) {
+		const [a, b] = target.querySelectorAll('p');
+		const btn = target.querySelector('button');
+
+		flushSync(() => {
+			btn?.click();
+		});
+
+		assert.equal(a.textContent, '1337');
+		assert.equal(b.textContent, '42');
+	}
+});

packages/svelte/tests/runtime-runes/samples/private-identifiers-not-this/main.svelte

@@ -0,0 +1,26 @@
+<script>
+	class Box {
+		#value = $state(0);
+
+		get value(){
+			return this.#value;
+		}
+		
+		constructor(num){
+			this.#value = num;
+		}
+		
+		swap(other) {
+			const value = this.#value;
+			this.#value = other.value;
+			other.#value = value;
+		}
+	}
+
+	const a = new Box(42);
+	const b = new Box(1337);
+</script>
+
+<p>{a.value}</p>
+<p>{b.value}</p>
+<button onclick={()=>{a.swap(b)}}></button>