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;
 }