From a08f063b37cb89bc9711d20bd9dd4288cdb08db3 Mon Sep 17 00:00:00 2001
From: Simon H <5968653+dummdidumm@users.noreply.github.com>
Date: Mon, 21 Oct 2024 14:56:57 +0200
Subject: [PATCH] fix: more accurately detect `$derived` migration use cases
 (#13740)

- detect store mutations and not use `$derived` in that case, fixes #13723
- better detect `let x` that can be folded into `$derived`, fixes #13727

---------

Co-authored-by: ComputerGuy <63362464+Ocean-OS@users.noreply.github.com>
---
 .changeset/nasty-geese-turn.md                |  5 ++++
 packages/svelte/src/compiler/migrate/index.js | 23 +++++++++++++++----
 .../migrate/samples/derivations/input.svelte  |  5 ++++
 .../migrate/samples/derivations/output.svelte |  5 ++++
 .../migrate/samples/effects/input.svelte      |  3 +++
 .../migrate/samples/effects/output.svelte     |  9 ++++++--
 6 files changed, 43 insertions(+), 7 deletions(-)
 create mode 100644 .changeset/nasty-geese-turn.md

diff --git a/.changeset/nasty-geese-turn.md b/.changeset/nasty-geese-turn.md
new file mode 100644
index 0000000000..b8b1a9fc79
--- /dev/null
+++ b/.changeset/nasty-geese-turn.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: more accurately detect `$derived` migration opportunities
diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js
index c72988582d..8c51b48bfc 100644
--- a/packages/svelte/src/compiler/migrate/index.js
+++ b/packages/svelte/src/compiler/migrate/index.js
@@ -569,14 +569,22 @@ const instance_script = {
 						const declaration = reference.path.find((el) => el.type === 'VariableDeclaration');
 						const assignment = reference.path.find((el) => el.type === 'AssignmentExpression');
 						const update = reference.path.find((el) => el.type === 'UpdateExpression');
-						const labeled = reference.path.find(
-							(el) => el.type === 'LabeledStatement' && el.label.name === '$'
+						const labeled = /** @type {LabeledStatement | undefined} */ (
+							reference.path.find((el) => el.type === 'LabeledStatement' && el.label.name === '$')
 						);
 
-						if (assignment && labeled) {
+						if (
+							assignment &&
+							labeled &&
+							// ensure that $: foo = bar * 2 is not counted as a reassignment of bar
+							(labeled.body.type !== 'ExpressionStatement' ||
+								labeled.body.expression !== assignment ||
+								(assignment.left.type === 'Identifier' &&
+									assignment.left.name === binding.node.name))
+						) {
 							if (assignment_in_labeled) return false;
 							assignment_in_labeled = /** @type {AssignmentExpression} */ (assignment);
-							labeled_statement = /** @type {LabeledStatement} */ (labeled);
+							labeled_statement = labeled;
 						}
 
 						return (
@@ -750,7 +758,12 @@ const instance_script = {
 			);
 			const bindings = ids.map((id) => state.scope.get(id.name));
 			const reassigned_bindings = bindings.filter((b) => b?.reassigned);
-			if (reassigned_bindings.length === 0 && !bindings.some((b) => b?.kind === 'store_sub')) {
+
+			if (
+				reassigned_bindings.length === 0 &&
+				!bindings.some((b) => b?.kind === 'store_sub') &&
+				node.body.expression.left.type !== 'MemberExpression'
+			) {
 				let { start, end } = /** @type {{ start: number, end: number }} */ (
 					node.body.expression.right
 				);
diff --git a/packages/svelte/tests/migrate/samples/derivations/input.svelte b/packages/svelte/tests/migrate/samples/derivations/input.svelte
index 42b47b8272..8f89a59385 100644
--- a/packages/svelte/tests/migrate/samples/derivations/input.svelte
+++ b/packages/svelte/tests/migrate/samples/derivations/input.svelte
@@ -6,6 +6,11 @@
 	// no semicolon at the end
 	$: time_8 = count * 8
 	$: ({ time_16 } = { time_16: count * 16 })
+	// preceeding let that doesn't do anything
+	let time_32;
+	$: time_32 = count * doubled;
+	let very_high;
+	$: very_high = time_32 * count;
 </script>
 
 {count} / {doubled} / {quadrupled} / {time_8} / {time_16}
diff --git a/packages/svelte/tests/migrate/samples/derivations/output.svelte b/packages/svelte/tests/migrate/samples/derivations/output.svelte
index ed6e72dfab..23bac4ccaf 100644
--- a/packages/svelte/tests/migrate/samples/derivations/output.svelte
+++ b/packages/svelte/tests/migrate/samples/derivations/output.svelte
@@ -6,6 +6,11 @@
 	// no semicolon at the end
 	let time_8 = $derived(count * 8)
 	let { time_16 } = $derived({ time_16: count * 16 })
+	// preceeding let that doesn't do anything
+	let time_32 = $derived(count * doubled);
+	
+	let very_high = $derived(time_32 * count);
+	
 </script>
 
 {count} / {doubled} / {quadrupled} / {time_8} / {time_16}
\ No newline at end of file
diff --git a/packages/svelte/tests/migrate/samples/effects/input.svelte b/packages/svelte/tests/migrate/samples/effects/input.svelte
index f7f1edfb97..35d7c1bd51 100644
--- a/packages/svelte/tests/migrate/samples/effects/input.svelte
+++ b/packages/svelte/tests/migrate/samples/effects/input.svelte
@@ -10,4 +10,7 @@
 		console.log('bar');
 	}
 	$: $count = 1;
+	$: foo.x = count;
 </script>
+
+<button onclick={() => count++}>increment</button>
\ No newline at end of file
diff --git a/packages/svelte/tests/migrate/samples/effects/output.svelte b/packages/svelte/tests/migrate/samples/effects/output.svelte
index 006ce4b23d..59d7eec618 100644
--- a/packages/svelte/tests/migrate/samples/effects/output.svelte
+++ b/packages/svelte/tests/migrate/samples/effects/output.svelte
@@ -1,7 +1,7 @@
 <script>
 	import { run } from 'svelte/legacy';
 
-	let count = 0;
+	let count = $state(0);
 	run(() => {
 		console.log(count);
 	});
@@ -18,4 +18,9 @@
 	run(() => {
 		$count = 1;
 	});
-</script>
\ No newline at end of file
+	run(() => {
+		foo.x = count;
+	});
+</script>
+
+<button onclick={() => count++}>increment</button>
\ No newline at end of file