fix: detect mutations within assignments expressions

This enhances our "variable was mutated" detection to also recognize that `foo` in `[foo[1]] = [..]` was mutated. This allows us to be more granular about detecting mutations to each expressions in legacy mode, which fixes #12169

Author: dummdidumm

.changeset/thirty-dogs-whisper.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: detect mutations within assignment expressions

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

@@ -769,8 +769,7 @@ const legacy_scope_tweaker = {
 			}
 
 			if (
-				binding !== null &&
-				binding.kind === 'normal' &&
+				binding?.kind === 'normal' &&
 				((binding.scope === state.instance_scope && binding.declaration_kind !== 'function') ||
 					binding.declaration_kind === 'import')
 			) {
@@ -795,23 +794,24 @@ const legacy_scope_tweaker = {
 					parent.left === binding.node
 				) {
 					binding.kind = 'derived';
-				} else {
-					let idx = -1;
-					let ancestor = path.at(idx);
-					while (ancestor) {
-						if (ancestor.type === 'EachBlock') {
-							// Ensures that the array is reactive when only its entries are mutated
-							// TODO: this doesn't seem correct. We should be checking at the points where
-							// the identifier (the each expression) is used in a way that makes it reactive.
-							// This just forces the collection identifier to always be reactive even if it's
-							// not.
-							if (ancestor.expression === (idx === -1 ? node : path.at(idx + 1))) {
-								binding.kind = 'state';
-								break;
+				}
+			} else if (binding?.kind === 'each' && binding.mutated) {
+				// Ensure that the array is marked as reactive even when only its entries are mutated
+				let idx = -1;
+				let ancestor = path.at(idx);
+				while (ancestor) {
+					if (
+						ancestor.type === 'EachBlock' &&
+						state.analysis.template.scopes.get(ancestor)?.declarations.get(node.name) === binding
+					) {
+						for (const reference of ancestor.metadata.references) {
+							if (reference.kind === 'normal') {
+								reference.kind = 'state';
 							}
 						}
-						ancestor = path.at(--idx);
+						break;
 					}
+					ancestor = path.at(--idx);
 				}
 			}
 		}

packages/svelte/src/compiler/phases/scope.js

@@ -713,7 +713,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
 			const binding = scope.get(/** @type {import('estree').Identifier} */ (object).name);
 			if (binding) binding.mutated = true;
 		} else {
-			extract_identifiers(node).forEach((identifier) => {
+			extract_identifiers(node, [], true).forEach((identifier) => {
 				const binding = scope.get(identifier.name);
 				if (binding && identifier !== binding.node) {
 					binding.mutated = true;

packages/svelte/src/compiler/utils/ast.js

@@ -54,41 +54,51 @@ export function is_event_attribute(attribute) {
 }
 
 /**
- * Extracts all identifiers from a pattern.
+ * Extracts all identifiers from a pattern. By default only those which can be newly declared as part of an assignment expression are included.
  * @param {ESTree.Pattern} param
  * @param {ESTree.Identifier[]} [nodes]
+ * @param {boolean} [include_member_expressions] If `true`, will also include member expressions which can be important in a case like `[a, b] = [c, d]`
  * @returns {ESTree.Identifier[]}
  */
-export function extract_identifiers(param, nodes = []) {
+export function extract_identifiers(param, nodes = [], include_member_expressions = false) {
 	switch (param.type) {
 		case 'Identifier':
 			nodes.push(param);
 			break;
 
+		case 'MemberExpression':
+			if (
+				include_member_expressions &&
+				(param.object.type === 'Identifier' || param.object.type === 'MemberExpression')
+			) {
+				extract_identifiers(param.object, nodes, include_member_expressions);
+			}
+			break;
+
 		case 'ObjectPattern':
 			for (const prop of param.properties) {
 				if (prop.type === 'RestElement') {
-					extract_identifiers(prop.argument, nodes);
+					extract_identifiers(prop.argument, nodes, include_member_expressions);
 				} else {
-					extract_identifiers(prop.value, nodes);
+					extract_identifiers(prop.value, nodes, include_member_expressions);
 				}
 			}
 
 			break;
 
 		case 'ArrayPattern':
 			for (const element of param.elements) {
-				if (element) extract_identifiers(element, nodes);
+				if (element) extract_identifiers(element, nodes, include_member_expressions);
 			}
 
 			break;
 
 		case 'RestElement':
-			extract_identifiers(param.argument, nodes);
+			extract_identifiers(param.argument, nodes, include_member_expressions);
 			break;
 
 		case 'AssignmentPattern':
-			extract_identifiers(param.left, nodes);
+			extract_identifiers(param.left, nodes, include_member_expressions);
 			break;
 	}
 

packages/svelte/tests/migrate/samples/each-block-const/input.svelte

@@ -0,0 +1,5 @@
+<script>
+	const { foo } = x();
+</script>
+
+{#each foo as f}{/each}

packages/svelte/tests/migrate/samples/each-block-const/output.svelte

@@ -0,0 +1,5 @@
+<script>
+	const { foo } = x();
+</script>
+
+{#each foo as f}{/each}