fix: leave update expressions untransformed unless a transformer is provided (#14507)

* fix: leave update expressions untransformed unless a transformer is provided

* fix more cases

Author: Rich-Harris

.changeset/neat-news-dance.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: leave update expressions untransformed unless a transformer is provided

packages/svelte/src/compiler/phases/3-transform/client/types.d.ts

@@ -32,7 +32,7 @@ export interface ClientTransformState extends TransformState {
 			/** turn `foo = bar` into e.g. `$.set(foo, bar)` */
 			assign?: (node: Identifier, value: Expression) => Expression;
 			/** turn `foo.bar = baz` into e.g. `$.mutate(foo, $.get(foo).bar = baz);` */
-			mutate?: (node: Identifier, mutation: AssignmentExpression) => Expression;
+			mutate?: (node: Identifier, mutation: AssignmentExpression | UpdateExpression) => Expression;
 			/** turn `foo++` into e.g. `$.update(foo)` */
 			update?: (node: UpdateExpression) => Expression;
 		}

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

@@ -214,7 +214,10 @@ export function EachBlock(node, context) {
 
 				return b.sequence([b.assignment('=', left, value), ...sequence]);
 			},
-			mutate: (_, mutation) => b.sequence([mutation, ...sequence])
+			mutate: (_, mutation) => {
+				uses_index = true;
+				return b.sequence([mutation, ...sequence]);
+			}
 		};
 
 		delete key_state.transform[node.context.name];

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

@@ -77,13 +77,15 @@ export function Program(_, context) {
 					return b.call(
 						'$.store_mutate',
 						get_store(),
-						b.assignment(
-							mutation.operator,
-							/** @type {MemberExpression} */ (
-								replace(/** @type {MemberExpression} */ (mutation.left))
-							),
-							mutation.right
-						),
+						mutation.type === 'AssignmentExpression'
+							? b.assignment(
+									mutation.operator,
+									/** @type {MemberExpression} */ (
+										replace(/** @type {MemberExpression} */ (mutation.left))
+									),
+									mutation.right
+								)
+							: b.update(mutation.operator, replace(mutation.argument), mutation.prefix),
 						untracked
 					);
 				},

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

@@ -1,4 +1,4 @@
-/** @import { Expression, Node, Pattern, Statement, UpdateExpression } from 'estree' */
+/** @import { AssignmentExpression, Expression, UpdateExpression } from 'estree' */
 /** @import { Context } from '../types' */
 import { is_ignored } from '../../../../state.js';
 import { object } from '../../../../utils/ast.js';
@@ -34,34 +34,22 @@ export function UpdateExpression(node, context) {
 	}
 
 	const left = object(argument);
-	if (left === null) return context.next();
+	const transformers = left && context.state.transform[left.name];
 
-	if (left === argument) {
-		const transform = context.state.transform;
-		const update = transform[left.name]?.update;
-
-		if (update && Object.hasOwn(transform, left.name)) {
-			return update(node);
-		}
+	if (left === argument && transformers?.update) {
+		// we don't need to worry about ownership_invalid_mutation here, because
+		// we're not mutating but reassigning
+		return transformers.update(node);
 	}
 
-	const assignment = /** @type {Expression} */ (
-		context.visit(
-			b.assignment(
-				node.operator === '++' ? '+=' : '-=',
-				/** @type {Pattern} */ (argument),
-				b.literal(1)
-			)
-		)
-	);
+	let update = /** @type {Expression} */ (context.next());
 
-	const parent = /** @type {Node} */ (context.path.at(-1));
-	const is_standalone = parent.type === 'ExpressionStatement'; // TODO and possibly others, but not e.g. the `test` of a WhileStatement
-
-	const update =
-		node.prefix || is_standalone
-			? assignment
-			: b.binary(node.operator === '++' ? '-' : '+', assignment, b.literal(1));
+	if (left && transformers?.mutate) {
+		update = transformers.mutate(
+			left,
+			/** @type {AssignmentExpression | UpdateExpression} */ (update)
+		);
+	}
 
 	return is_ignored(node, 'ownership_invalid_mutation')
 		? b.call('$.skip_ownership_validation', b.thunk(update))

packages/svelte/tests/runtime-runes/samples/bigint-increment-mutation/_config.js

@@ -0,0 +1,15 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	html: '<button>mutate</button><button>reassign</button><p>0</p>',
+	test({ assert, target }) {
+		const [btn1, btn2] = target.querySelectorAll('button');
+
+		flushSync(() => btn1.click());
+		assert.htmlEqual(target.innerHTML, '<button>mutate</button><button>reassign</button><p>1</p>');
+
+		flushSync(() => btn2.click());
+		assert.htmlEqual(target.innerHTML, '<button>mutate</button><button>reassign</button><p>0</p>');
+	}
+});

packages/svelte/tests/runtime-runes/samples/bigint-increment-mutation/main.svelte

@@ -0,0 +1,16 @@
+<script>
+	let object = $state({ n: 0n });
+
+	function reassign() {
+		object = { n: 0n };
+	}
+
+	function mutate() {
+		return object.n++;
+	}
+</script>
+
+<button onclick={mutate}>mutate</button>
+<button onclick={reassign}>reassign</button>
+
+<p>{object.n}</p>

packages/svelte/tests/runtime-runes/samples/bigint-increment/_config.js

@@ -0,0 +1,7 @@
+import { test } from '../../test';
+
+export default test({
+	test({ assert, logs }) {
+		assert.deepEqual(logs, [0n, 1n, 2n, 3n, 4n, 5n]);
+	}
+});

packages/svelte/tests/runtime-runes/samples/bigint-increment/main.svelte

@@ -0,0 +1,5 @@
+<script>
+	for (let i = 0n; i <= 5n; i++) {
+		console.log(i);
+	}
+</script>