fix: untrack reads of mutated object

prevents infinite loops when mutation happens inside an effect
came up in #9639

.changeset/beige-donkeys-remain.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: untrack reads of mutated object

packages/svelte/src/compiler/phases/2-analyze/validation.js

@@ -1,5 +1,5 @@
 import { error } from '../../errors.js';
-import { extract_identifiers, is_text_attribute } from '../../utils/ast.js';
+import { extract_identifiers, is_text_attribute, object } from '../../utils/ast.js';
 import { warn } from '../../warnings.js';
 import fuzzymatch from '../1-parse/utils/fuzzymatch.js';
 import { binding_properties } from '../bindings.js';
@@ -248,12 +248,8 @@ export const validation = {
 	BindDirective(node, context) {
 		validate_no_const_assignment(node, node.expression, context.state.scope, true);
 
-		let left = node.expression;
+		const left = object(node.expression);
-		while (left.type === 'MemberExpression') {
+		if (left === null) {
-			left = /** @type {import('estree').MemberExpression} */ (left.object);
-		}
-
-		if (left.type !== 'Identifier') {
 			error(node, 'invalid-binding-expression');
 		}
 

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

@@ -205,27 +205,31 @@ export function serialize_set_binding(node, context, fallback) {
 				return b.call('$.set', b.id(left_name), value);
 			}
 		} else {
+			const left = /** @type {import('estree').MemberExpression} */ (visit(node.left));
+			// When reading the object that's mutated we need to untrack that read in order
+			// to avoid infinite loops. $.mutate(_store) does that by accepting a callback
+			// function it hands the value to. For this, we need to adjust the code to
+			// reference that passed value instead of reading the object again.
+			let id = left;
+			while (id.type === 'MemberExpression') {
+				if (id.object.type !== 'MemberExpression') {
+					id.object = b.id('$$to_mutate');
+					break;
+				} else {
+					id = id.object;
+				}
+			}
+			const update = b.arrow([b.id('$$to_mutate')], b.assignment(node.operator, left, value));
+
 			if (is_store) {
 				return b.call(
 					'$.mutate_store',
 					serialize_get_binding(b.id(left_name), state),
-					b.assignment(
+					b.id('$' + left_name),
-						node.operator,
+					update
-						/** @type {import('estree').Pattern} */ (visit(node.left)),
-						value
-					),
-					b.call('$' + left_name)
 				);
 			} else {
-				return b.call(
+				return b.call('$.mutate', b.id(left_name), update);
-					'$.mutate',
-					b.id(left_name),
-					b.assignment(
-						node.operator,
-						/** @type {import('estree').Pattern} */ (visit(node.left)),
-						value
-					)
-				);
 			}
 		}
 	};

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

@@ -206,13 +206,7 @@ function collect_transitive_dependencies(binding, seen = new Set()) {
  * @param {import('../types.js').ComponentContext} context
  */
 function setup_select_synchronization(value_binding, context) {
-	let bound = value_binding.expression;
+	const bound = /** @type {import('estree').Identifier} */ (object(value_binding.expression));
-	while (bound.type === 'MemberExpression') {
-		bound = /** @type {import('estree').Identifier | import('estree').MemberExpression} */ (
-			bound.object
-		);
-	}
-
 	/** @type {string[]} */
 	const names = [];
 

packages/svelte/src/compiler/phases/3-transform/server/transform-server.js

@@ -1,6 +1,11 @@
 import { walk } from 'zimmerframe';
 import { set_scope, get_rune } from '../../scope.js';
-import { extract_identifiers, extract_paths, is_event_attribute } from '../../../utils/ast.js';
+import {
+	extract_identifiers,
+	extract_paths,
+	is_event_attribute,
+	object
+} from '../../../utils/ast.js';
 import * as b from '../../../utils/builders.js';
 import is_reference from 'is-reference';
 import {
@@ -417,14 +422,8 @@ function serialize_set_binding(node, context, fallback) {
 		error(node, 'INTERNAL', `Unexpected assignment type ${node.left.type}`);
 	}
 
-	let left = node.left;
+	const left = object(node.left);
-
+	if (left === null) {
-	while (left.type === 'MemberExpression') {
-		// @ts-expect-error
-		left = left.object;
-	}
-
-	if (left.type !== 'Identifier') {
 		return fallback();
 	}
 

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

@@ -862,7 +862,7 @@ export function invalidate_inner_signals(fn) {
 	}
 	let signal;
 	for (signal of captured_signals) {
-		mutate(signal, null /* doesnt matter */);
+		mutate(signal, () => {} /* doesnt matter */);
 	}
 	return captured_signals;
 }
@@ -870,26 +870,27 @@ export function invalidate_inner_signals(fn) {
 /**
  * @template V
  * @param {import('./types.js').Signal<V>} source
- * @param {V} value
+ * @param {(v: V) => any} update
  */
-export function mutate(source, value) {
+export function mutate(source, update) {
-	set_signal_value(
+	var value = untrack(() => get(source));
-		source,
+	var updated = update(value);
-		untrack(() => get(source))
+	set_signal_value(source, value);
-	);
+	return updated;
-	return value;
 }
 
 /**
  * Updates a store with a new value.
  * @param {import('./types.js').Store<V>} store  the store to update
- * @param {any} expression  the expression that mutates the store
+ * @param {() => V} get_value  function to retrieve the current store value
- * @param {V} new_value  the new store value
+ * @param {(v: V) => any} update  the expression that mutates the store
  * @template V
  */
-export function mutate_store(store, expression, new_value) {
+export function mutate_store(store, get_value, update) {
-	store.set(new_value);
+	var updated = update(untrack(get_value));
-	return expression;
+	// mutation could result in a new store being tracked, therefore call get_value again
+	store.set(untrack(get_value));
+	return updated;
 }
 
 /**

packages/svelte/tests/runtime-runes/samples/mutating-state/_config.js

@@ -0,0 +1,54 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+// Test ensures that reading the state that's mutated is done in a manner
+// so that the read is untracked so it doesn't trigger infinite loops
+export default test({
+	html: `
+		<input>
+		<input>
+		<p>text: foo</p>
+		<p>wrapped.contents: foo</p>
+	`,
+	skip_if_ssr: true,
+
+	test({ assert, target }) {
+		const [input1, input2] = target.querySelectorAll('input');
+
+		input1.value = 'bar';
+		flushSync(() => input1.dispatchEvent(new Event('input')));
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<input>
+			<input>
+			<p>text: bar</p>
+			<p>wrapped.contents: bar</p>
+		`
+		);
+
+		input2.value = 'baz';
+		flushSync(() => input2.dispatchEvent(new Event('input')));
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<input>
+			<input>
+			<p>text: bar</p>
+			<p>wrapped.contents: baz</p>
+		`
+		);
+
+		input1.value = 'foo';
+		flushSync(() => input1.dispatchEvent(new Event('input')));
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<input>
+			<input>
+			<p>text: foo</p>
+			<p>wrapped.contents: foo</p>
+		`
+		);
+	}
+});

packages/svelte/tests/runtime-runes/samples/mutating-state/main.svelte

@@ -0,0 +1,10 @@
+<script>
+	let text = $state('foo');
+	let wrapped = $state({});
+	$effect(() => { wrapped.contents = text; })
+</script>
+
+<input bind:value={text} />
+<input bind:value={wrapped.contents} />
+<p>text: {text}</p>
+<p>wrapped.contents: {wrapped.contents}</p> 

packages/svelte/tests/runtime-runes/samples/mutating-store-state/_config.js

@@ -0,0 +1,54 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+// Test ensures that reading the state that's mutated is done in a manner
+// so that the read is untracked so it doesn't trigger infinite loops
+export default test({
+	html: `
+		<input>
+		<input>
+		<p>text: foo</p>
+		<p>wrapped.contents: foo</p>
+	`,
+	skip_if_ssr: true,
+
+	test({ assert, target }) {
+		const [input1, input2] = target.querySelectorAll('input');
+
+		input1.value = 'bar';
+		flushSync(() => input1.dispatchEvent(new Event('input')));
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<input>
+			<input>
+			<p>text: bar</p>
+			<p>wrapped.contents: bar</p>
+		`
+		);
+
+		input2.value = 'baz';
+		flushSync(() => input2.dispatchEvent(new Event('input')));
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<input>
+			<input>
+			<p>text: bar</p>
+			<p>wrapped.contents: baz</p>
+		`
+		);
+
+		input1.value = 'foo';
+		flushSync(() => input1.dispatchEvent(new Event('input')));
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<input>
+			<input>
+			<p>text: foo</p>
+			<p>wrapped.contents: foo</p>
+		`
+		);
+	}
+});

packages/svelte/tests/runtime-runes/samples/mutating-store-state/main.svelte

@@ -0,0 +1,12 @@
+<script>
+	import { writable } from "svelte/store";
+
+	let text = writable('foo');
+	let wrapped = writable({});
+	$effect(() => { $wrapped.contents = $text; })
+</script>
+
+<input bind:value={$text} />
+<input bind:value={$wrapped.contents} />
+<p>text: {$text}</p>
+<p>wrapped.contents: {$wrapped.contents}</p>