fix: ensure deep mutation ownership widening

.changeset/mighty-frogs-obey.md

@@ -0,0 +1,5 @@
+---
+"svelte": patch
+---
+
+fix: ensure deep mutation ownership widening

packages/svelte/src/internal/client/dev/ownership.js

@@ -2,6 +2,7 @@
 
 import { STATE_SYMBOL } from '../constants.js';
 import { untrack } from '../runtime.js';
+import { get_descriptors } from '../utils.js';
 
 /** @type {Record<string, Array<{ start: Location, end: Location, component: Function }>>} */
 const boundaries = {};
@@ -91,49 +92,107 @@ export function mark_module_end() {
 	}
 }
 
+let add_owner_visited = new Set();
+
 /**
  *
  * @param {any} object
  * @param {any} owner
  */
 export function add_owner(object, owner) {
-	untrack(() => {
-		add_owner_to_object(object, owner);
-	});
+	// Needed because ownership addition can invoke getters on a proxy,
+	// calling add_owner anew, so just keeping the set as part of
+	// add_owner_to_object would not be enough.
+	const prev = add_owner_visited;
+	try {
+		add_owner_visited = new Set(add_owner_visited);
+		untrack(() => {
+			add_owner_to_object(object, owner, add_owner_visited);
+		});
+	} finally {
+		add_owner_visited = prev;
+	}
 }
 
 /**
  * @param {any} object
  * @param {Function} owner
+ * @param {Set<any>} visited
  */
-function add_owner_to_object(object, owner) {
+function add_owner_to_object(object, owner, visited) {
+	if (visited.has(object)) return;
+	visited.add(object);
+
 	if (object?.[STATE_SYMBOL]?.o && !object[STATE_SYMBOL].o.has(owner)) {
 		object[STATE_SYMBOL].o.add(owner);
-
-		for (const key in object) {
-			add_owner_to_object(object[key], owner);
-		}
 	}
+	// Not inside previous if-block; there could be normal objects in-between
+	traverse_for_owners(object, (nested) => add_owner_to_object(nested, owner, visited));
 }
 
+let strip_owner_visited = new Set();
+
 /**
  * @param {any} object
  */
 export function strip_owner(object) {
-	untrack(() => {
-		strip_owner_from_object(object);
-	});
+	// Needed because ownership stripping can invoke getters on a proxy,
+	// calling strip_owner anew, so just keeping the set as part of
+	// strip_owner_from_object would not be enough.
+	const prev = strip_owner_visited;
+	try {
+		untrack(() => {
+			strip_owner_from_object(object, strip_owner_visited);
+		});
+	} finally {
+		strip_owner_visited = prev;
+	}
 }
 
 /**
  * @param {any} object
+ * @param {Set<any>} visited
  */
-function strip_owner_from_object(object) {
+function strip_owner_from_object(object, visited) {
+	if (visited.has(object)) return;
+	visited.add(object);
+
 	if (object?.[STATE_SYMBOL]?.o) {
 		object[STATE_SYMBOL].o = null;
+	}
+	// Not inside previous if-block; there could be normal objects in-between
+	traverse_for_owners(object, (nested) => strip_owner_from_object(nested, visited));
+}
 
+/**
+ * @param {any} object
+ * @param {(obj: any) => void} cb
+ */
+function traverse_for_owners(object, cb) {
+	if (typeof object === 'object' && object !== null && !(object instanceof EventTarget)) {
 		for (const key in object) {
-			strip_owner(object[key]);
+			cb(object[key]);
+		}
+		// deal with state on classes
+		const proto = Object.getPrototypeOf(object);
+		if (
+			proto !== Object.prototype &&
+			proto !== Array.prototype &&
+			proto !== Map.prototype &&
+			proto !== Set.prototype &&
+			proto !== Date.prototype
+		) {
+			const descriptors = get_descriptors(proto);
+			for (let key in descriptors) {
+				const get = descriptors[key].get;
+				if (get) {
+					try {
+						cb(object[key]);
+					} catch (e) {
+						// continue
+					}
+				}
+			}
 		}
 	}
 }

packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-6/_config.js

@@ -0,0 +1,35 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+/** @type {typeof console.warn} */
+let warn;
+
+/** @type {any[]} */
+let warnings = [];
+
+export default test({
+	compileOptions: {
+		dev: true
+	},
+
+	before_test: () => {
+		warn = console.warn;
+
+		console.warn = (...args) => {
+			warnings.push(...args);
+		};
+	},
+
+	after_test: () => {
+		console.warn = warn;
+		warnings = [];
+	},
+
+	async test({ assert, target }) {
+		const btn = target.querySelector('button');
+
+		await btn?.click();
+		await tick();
+		assert.deepEqual(warnings.length, 0);
+	}
+});

packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-6/main.svelte

@@ -0,0 +1,25 @@
+<script lang="ts">
+	import { setContext } from 'svelte';
+	import Sub from './sub.svelte';
+
+	class Person1 {
+		value = $state({ person: 'John', age: 33 })
+	}
+	const class_nested_state = $state(new Person1());
+
+	class Person2 {
+		person = $state('John');
+		age = $state(33);
+	}
+	const state_nested_class = $state({ value: new Person2() });
+
+	const nested_state = $state({ person: 'John', age: 33 });
+
+	setContext('foo', {
+		nested_state,
+		get class_nested_state() { return class_nested_state },
+		get state_nested_class() { return state_nested_class }
+	})
+</script>
+
+<Sub {class_nested_state} {state_nested_class} {nested_state} />

packages/svelte/tests/runtime-runes/samples/non-local-mutation-inherited-owner-6/sub.svelte

@@ -0,0 +1,11 @@
+<script>
+	import { getContext } from "svelte";
+
+	const foo = getContext('foo')
+</script>
+
+<button onclick={() => {
+	foo.class_nested_state.value.age++;
+	foo.state_nested_class.value.age++;
+	foo.nested_state.age++;
+}}>mutate</button>