feat: use state proxy ancestry for ownership validation

.changeset/late-grapes-judge.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+feat: use state proxy ancestry for ownership validation

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

@@ -168,6 +168,27 @@ export const javascript_visitors_runes = {
 			body.push(/** @type {import('estree').MethodDefinition} **/ (visit(definition, child_state)));
 		}
 
+		if (state.options.dev && public_state.size > 0) {
+			// add an `[$.ADD_OWNER]` method so that a class with state fields can widen ownership
+			body.push(
+				b.method(
+					'method',
+					b.id('$.ADD_OWNER'),
+					[b.id('owner')],
+					Array.from(public_state.keys()).map((name) =>
+						b.stmt(
+							b.call(
+								'$.add_owner',
+								b.call('$.get', b.member(b.this, b.private_id(name))),
+								b.id('owner')
+							)
+						)
+					),
+					true
+				)
+			);
+		}
+
 		return { ...node, body };
 	},
 	VariableDeclaration(node, { state, visit }) {

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

@@ -1,7 +1,8 @@
 /** @typedef {{ file: string, line: number, column: number }} Location */
 
 import { STATE_SYMBOL } from '../constants.js';
-import { untrack } from '../runtime.js';
+import { render_effect } from '../reactivity/effects.js';
+import { current_component_context, untrack } from '../runtime.js';
 
 /** @type {Record<string, Array<{ start: Location, end: Location, component: Function }>>} */
 const boundaries = {};
@@ -63,6 +64,8 @@ function get_component() {
 	return null;
 }
 
+export const ADD_OWNER = Symbol('ADD_OWNER');
+
 /**
  * Together with `mark_module_end`, this function establishes the boundaries of a `.svelte` file,
  * such that subsequent calls to `get_component` can tell us which component is responsible
@@ -98,60 +101,87 @@ export function mark_module_end(component) {
 }
 
 /**
- *
  * @param {any} object
  * @param {any} owner
+ * @param {boolean} [global]
  */
-export function add_owner(object, owner) {
-	untrack(() => {
-		add_owner_to_object(object, owner);
-	});
+export function add_owner(object, owner, global = false) {
+	if (object && !global) {
+		// @ts-expect-error
+		const component = current_component_context.function;
+		const metadata = object[STATE_SYMBOL];
+		if (metadata && !has_owner(metadata, component)) {
+			let original = get_owner(metadata);
+
+			if (owner.filename !== component.filename) {
+				let message = `${component.filename} passed a value to ${owner.filename} with \`bind:\`, but the value is owned by ${original.filename}. Consider creating a binding between ${original.filename} and ${component.filename}`;
+
+				// eslint-disable-next-line no-console
+				console.warn(message);
+			}
+		}
+	}
+
+	add_owner_to_object(object, owner);
 }
 
 /**
  * @param {any} object
  * @param {Function} owner
  */
 function add_owner_to_object(object, owner) {
-	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);
+	const metadata = /** @type {import('#client').ProxyMetadata} */ (object?.[STATE_SYMBOL]);
+
+	if (metadata) {
+		// this is a state proxy, add owner directly
+		(metadata.owners ??= new Set()).add(owner);
+	} else if (object && typeof object === 'object') {
+		if (object[ADD_OWNER]) {
+			// this is a class with state fields
+			render_effect(() => {
+				object[ADD_OWNER](owner);
+			});
+		} else {
+			// recurse until we find a state proxy
+			for (const key in object) {
+				add_owner_to_object(object[key], owner);
+			}
 		}
 	}
 }
 
 /**
- * @param {any} object
+ * @param {import('#client').ProxyMetadata} metadata
+ * @param {Function} component
+ * @returns {boolean}
  */
-export function strip_owner(object) {
-	untrack(() => {
-		strip_owner_from_object(object);
-	});
+function has_owner(metadata, component) {
+	if (metadata.owners === null) {
+		return metadata.parent === null ? true : has_owner(metadata.parent, component);
+	}
+
+	return metadata.owners.has(component);
 }
 
 /**
- * @param {any} object
+ * @param {import('#client').ProxyMetadata} metadata
+ * @returns {any}
  */
-function strip_owner_from_object(object) {
-	if (object?.[STATE_SYMBOL]?.o) {
-		object[STATE_SYMBOL].o = null;
-
-		for (const key in object) {
-			strip_owner(object[key]);
-		}
-	}
+function get_owner(metadata) {
+	return (
+		metadata?.owners?.values().next().value ??
+		get_owner(/** @type {import('#client').ProxyMetadata} */ (metadata.parent))
+	);
 }
 
 /**
- * @param {Set<Function>} owners
+ * @param {import('#client').ProxyMetadata} metadata
  */
-export function check_ownership(owners) {
+export function check_ownership(metadata) {
 	const component = get_component();
 
-	if (component && !owners.has(component)) {
-		let original = [...owners][0];
+	if (component && !has_owner(metadata, component)) {
+		let original = get_owner(metadata);
 
 		let message =
 			// @ts-expect-error

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

@@ -1,5 +1,5 @@
 export { hmr } from './dev/hmr.js';
-export { add_owner, mark_module_start, mark_module_end } from './dev/ownership.js';
+export { ADD_OWNER, add_owner, mark_module_start, mark_module_end } from './dev/ownership.js';
 export { await_block as await } from './dom/blocks/await.js';
 export { if_block as if } from './dom/blocks/if.js';
 export { key_block as key } from './dom/blocks/key.js';

packages/svelte/src/internal/client/proxy.js

@@ -16,7 +16,7 @@ import {
 	is_frozen,
 	object_prototype
 } from './utils.js';
-import { add_owner, check_ownership, strip_owner } from './dev/ownership.js';
+import { check_ownership } from './dev/ownership.js';
 import { mutable_source, source, set } from './reactivity/sources.js';
 import { STATE_SYMBOL } from './constants.js';
 import { UNINITIALIZED } from '../../constants.js';
@@ -25,26 +25,20 @@ import { UNINITIALIZED } from '../../constants.js';
  * @template T
  * @param {T} value
  * @param {boolean} [immutable]
- * @param {Set<Function> | null} [owners]
+ * @param {import('#client').ProxyMetadata | null} [parent]
  * @returns {import('#client').ProxyStateObject<T> | T}
  */
-export function proxy(value, immutable = true, owners) {
+export function proxy(value, immutable = true, parent = null) {
 	if (typeof value === 'object' && value != null && !is_frozen(value)) {
 		// If we have an existing proxy, return it...
 		if (STATE_SYMBOL in value) {
 			const metadata = /** @type {import('#client').ProxyMetadata<T>} */ (value[STATE_SYMBOL]);
+
 			// ...unless the proxy belonged to a different object, because
 			// someone copied the state symbol using `Reflect.ownKeys(...)`
 			if (metadata.t === value || metadata.p === value) {
 				if (DEV) {
-					// update ownership
-					if (owners) {
-						for (const owner of owners) {
-							add_owner(value, owner);
-						}
-					} else {
-						strip_owner(value);
-					}
+					metadata.parent = parent;
 				}
 
 				return metadata.p;
@@ -70,16 +64,15 @@ export function proxy(value, immutable = true, owners) {
 			});
 
 			if (DEV) {
-				// set ownership — either of the parent proxy's owners (if provided) or,
-				// when calling `$.proxy(...)`, to the current component if such there be
 				// @ts-expect-error
-				value[STATE_SYMBOL].o =
-					owners === undefined
-						? current_component_context
-							? // @ts-expect-error
-								new Set([current_component_context.function])
-							: null
-						: owners && new Set(owners);
+				value[STATE_SYMBOL].parent = parent;
+
+				// @ts-expect-error
+				value[STATE_SYMBOL].owners =
+					parent === null && current_component_context !== null
+						? // @ts-expect-error
+							new Set([current_component_context.function])
+						: null;
 			}
 
 			return proxy;
@@ -162,7 +155,7 @@ const state_proxy_handler = {
 			const metadata = target[STATE_SYMBOL];
 
 			const s = metadata.s.get(prop);
-			if (s !== undefined) set(s, proxy(descriptor.value, metadata.i, metadata.o));
+			if (s !== undefined) set(s, proxy(descriptor.value, metadata.i, metadata));
 		}
 
 		return Reflect.defineProperty(target, prop, descriptor);
@@ -208,7 +201,7 @@ const state_proxy_handler = {
 
 		// create a source, but only if it's an own property and not a prototype property
 		if (s === undefined && (!(prop in target) || get_descriptor(target, prop)?.writable)) {
-			s = (metadata.i ? source : mutable_source)(proxy(target[prop], metadata.i, metadata.o));
+			s = (metadata.i ? source : mutable_source)(proxy(target[prop], metadata.i, metadata));
 			metadata.s.set(prop, s);
 		}
 
@@ -255,7 +248,7 @@ const state_proxy_handler = {
 		) {
 			if (s === undefined) {
 				s = (metadata.i ? source : mutable_source)(
-					has ? proxy(target[prop], metadata.i, metadata.o) : UNINITIALIZED
+					has ? proxy(target[prop], metadata.i, metadata) : UNINITIALIZED
 				);
 				metadata.s.set(prop, s);
 			}
@@ -281,23 +274,13 @@ const state_proxy_handler = {
 			s = metadata.s.get(prop);
 		}
 		if (s !== undefined) {
-			set(s, proxy(value, metadata.i, metadata.o));
+			set(s, proxy(value, metadata.i, metadata));
 		}
 		const is_array = metadata.a;
 		const not_has = !(prop in target);
 
 		if (DEV) {
-			// First check ownership of the object that is assigned to.
-			// Then, if the new object has owners, widen them with the ones from the current object.
-			// If it doesn't have owners that means it's ownerless, and so the assigned object should be, too.
-			if (metadata.o) {
-				check_ownership(metadata.o);
-				for (const owner in metadata.o) {
-					add_owner(value, owner);
-				}
-			} else {
-				strip_owner(value);
-			}
+			check_ownership(metadata);
 		}
 
 		// variable.length = value -> clear all signals with index >= value

packages/svelte/src/internal/client/runtime.js

@@ -8,7 +8,7 @@ import {
 	object_prototype
 } from './utils.js';
 import { unstate } from './proxy.js';
-import { destroy_effect, effect, user_pre_effect } from './reactivity/effects.js';
+import { destroy_effect, effect, render_effect, user_pre_effect } from './reactivity/effects.js';
 import {
 	EFFECT,
 	RENDER_EFFECT,
@@ -894,7 +894,7 @@ export function getContext(key) {
 		// @ts-expect-error
 		const fn = current_component_context?.function;
 		if (fn) {
-			add_owner(result, fn);
+			add_owner(result, fn, true);
 		}
 	}
 
@@ -950,7 +950,7 @@ export function getAllContexts() {
 		const fn = current_component_context?.function;
 		if (fn) {
 			for (const value of context_map.values()) {
-				add_owner(value, fn);
+				add_owner(value, fn, true);
 			}
 		}
 	}

packages/svelte/src/internal/client/types.d.ts

@@ -166,7 +166,9 @@ export interface ProxyMetadata<T = Record<string | symbol, any>> {
 	/** The original target this proxy was created for */
 	t: T;
 	/** Dev-only — the components that 'own' this state, if any */
-	o: null | Set<Function>;
+	owners: null | Set<Function>;
+	/** Dev-only — the parent metadata object */
+	parent: null | ProxyMetadata;
 }
 
 export type ProxyStateObject<T = Record<string | symbol, any>> = T & {

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

@@ -0,0 +1,9 @@
+<script>
+	import { getContext } from 'svelte';
+
+	const foo = getContext('foo')
+</script>
+
+<button onclick={() => {
+	foo.person.name.last = 'T';
+}}>mutate</button>

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,17 @@
+<script>
+	import { setContext } from 'svelte';
+	import Child from './Child.svelte';
+
+	class Person {
+		name = $state({
+			first: 'Mr',
+			last: 'Bean'
+		});
+	}
+
+	setContext('foo', {
+		person: new Person()
+	});
+</script>
+
+<Child />