fix: better readonly checks for proxies (#9808)

- Expect the thing that's checked to be wrapped with the proxy already, so that we can just check for the state symbol
- Make error message more descriptive

Author: dummdidumm

.changeset/five-tigers-search.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: better readonly checks for proxies

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

@@ -16,12 +16,12 @@ import {
 	is_array,
 	object_keys
 } from '../utils.js';
-import { READONLY_SYMBOL } from './readonly.js';
 
 /** @typedef {{ s: Map<string | symbol, import('../types.js').SourceSignal<any>>; v: import('../types.js').SourceSignal<number>; a: boolean, i: boolean }} Metadata */
 /** @typedef {Record<string | symbol, any> & { [STATE_SYMBOL]: Metadata }} StateObject */
 
 export const STATE_SYMBOL = Symbol('$state');
+export const READONLY_SYMBOL = Symbol('readonly');
 
 const object_prototype = Object.prototype;
 const array_prototype = Array.prototype;

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

@@ -1,18 +1,16 @@
-import { define_property, get_descriptor } from '../utils.js';
+import { define_property } from '../utils.js';
+import { READONLY_SYMBOL, STATE_SYMBOL } from './proxy.js';
 
 /**
  * @template {Record<string | symbol, any>} T
  * @typedef {T & { [READONLY_SYMBOL]: Proxy<T> }} StateObject
  */
 
-export const READONLY_SYMBOL = Symbol('readonly');
-
-const object_prototype = Object.prototype;
-const array_prototype = Array.prototype;
-const get_prototype_of = Object.getPrototypeOf;
 const is_frozen = Object.isFrozen;
 
 /**
+ * Expects a value that was wrapped with `proxy` and makes it readonly.
+ *
  * @template {Record<string | symbol, any>} T
  * @template {StateObject<T>} U
  * @param {U} value
@@ -26,25 +24,26 @@ export function readonly(value) {
 		typeof value === 'object' &&
 		value != null &&
 		!is_frozen(value) &&
+		STATE_SYMBOL in value && // TODO handle Map and Set as well
 		!(READONLY_SYMBOL in value)
 	) {
-		const prototype = get_prototype_of(value);
-
-		// TODO handle Map and Set as well
-		if (prototype === object_prototype || prototype === array_prototype) {
-			const proxy = new Proxy(value, handler);
-			define_property(value, READONLY_SYMBOL, { value: proxy, writable: false });
-
-			return proxy;
-		}
+		const proxy = new Proxy(value, handler);
+		define_property(value, READONLY_SYMBOL, { value: proxy, writable: false });
+		return proxy;
 	}
 
 	return value;
 }
 
-/** @returns {never} */
-const readonly_error = () => {
-	throw new Error(`Props cannot be mutated, unless used with \`bind:\``);
+/**
+ * @param {any}	_
+ * @param {string} prop
+ * @returns {never}
+ */
+const readonly_error = (_, prop) => {
+	throw new Error(
+		`Props cannot be mutated, unless used with \`bind:\`. Use \`bind:prop-in-question={..}\` to make \`${prop}\` settable. Fallback values can never be mutated.`
+	);
 };
 
 /** @type {ProxyHandler<StateObject<any>>} */

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

@@ -4,7 +4,7 @@ import { EMPTY_FUNC, run_all } from '../common.js';
 import { get_descriptor, get_descriptors, is_array } from './utils.js';
 import { PROPS_CALL_DEFAULT_VALUE, PROPS_IS_IMMUTABLE, PROPS_IS_RUNES } from '../../constants.js';
 import { readonly } from './proxy/readonly.js';
-import { observe } from './proxy/proxy.js';
+import { observe, proxy } from './proxy/proxy.js';
 
 export const SOURCE = 1;
 export const DERIVED = 1 << 1;
@@ -1426,7 +1426,7 @@ export function prop_source(props, key, flags, default_value) {
 			call_default_value ? default_value() : default_value;
 
 		if (DEV && runes) {
-			value = readonly(/** @type {any} */ (value));
+			value = readonly(proxy(/** @type {any} */ (value)));
 		}
 	}
 

packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/Counter.svelte

@@ -0,0 +1,8 @@
+<script>
+	/** @type {{ object: { count: number }}} */
+	let { object } = $props();
+</script>
+
+<button onclick={() => object = { count: object.count + 1 } }>
+	clicks: {object.count}
+</button>

packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/_config.js

@@ -0,0 +1,22 @@
+import { test } from '../../test';
+
+// Tests that readonly bails on setters/classes
+export default test({
+	html: `<button>clicks: 0</button><button>clicks: 0</button>`,
+
+	compileOptions: {
+		dev: true
+	},
+
+	async test({ assert, target }) {
+		const [btn1, btn2] = target.querySelectorAll('button');
+
+		await btn1.click();
+		await btn2.click();
+		assert.htmlEqual(target.innerHTML, `<button>clicks: 1</button><button>clicks: 1</button>`);
+
+		await btn1.click();
+		await btn2.click();
+		assert.htmlEqual(target.innerHTML, `<button>clicks: 2</button><button>clicks: 2</button>`);
+	}
+});

packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly-bail/main.svelte

@@ -0,0 +1,25 @@
+<script>
+	import Counter from './Counter.svelte';
+
+	function createCounter() {
+		let count = $state(0)
+		return {
+			get count() {
+				return count;
+			},
+			set count(upd) {
+				count = upd
+			}
+		}
+	}
+
+	class CounterClass {
+		count = $state(0);
+	}
+
+	const counterSetter = createCounter();
+	const counterClass = new CounterClass();
+</script>
+
+<Counter object={counterSetter} />
+<Counter object={counterClass} />

packages/svelte/tests/runtime-runes/samples/proxy-prop-default-readonly/_config.js

@@ -14,5 +14,6 @@ export default test({
 		assert.htmlEqual(target.innerHTML, `<button>clicks: 0</button>`);
 	},
 
-	runtime_error: 'Props cannot be mutated, unless used with `bind:`'
+	runtime_error:
+		'Props cannot be mutated, unless used with `bind:`. Use `bind:prop-in-question={..}` to make `count` settable. Fallback values can never be mutated.'
 });

packages/svelte/tests/runtime-runes/samples/proxy-prop-readonly/_config.js

@@ -14,5 +14,6 @@ export default test({
 		assert.htmlEqual(target.innerHTML, `<button>clicks: 0</button>`);
 	},
 
-	runtime_error: 'Props cannot be mutated, unless used with `bind:`'
+	runtime_error:
+		'Props cannot be mutated, unless used with `bind:`. Use `bind:prop-in-question={..}` to make `count` settable. Fallback values can never be mutated.'
 });