fix: ensure deep mutation ownership widening (#11094)

Previously, ownership widening/removal was only done if the immediate object that was encountered was state. This isn't always the case. It also didn't take into account classes with state on it (which turn into getters). This change takes both these cases into account and now always traverses the given object deeply.
fixes #11060

Author: dummdidumm

.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>