fix: rewrite destructuring logic to handle iterators

.changeset/serious-moles-yell.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: rewrite destructuring logic to handle iterators

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

@@ -2,7 +2,7 @@
 /** @import { Binding } from '#compiler' */
 /** @import { ComponentClientTransformState, ComponentContext } from '../types' */
 import { dev } from '../../../../state.js';
-import { extract_paths } from '../../../../utils/ast.js';
+import { build_pattern, extract_paths } from '../../../../utils/ast.js';
 import * as b from '#compiler/builders';
 import * as assert from '../../../../utils/assert.js';
 import { get_rune } from '../../../scope.js';
@@ -141,20 +141,20 @@ export function VariableDeclaration(node, context) {
 						b.declarator(declarator.id, create_state_declarator(declarator.id, value))
 					);
 				} else {
-					const tmp = context.state.scope.generate('tmp');
-					const paths = extract_paths(declarator.id);
+					const [pattern, replacements] = build_pattern(declarator.id, context.state.scope);
 					declarations.push(
-						b.declarator(b.id(tmp), value),
-						...paths.map((path) => {
-							const value = path.expression?.(b.id(tmp));
-							const binding = context.state.scope.get(/** @type {Identifier} */ (path.node).name);
-							return b.declarator(
-								path.node,
-								binding?.kind === 'state' || binding?.kind === 'raw_state'
-									? create_state_declarator(binding.node, value)
-									: value
-							);
-						})
+						b.declarator(pattern, value),
+						.../** @type {[Identifier, Identifier][]} */ ([...replacements]).map(
+							([original, replacement]) => {
+								const binding = context.state.scope.get(original.name);
+								return b.declarator(
+									original,
+									binding?.kind === 'state' || binding?.kind === 'raw_state'
+										? create_state_declarator(binding.node, replacement)
+										: replacement
+								);
+							}
+						)
 					);
 				}
 
@@ -170,8 +170,7 @@ export function VariableDeclaration(node, context) {
 						)
 					);
 				} else {
-					const bindings = extract_paths(declarator.id);
-
+					const [pattern, replacements] = build_pattern(declarator.id, context.state.scope);
 					const init = /** @type {CallExpression} */ (declarator.init);
 
 					/** @type {Identifier} */
@@ -189,10 +188,16 @@ export function VariableDeclaration(node, context) {
 						);
 					}
 
-					for (let i = 0; i < bindings.length; i++) {
-						const binding = bindings[i];
+					for (let i = 0; i < replacements.size; i++) {
+						const [original, replacement] = [...replacements][i];
 						declarations.push(
-							b.declarator(binding.node, b.call('$.derived', b.thunk(binding.expression(rhs))))
+							b.declarator(
+								original,
+								b.call(
+									'$.derived',
+									b.arrow([], b.block([b.let(pattern, rhs), b.return(replacement)]))
+								)
+							)
 						);
 					}
 				}
@@ -304,19 +309,19 @@ function create_state_declarators(declarator, { scope, analysis }, value) {
 		];
 	}
 
-	const tmp = scope.generate('tmp');
-	const paths = extract_paths(declarator.id);
+	const [pattern, replacements] = build_pattern(declarator.id, scope);
 	return [
-		b.declarator(b.id(tmp), value),
-		...paths.map((path) => {
-			const value = path.expression?.(b.id(tmp));
-			const binding = scope.get(/** @type {Identifier} */ (path.node).name);
-			return b.declarator(
-				path.node,
-				binding?.kind === 'state'
-					? b.call('$.mutable_source', value, analysis.immutable ? b.true : undefined)
-					: value
-			);
-		})
+		b.declarator(pattern, value),
+		.../** @type {[Identifier, Identifier][]} */ ([...replacements]).map(
+			([original, replacement]) => {
+				const binding = scope.get(original.name);
+				return b.declarator(
+					original,
+					binding?.kind === 'state'
+						? b.call('$.mutable_source', replacement, analysis.immutable ? b.true : undefined)
+						: replacement
+				);
+			}
+		)
 	];
 }

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

