fix: remove readonly validation

The readonly dev time validation results in false-negative object equality checks: The original object is proxified and thus comparisons could be between the unproxified and proxified version, which will always return false. Fixes #10372

There's also the problem that an object could be passed into a component but then passed upwards into shared state. At that point, it should no longer be readonly, but it's not possible to statically analyze these points. Fixes #10372

Lastly, the each block logic mutates an internal array and that also throws errors with the readonly logic. Fixes #10037

Author: dummdidumm

.changeset/tough-houses-sparkle.md

@@ -0,0 +1,5 @@
+---
+"svelte": patch
+---
+
+fix: remove readonly validation as it results in false-negative object equality checks

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

@@ -835,8 +835,6 @@ function serialize_inline_component(node, component_name, context) {
 					arg = b.call('$.get', id);
 				}
 
-				if (context.state.options.dev) arg = b.call('$.readonly', arg);
-
 				push_prop(b.get(attribute.name, [b.return(arg)]));
 			} else {
 				push_prop(b.init(attribute.name, value));
@@ -2336,17 +2334,16 @@ export const template_visitors = {
 					/** @type {import('estree').BlockStatement} */ (context.visit(node.fallback))
 				)
 			: b.literal(null);
-		const key_function =
-			node.key && ((each_type & EACH_ITEM_REACTIVE) !== 0 || context.state.options.dev)
-				? b.arrow(
-						[node.context.type === 'Identifier' ? node.context : b.id('$$item'), index],
-						b.block(
-							declarations.concat(
-								b.return(/** @type {import('estree').Expression} */ (context.visit(node.key)))
-							)
+		const key_function = node.key
+			? b.arrow(
+					[node.context.type === 'Identifier' ? node.context : b.id('$$item'), index],
+					b.block(
+						declarations.concat(
+							b.return(/** @type {import('estree').Expression} */ (context.visit(node.key)))
 						)
 					)
-				: b.literal(null);
+				)
+			: b.literal(null);
 
 		if (node.index && each_node_meta.contains_group_binding) {
 			// We needed to create a unique identifier for the index above, but we want to use the

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

@@ -18,12 +18,10 @@ import {
 	get_prototype_of,
 	is_array,
 	is_frozen,
-	object_keys,
 	object_prototype
 } from './utils.js';
 
 export const STATE_SYMBOL = Symbol('$state');
-export const READONLY_SYMBOL = Symbol('readonly');
 
 /**
  * @template T
@@ -93,7 +91,7 @@ function unwrap(value, already_unwrapped = new Map()) {
 			const descriptors = get_descriptors(value);
 			already_unwrapped.set(value, obj);
 			for (const key of keys) {
-				if (key === STATE_SYMBOL || (DEV && key === READONLY_SYMBOL)) continue;
+				if (key === STATE_SYMBOL) continue;
 				if (descriptors[key].get) {
 					define_property(obj, key, descriptors[key]);
 				} else {
@@ -175,9 +173,6 @@ const state_proxy_handler = {
 	},
 
 	get(target, prop, receiver) {
-		if (DEV && prop === READONLY_SYMBOL) {
-			return Reflect.get(target, READONLY_SYMBOL);
-		}
 		if (prop === STATE_SYMBOL) {
 			return Reflect.get(target, STATE_SYMBOL);
 		}
@@ -224,9 +219,6 @@ const state_proxy_handler = {
 	},
 
 	has(target, prop) {
-		if (DEV && prop === READONLY_SYMBOL) {
-			return Reflect.has(target, READONLY_SYMBOL);
-		}
 		if (prop === STATE_SYMBOL) {
 			return true;
 		}
@@ -250,10 +242,6 @@ const state_proxy_handler = {
 	},
 
 	set(target, prop, value) {
-		if (DEV && prop === READONLY_SYMBOL) {
-			target[READONLY_SYMBOL] = value;
-			return true;
-		}
 		const metadata = target[STATE_SYMBOL];
 		const s = metadata.s.get(prop);
 		if (s !== undefined) set(s, proxy(value, metadata.i));
@@ -304,69 +292,3 @@ if (DEV) {
 		throw new Error('Cannot set prototype of $state object');
 	};
 }
-
-/**
- * Expects a value that was wrapped with `proxy` and makes it readonly.
- *
- * @template {Record<string | symbol, any>} T
- * @template {import('./types.js').ProxyReadonlyObject<T> | T} U
- * @param {U} value
- * @returns {Proxy<U> | U}
- */
-export function readonly(value) {
-	const proxy = value && value[READONLY_SYMBOL];
-	if (proxy) {
-		const metadata = value[STATE_SYMBOL];
-		// Check that the incoming value is the same proxy that this readonly symbol was created for:
-		// If someone copies over the readonly symbol to a new object (using Reflect.ownKeys) the referenced
-		// proxy could be stale and we should not return it.
-		if (metadata.p === value) return proxy;
-	}
-
-	if (
-		typeof value === 'object' &&
-		value != null &&
-		!is_frozen(value) &&
-		STATE_SYMBOL in value && // TODO handle Map and Set as well
-		!(READONLY_SYMBOL in value)
-	) {
-		const proxy = new Proxy(
-			value,
-			/** @type {ProxyHandler<import('./types.js').ProxyReadonlyObject<U>>} */ (
-				readonly_proxy_handler
-			)
-		);
-		define_property(value, READONLY_SYMBOL, { value: proxy, writable: false });
-		return proxy;
-	}
-
-	return value;
-}
-
-/**
- * @param {any}	_
- * @param {string} prop
- * @returns {never}
- */
-const readonly_error = (_, prop) => {
-	throw new Error(
-		`Non-bound props cannot be mutated — to make the \`${prop}\` settable, ensure the object it is used within is bound as a prop \`bind:<prop>={...}\`. Fallback values can never be mutated.`
-	);
-};
-
-/** @type {ProxyHandler<import('./types.js').ProxyReadonlyObject>} */
-const readonly_proxy_handler = {
-	defineProperty: readonly_error,
-	deleteProperty: readonly_error,
-	set: readonly_error,
-
-	get(target, prop, receiver) {
-		const value = Reflect.get(target, prop, receiver);
-
-		if (!(prop in target)) {
-			return readonly(value);
-		}
-
-		return value;
-	}
-};

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

@@ -17,7 +17,7 @@ import {
 	PROPS_IS_RUNES,
 	PROPS_IS_UPDATED
 } from '../../constants.js';
-import { READONLY_SYMBOL, STATE_SYMBOL, proxy, readonly, unstate } from './proxy.js';
+import { STATE_SYMBOL, unstate } from './proxy.js';
 import { EACH_BLOCK, IF_BLOCK } from './block.js';
 
 export const SOURCE = 1;
@@ -1619,10 +1619,6 @@ export function prop(props, key, flags, initial) {
 		// @ts-expect-error would need a cumbersome method overload to type this
 		if ((flags & PROPS_IS_LAZY_INITIAL) !== 0) initial = initial();
 
-		if (DEV && runes) {
-			initial = readonly(proxy(/** @type {any} */ (initial)));
-		}
-
 		prop_value = /** @type {V} */ (initial);
 
 		if (setter) setter(prop_value);
@@ -2126,10 +2122,6 @@ export function freeze(value) {
 		if (STATE_SYMBOL in value) {
 			return object_freeze(unstate(value));
 		}
-		// If the value is already read-only then just use that
-		if (DEV && READONLY_SYMBOL in value) {
-			return value;
-		}
 		// Otherwise freeze the object
 		object_freeze(value);
 	}

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

@@ -10,7 +10,7 @@ import {
 	DYNAMIC_ELEMENT_BLOCK,
 	SNIPPET_BLOCK
 } from './block.js';
-import type { READONLY_SYMBOL, STATE_SYMBOL } from './proxy.js';
+import type { STATE_SYMBOL } from './proxy.js';
 import { DERIVED, EFFECT, RENDER_EFFECT, SOURCE, PRE_EFFECT, LAZY_PROPERTY } from './runtime.js';
 
 // Put all internal types in this file. Once we convert to JSDoc, we can make this a d.ts file
@@ -409,15 +409,11 @@ export interface ProxyMetadata<T = Record<string | symbol, any>> {
 	/** Immutable: Whether to use a source or mutable source under the hood */
 	i: boolean;
 	/** The associated proxy */
-	p: ProxyStateObject<T> | ProxyReadonlyObject<T>;
+	p: ProxyStateObject<T>;
 	/** The original target this proxy was created for */
 	t: T;
 }
 
 export type ProxyStateObject<T = Record<string | symbol, any>> = T & {
 	[STATE_SYMBOL]: ProxyMetadata;
 };
-
-export type ProxyReadonlyObject<T = Record<string | symbol, any>> = ProxyStateObject<T> & {
-	[READONLY_SYMBOL]: ProxyMetadata;
-};

packages/svelte/src/internal/index.js

@@ -44,7 +44,7 @@ export * from './client/each.js';
 export * from './client/render.js';
 export * from './client/validate.js';
 export { raf } from './client/timing.js';
-export { proxy, readonly, unstate } from './client/proxy.js';
+export { proxy, unstate } from './client/proxy.js';
 export { create_custom_element } from './client/custom-element.js';
 export {
 	child,

packages/svelte/tests/runtime-runes/samples/props-array-each/_config.js

@@ -0,0 +1,15 @@
+import { test } from '../../test';
+
+export default test({
+	html: `<button>add</button> <p>1</p><p>1</p><p>1</p>`,
+
+	async test({ assert, target }) {
+		const btn = target.querySelector('button');
+
+		await btn?.click();
+		assert.htmlEqual(
+			target.innerHTML,
+			`<button>add</button> <p>1</p><p>2</p><p>1</p><p>2</p><p>1</p><p>2</p>`
+		);
+	}
+});

packages/svelte/tests/runtime-runes/samples/props-array-each/child.svelte

@@ -0,0 +1,15 @@
+<script>
+	let { array } = $props();
+</script>
+
+{#each array as number}
+	<p>{number.v}</p>
+{/each}
+
+{#each array as number (number)}
+	<p>{number.v}</p>
+{/each}
+
+{#each array as number (number.v)}
+	<p>{number.v}</p>
+{/each}

packages/svelte/tests/runtime-runes/samples/props-array-each/main.svelte

@@ -0,0 +1,12 @@
+<script>
+	import Child from './child.svelte';
+
+	let array = $state([{v: 1}]);
+
+	const addNew = () => {
+		array.push({v: 2})
+	}
+</script>
+
+<button onclick={addNew}>add</button>
+<Child {array} />

packages/svelte/tests/runtime-runes/samples/props-equality/_config.js

@@ -0,0 +1,45 @@
+import { test } from '../../test';
+
+export default test({
+	html: `
+		<button>a true</button><button>b true</button>
+		<button>a true</button><button>b true</button>
+		<button>a true</button><button>b true</button>
+	`,
+
+	async test({ assert, target }) {
+		let [btn1, _btn2, btn3, _btn4, btn5] = target.querySelectorAll('button');
+
+		await btn1.click();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<button>a+ true</button><button>b true</button>
+			<button>a+ true</button><button>b true</button>
+			<button>a+ true</button><button>b true</button>
+		`
+		);
+
+		[btn1, _btn2, btn3, _btn4, btn5] = target.querySelectorAll('button');
+		await btn3.click();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<button>a++ true</button><button>b true</button>
+			<button>a++ true</button><button>b true</button>
+			<button>a++ true</button><button>b true</button>
+		`
+		);
+
+		[btn1, _btn2, btn3, _btn4, btn5] = target.querySelectorAll('button');
+		await btn5.click();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<button>a+++ true</button><button>b true</button>
+			<button>a+++ true</button><button>b true</button>
+			<button>a+++ true</button><button>b true</button>
+		`
+		);
+	}
+});

packages/svelte/tests/runtime-runes/samples/props-equality/item.svelte

@@ -0,0 +1,7 @@
+<script>
+	let {item, items, onclick} = $props()
+</script>
+
+<button {onclick}>
+	{item.name} {items.includes(item)}
+</button>

packages/svelte/tests/runtime-runes/samples/props-equality/main.svelte

@@ -0,0 +1,19 @@
+<script>
+	import Item from './item.svelte'
+
+	let items = $state([{name: 'a'}, {name: 'b'}]);
+</script>
+
+<!-- test that each block doesn't mess with item identity -->
+
+{#each items as item (item)}
+	<Item {item} {items} onclick={() => item.name = item.name + '+'} />
+{/each}
+
+{#each items as item (item.name)}
+	<Item {item} {items} onclick={() => {console.log('hello'); item.name = item.name + '+'}} />
+{/each}
+
+{#each items as item}
+	<Item {item} {items} onclick={() => item.name = item.name + '+'} />
+{/each}