From 2d2bae52dc468e65fa03b40b11ea41b9d0547f92 Mon Sep 17 00:00:00 2001 From: Ashish Choubey Date: Sun, 26 Apr 2026 06:51:23 +0530 Subject: [PATCH] fix: walk prototype chain when resolving prop descriptors `Object.getOwnPropertyDescriptor` skips inherited accessors, so spread/`mount` props that come from class instances (including class fields with `$state`) silently lost their setter and bindings became no-ops. Walks the prototype chain to find accessor descriptors, routes spread-proxy writes through `props[key] = v` so `this` resolves correctly, and applies the same fix to SSR `spread_props` / `bind_props`. Closes #18140 Co-Authored-By: Claude Opus 4.7 (1M context) --- .changeset/class-instance-props-binding.md | 5 ++ .../src/internal/client/reactivity/props.js | 16 +++--- packages/svelte/src/internal/server/index.js | 52 +++++++++++++++---- packages/svelte/src/internal/shared/utils.js | 19 +++++++ .../bind-and-spread-class-instance/_config.js | 22 ++++++++ .../counter.svelte | 5 ++ .../main.svelte | 24 +++++++++ .../mount-class-instance-props/_config.js | 25 +++++++++ .../component.svelte | 5 ++ .../mount-class-instance-props/main.svelte | 31 +++++++++++ 10 files changed, 188 insertions(+), 16 deletions(-) create mode 100644 .changeset/class-instance-props-binding.md create mode 100644 packages/svelte/tests/runtime-runes/samples/bind-and-spread-class-instance/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/bind-and-spread-class-instance/counter.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/bind-and-spread-class-instance/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/mount-class-instance-props/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/mount-class-instance-props/component.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/mount-class-instance-props/main.svelte diff --git a/.changeset/class-instance-props-binding.md b/.changeset/class-instance-props-binding.md new file mode 100644 index 0000000000..f112f4ba78 --- /dev/null +++ b/.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`) diff --git a/packages/svelte/src/internal/client/reactivity/props.js b/packages/svelte/src/internal/client/reactivity/props.js index e208d3b6f6..b00a36dff3 100644 --- a/packages/svelte/src/internal/client/reactivity/props.js +++ b/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 { @@ -201,9 +201,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; } } @@ -215,7 +215,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, @@ -304,9 +304,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} */ diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index 34d0133a31..6de2dbec9a 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/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, @@ -181,23 +181,57 @@ export function attributes(attrs, css_hash, classes, styles, flags = 0) { export function spread_props(props) { /** @type {Record} */ 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} */ + 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} target + * @param {Record} 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} @@ -394,7 +428,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; } diff --git a/packages/svelte/src/internal/shared/utils.js b/packages/svelte/src/internal/shared/utils.js index 393b51383e..f812a599e6 100644 --- a/packages/svelte/src/internal/shared/utils.js +++ b/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; diff --git a/packages/svelte/tests/runtime-runes/samples/bind-and-spread-class-instance/_config.js b/packages/svelte/tests/runtime-runes/samples/bind-and-spread-class-instance/_config.js new file mode 100644 index 0000000000..f9065e7568 --- /dev/null +++ b/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: `

accessor: 0 field: 0

`, + + test({ assert, target }) { + const [btn1, btn2] = target.querySelectorAll('button'); + + flushSync(() => btn1?.click()); + assert.htmlEqual( + target.innerHTML, + `

accessor: 1 field: 0

` + ); + + flushSync(() => btn2?.click()); + assert.htmlEqual( + target.innerHTML, + `

accessor: 1 field: 1

` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/bind-and-spread-class-instance/counter.svelte b/packages/svelte/tests/runtime-runes/samples/bind-and-spread-class-instance/counter.svelte new file mode 100644 index 0000000000..fdf8692e29 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bind-and-spread-class-instance/counter.svelte @@ -0,0 +1,5 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/bind-and-spread-class-instance/main.svelte b/packages/svelte/tests/runtime-runes/samples/bind-and-spread-class-instance/main.svelte new file mode 100644 index 0000000000..9a637cc38c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/bind-and-spread-class-instance/main.svelte @@ -0,0 +1,24 @@ + + + + +

accessor: {accessor.value} field: {field.value}

diff --git a/packages/svelte/tests/runtime-runes/samples/mount-class-instance-props/_config.js b/packages/svelte/tests/runtime-runes/samples/mount-class-instance-props/_config.js new file mode 100644 index 0000000000..1868b08b9d --- /dev/null +++ b/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, + `

accessor: 0 field: 0

` + ); + + const [btn1, btn2] = target.querySelectorAll('button'); + + flushSync(() => btn1?.click()); + assert.htmlEqual( + target.innerHTML, + `

accessor: 1 field: 0

` + ); + + flushSync(() => btn2?.click()); + assert.htmlEqual( + target.innerHTML, + `

accessor: 1 field: 1

` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/mount-class-instance-props/component.svelte b/packages/svelte/tests/runtime-runes/samples/mount-class-instance-props/component.svelte new file mode 100644 index 0000000000..fdf8692e29 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/mount-class-instance-props/component.svelte @@ -0,0 +1,5 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/mount-class-instance-props/main.svelte b/packages/svelte/tests/runtime-runes/samples/mount-class-instance-props/main.svelte new file mode 100644 index 0000000000..b9fb09f872 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/mount-class-instance-props/main.svelte @@ -0,0 +1,31 @@ + + +
+
+

accessor: {accessor.value} field: {field.value}