@@ -3,7 +3,7 @@
 /** @import { Context } from '../types.js' */
 /** @import { ComponentAnalysis } from '../../../types.js' */
 /** @import { Scope } from '../../../scope.js' */
-import { build_fallback, extract_paths } from '../../../../utils/ast.js';
+import { build_pattern, build_fallback, extract_paths } from '../../../../utils/ast.js';
 import * as b from '#compiler/builders';
 import { get_rune } from '../../../scope.js';
 import { walk } from 'zimmerframe';
@@ -188,13 +188,10 @@ function create_state_declarators(declarator, scope, value) {
 		return [b.declarator(declarator.id, value)];
 	}
 
-	const tmp = scope.generate('tmp');
-	const paths = extract_paths(declarator.id);
+	const [pattern, replacements] = build_pattern(declarator.id, scope);
 	return [
-		b.declarator(b.id(tmp), value), // TODO inject declarator for opts, so we can use it below
-		...paths.map((path) => {
-			const value = path.expression?.(b.id(tmp));
-			return b.declarator(path.node, value);
-		})
+		b.declarator(pattern, value),
+		// TODO inject declarator for opts, so we can use it below
+		...[...replacements].map(([original, replacement]) => b.declarator(original, replacement))
 	];
 }

packages/svelte/src/compiler/phases/3-transform/shared/assignments.js

@@ -1,7 +1,7 @@
-/** @import { AssignmentExpression, AssignmentOperator, Expression, Node, Pattern } from 'estree' */
+/** @import { AssignmentExpression, AssignmentOperator, Expression, Identifier, Node, Pattern } from 'estree' */
 /** @import { Context as ClientContext } from '../client/types.js' */
 /** @import { Context as ServerContext } from '../server/types.js' */
-import { extract_paths, is_expression_async } from '../../../utils/ast.js';
+import { build_pattern, is_expression_async } from '../../../utils/ast.js';
 import * as b from '#compiler/builders';
 
 /**
@@ -23,47 +23,60 @@ export function visit_assignment_expression(node, context, build_assignment) {
 
 		let changed = false;
 
-		const assignments = extract_paths(node.left).map((path) => {
-			const value = path.expression?.(rhs);
+		const [pattern, replacements] = build_pattern(node.left, context.state.scope);
 
-			let assignment = build_assignment('=', path.node, value, context);
-			if (assignment !== null) changed = true;
-
-			return (
-				assignment ??
-				b.assignment(
-					'=',
-					/** @type {Pattern} */ (context.visit(path.node)),
-					/** @type {Expression} */ (context.visit(value))
-				)
-			);
-		});
+		const assignments = [
+			b.let(pattern, rhs),
+			...[...replacements].map(([original, replacement]) => {
+				let assignment = build_assignment(node.operator, original, replacement, context);
+				if (assignment !== null) changed = true;
+				return b.stmt(
+					assignment ??
+						b.assignment(
+							node.operator,
+							/** @type {Identifier} */ (context.visit(original)),
+							/** @type {Expression} */ (context.visit(replacement))
+						)
+				);
+			})
+		];
 
 		if (!changed) {
 			// No change to output -> nothing to transform -> we can keep the original assignment
 			return null;
 		}
 
 		const is_standalone = /** @type {Node} */ (context.path.at(-1)).type.endsWith('Statement');
-		const sequence = b.sequence(assignments);
+		const block = b.block(assignments);
 
 		if (!is_standalone) {
 			// this is part of an expression, we need the sequence to end with the value
-			sequence.expressions.push(rhs);
+			block.body.push(b.return(rhs));
 		}
 
