Merge 2d2bae52dc into 5895c637b0

.changeset/class-instance-props-binding.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: bindings now propagate writes through props that come from class instances (spread or `mount`)

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

@@ -7,7 +7,7 @@ import {
 	PROPS_IS_RUNES,
 	PROPS_IS_UPDATED
 } from '../../../constants.js';
-import { get_descriptor, is_function } from '../../shared/utils.js';
+import { get_descriptor_in_chain, is_function } from '../../shared/utils.js';
 import { set, source, update } from './sources.js';
 import { derived, derived_safe_equal } from './deriveds.js';
 import {
@@ -198,9 +198,9 @@ const spread_props_handler = {
 		while (i--) {
 			let p = target.props[i];
 			if (is_function(p)) p = p();
-			const desc = get_descriptor(p, key);
+			const desc = get_descriptor_in_chain(p, key);
 			if (desc && desc.set) {
-				desc.set(value);
+				desc.set.call(p, value);
 				return true;
 			}
 		}
@@ -212,7 +212,7 @@ const spread_props_handler = {
 			let p = target.props[i];
 			if (is_function(p)) p = p();
 			if (typeof p === 'object' && p !== null && key in p) {
-				const descriptor = get_descriptor(p, key);
+				const descriptor = get_descriptor_in_chain(p, key);
 				if (descriptor && !descriptor.configurable) {
 					// Prevent a "Non-configurability Report Error": The target is an array, it does
 					// not actually contain this property. If it is now described as non-configurable,
@@ -307,9 +307,11 @@ export function prop(props, key, flags, fallback) {
 		// or `createClassComponent(Component, props)`
 		var is_entry_props = STATE_SYMBOL in props || LEGACY_PROPS in props;
 
-		setter =
-			get_descriptor(props, key)?.set ??
-			(is_entry_props && key in props ? (v) => (props[key] = v) : undefined);
+		// Going through `props[key] = v` (rather than calling the descriptor's `set` directly)
+		// preserves `this` for prototype setters (class instances, including class fields with
+		// `$state`) and re-enters proxy traps so spread/state proxies route writes correctly.
+		var has_setter = get_descriptor_in_chain(props, key)?.set !== undefined;
+		setter = has_setter || (is_entry_props && key in props) ? (v) => (props[key] = v) : undefined;
 	}
 
 	/** @type {V} */

packages/svelte/src/internal/server/index.js

@@ -3,7 +3,7 @@
 /** @import { Store } from '#shared' */
 export { FILENAME, HMR } from '../../constants.js';
 import { attr, clsx, to_class, to_style } from '../shared/attributes.js';
-import { is_promise, noop } from '../shared/utils.js';
+import { get_descriptor_in_chain, is_promise, noop } from '../shared/utils.js';
 import { subscribe_to_store } from '../../store/utils.js';
 import {
 	UNINITIALIZED,
@@ -188,23 +188,57 @@ export function attributes(attrs, css_hash, classes, styles, flags = 0) {
 export function spread_props(props) {
 	/** @type {Record<string, unknown>} */
 	const merged_props = {};
-	let key;
 
 	for (let i = 0; i < props.length; i++) {
 		const obj = props[i];
 		if (obj == null) continue;
-		for (key of Object.keys(obj)) {
-			const desc = Object.getOwnPropertyDescriptor(obj, key);
-			if (desc) {
-				Object.defineProperty(merged_props, key, desc);
-			} else {
-				merged_props[key] = obj[key];
+		// `for..in` collects own + inherited enumerable string keys; class accessors
+		// aren't enumerable so we follow up with a prototype walk (excluding Object.prototype).
+		/** @type {Set<string>} */
+		const seen = new Set();
+		for (const key in obj) {
+			seen.add(key);
+			copy_prop(merged_props, obj, key);
+		}
+		let proto = Object.getPrototypeOf(obj);
+		while (proto != null && proto !== Object.prototype) {
+			for (const key of Object.getOwnPropertyNames(proto)) {
+				if (key === 'constructor' || seen.has(key)) continue;
+				seen.add(key);
+				copy_prop(merged_props, obj, key);
 			}
+			proto = Object.getPrototypeOf(proto);
 		}
 	}
 	return merged_props;
 }
 
+/**
+ * Copies a property from `source` to `target`. Accessors are bound to `source`
+ * so `this` resolves correctly when reading/writing through `target` (relevant
+ * for class instances spread into props).
+ * @param {Record<string, unknown>} target
+ * @param {Record<string, unknown>} source
+ * @param {string} key
+ */
+function copy_prop(target, source, key) {
+	const desc = get_descriptor_in_chain(source, key);
+	if (!desc) {
+		target[key] = source[key];
+		return;
+	}
+	if (desc.get || desc.set) {
+		Object.defineProperty(target, key, {
+			enumerable: true,
+			configurable: true,
+			get: desc.get && desc.get.bind(source),
+			set: desc.set && desc.set.bind(source)
+		});
+	} else {
+		Object.defineProperty(target, key, desc);
+	}
+}
+
 /**
  * @param {unknown} value
  * @returns {string}
@@ -401,7 +435,7 @@ export function bind_props(props_parent, props_now) {
 		if (
 			initial_value === undefined &&
 			value !== undefined &&
-			Object.getOwnPropertyDescriptor(props_parent, key)?.set
+			get_descriptor_in_chain(props_parent, key)?.set
 		) {
 			props_parent[key] = value;
 		}

packages/svelte/src/internal/shared/utils.js

@@ -11,6 +11,25 @@ export var get_descriptors = Object.getOwnPropertyDescriptors;
 export var object_prototype = Object.prototype;
 export var array_prototype = Array.prototype;
 export var get_prototype_of = Object.getPrototypeOf;
+
+/**
+ * Walks the prototype chain to find the property descriptor for `key`.
+ * Stops at `Object.prototype` so plain-object lookups behave the same as
+ * `Object.getOwnPropertyDescriptor`. Used so that class instances — whose
+ * accessors live on the prototype — work the same as POJOs in places where
+ * we need to detect a getter/setter pair (e.g. spread/`mount` props).
+ * @param {any} obj
+ * @param {string | symbol} key
+ * @returns {PropertyDescriptor | undefined}
+ */
+export function get_descriptor_in_chain(obj, key) {
+	var current = obj;
+	while (current != null && current !== object_prototype) {
+		var descriptor = get_descriptor(current, key);
+		if (descriptor !== undefined) return descriptor;
+		current = get_prototype_of(current);
+	}
+}
 export var is_extensible = Object.isExtensible;
 export var has_own_property = Object.prototype.hasOwnProperty;
 

packages/svelte/tests/runtime-runes/samples/bind-and-spread-class-instance/_config.js

@@ -0,0 +1,22 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	html: `<button>0</button><button>0</button><p>accessor: 0 field: 0</p>`,
+
+	test({ assert, target }) {
+		const [btn1, btn2] = target.querySelectorAll('button');
+
+		flushSync(() => btn1?.click());
+		assert.htmlEqual(
+			target.innerHTML,
+			`<button>1</button><button>0</button><p>accessor: 1 field: 0</p>`
+		);
+
+		flushSync(() => btn2?.click());
+		assert.htmlEqual(
+			target.innerHTML,
+			`<button>1</button><button>1</button><p>accessor: 1 field: 1</p>`
+		);
+	}
+});

packages/svelte/tests/runtime-runes/samples/bind-and-spread-class-instance/counter.svelte

@@ -0,0 +1,5 @@
+<script>
+	let { value = $bindable() } = $props();
+</script>
+
+<button onclick={() => value++}>{value}</button>

packages/svelte/tests/runtime-runes/samples/bind-and-spread-class-instance/main.svelte

@@ -0,0 +1,24 @@
+<script>
+	import Counter from './counter.svelte';
+
+	class AccessorProps {
+		#value = $state(0);
+		get value() {
+			return this.#value;
+		}
+		set value(v) {
+			this.#value = v;
+		}
+	}
+
+	class FieldProps {
+		value = $state(0);
+	}
+
+	const accessor = new AccessorProps();
+	const field = new FieldProps();
+</script>
+
+<Counter {...accessor} />
+<Counter {...field} />
+<p>accessor: {accessor.value} field: {field.value}</p>

packages/svelte/tests/runtime-runes/samples/mount-class-instance-props/_config.js

@@ -0,0 +1,25 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	test({ assert, target }) {
+		assert.htmlEqual(
+			target.innerHTML,
+			`<div><button>0</button></div><div><button>0</button></div><p>accessor: 0 field: 0</p>`
+		);
+
+		const [btn1, btn2] = target.querySelectorAll('button');
+
+		flushSync(() => btn1?.click());
+		assert.htmlEqual(
+			target.innerHTML,
+			`<div><button>1</button></div><div><button>0</button></div><p>accessor: 1 field: 0</p>`
+		);
+
+		flushSync(() => btn2?.click());
+		assert.htmlEqual(
+			target.innerHTML,
+			`<div><button>1</button></div><div><button>1</button></div><p>accessor: 1 field: 1</p>`
+		);
+	}
+});

packages/svelte/tests/runtime-runes/samples/mount-class-instance-props/component.svelte

@@ -0,0 +1,5 @@
+<script>
+	let { value = $bindable() } = $props();
+</script>
+
+<button onclick={() => value++}>{value}</button>

packages/svelte/tests/runtime-runes/samples/mount-class-instance-props/main.svelte

@@ -0,0 +1,31 @@
+<script>
+	import { mount, onMount } from 'svelte';
+	import Component from './component.svelte';
+
+	class AccessorProps {
+		#value = $state(0);
+		get value() {
+			return this.#value;
+		}
+		set value(v) {
+			this.#value = v;
+		}
+	}
+
+	class FieldProps {
+		value = $state(0);
+	}
+
+	let div1, div2;
+	const accessor = new AccessorProps();
+	const field = new FieldProps();
+
+	onMount(() => {
+		mount(Component, { target: div1, props: accessor });
+		mount(Component, { target: div2, props: field });
+	});
+</script>
+
+<div bind:this={div1}></div>
+<div bind:this={div2}></div>
+<p>accessor: {accessor.value} field: {field.value}</p>