-		if (should_cache) {
-			// the right hand side is a complex expression, wrap in an IIFE to cache it
-			const iife = b.arrow([rhs], sequence);
+		if (is_standalone && !should_cache) {
+			return block;
+		}
 
-			const iife_is_async =
-				is_expression_async(value) ||
-				assignments.some((assignment) => is_expression_async(assignment));
+		const iife = b.arrow(should_cache ? [rhs] : [], block);
 
-			return iife_is_async ? b.await(b.call(b.async(iife), value)) : b.call(iife, value);
-		}
+		const iife_is_async =
+			is_expression_async(value) ||
+			assignments.some(
+				(assignment) =>
+					(assignment.type === 'ExpressionStatement' &&
+						is_expression_async(assignment.expression)) ||
+					(assignment.type === 'VariableDeclaration' &&
+						assignment.declarations.some(
+							(declaration) =>
+								is_expression_async(declaration.id) ||
+								(declaration.init && is_expression_async(declaration.init))
+						))
+			);
 
-		return sequence;
+		return iife_is_async
+			? b.await(b.call(b.async(iife), should_cache ? value : undefined))
+			: b.call(iife, should_cache ? value : undefined);
 	}
 
 	if (node.left.type !== 'Identifier' && node.left.type !== 'MemberExpression') {

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

@@ -630,9 +630,10 @@ export class Scope {
 
 	/**
 	 * @param {string} preferred_name
+	 * @param {(name: string, counter: number) => string} [generator]
 	 * @returns {string}
 	 */
-	generate(preferred_name) {
+	generate(preferred_name, generator = (name, counter) => `${name}_${counter}`) {
 		if (this.#porous) {
 			return /** @type {Scope} */ (this.parent).generate(preferred_name);
 		}
@@ -647,7 +648,7 @@ export class Scope {
 			this.root.conflicts.has(name) ||
 			is_reserved(name)
 		) {
-			name = `${preferred_name}_${n++}`;
+			name = generator(preferred_name, n++);
 		}
 
 		this.references.set(name, []);

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

@@ -1,7 +1,8 @@
-/** @import { AST } from '#compiler' */
+/** @import { AST, Scope } from '#compiler' */
 /** @import * as ESTree from 'estree' */
 import { walk } from 'zimmerframe';
 import * as b from '#compiler/builders';
+import is_reference from 'is-reference';
 
 /**
  * Gets the left-most identifier of a member expression or identifier.
@@ -128,6 +129,49 @@ export function unwrap_pattern(pattern, nodes = []) {
 	return nodes;
 }
 
+/**
+ * @param {ESTree.Pattern} id
+ * @param {Scope} scope
+ * @returns {[ESTree.Pattern, Map<ESTree.Identifier | ESTree.MemberExpression, ESTree.Identifier>]}
+ */
+export function build_pattern(id, scope) {
+	/** @type {Map<ESTree.Identifier | ESTree.MemberExpression, ESTree.Identifier>} */
+	const map = new Map();
+
+	/** @type {Map<string, string>} */
+	const names = new Map();
+
+	let counter = 0;
+
+	for (const node of unwrap_pattern(id)) {
+		const name = scope.generate(`$$${++counter}`, (_, counter) => `$$${counter}`);
+
+		map.set(node, b.id(name));
+
+		if (node.type === 'Identifier') {
+			names.set(node.name, name);
+		}
+	}
+
+	const pattern = walk(id, null, {
+		Identifier(node, context) {
+			if (is_reference(node, /** @type {ESTree.Pattern} */ (context.path.at(-1)))) {
+				const name = names.get(node.name);
+				if (name) return b.id(name);
+			}
+		},
+
+		MemberExpression(node, context) {
+			const n = map.get(node);
+			if (n) return n;
+
+			context.next();
+		}
+	});
+
+	return [pattern, map];
+}
+
 /**
  * Extracts all identifiers from a pattern.
  * @param {ESTree.Pattern} pattern

packages/svelte/tests/snapshot/samples/destructured-assignments/_expected/client/index.svelte.js

@@ -7,10 +7,12 @@ let c = 3;
 let d = 4;
 
 export function update(array) {
-	(
-		$.set(a, array[0], true),
-		$.set(b, array[1], true)
-	);
+	{
+		let [$$1, $$2] = array;
+
+		$.set(a, $$1, true);
+		$.set(b, $$2, true);
+	};
 
 	[c, d] = array;
 }