diff --git a/.changeset/serious-moles-yell.md b/.changeset/serious-moles-yell.md new file mode 100644 index 0000000000..08f33dc059 --- /dev/null +++ b/.changeset/serious-moles-yell.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: rewrite destructuring logic to handle iterators diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js index 84044e4ded..84c205d163 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js +++ b/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 + ); + } + ) ]; } diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/VariableDeclaration.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/VariableDeclaration.js index b76455b5c1..17bf516a22 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/VariableDeclaration.js +++ b/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)) ]; } diff --git a/packages/svelte/src/compiler/phases/3-transform/shared/assignments.js b/packages/svelte/src/compiler/phases/3-transform/shared/assignments.js index 3e6bb0c4c6..85b0869a15 100644 --- a/packages/svelte/src/compiler/phases/3-transform/shared/assignments.js +++ b/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,21 +23,23 @@ 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 @@ -45,25 +47,36 @@ export function visit_assignment_expression(node, context, build_assignment) { } 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') { diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index 75a26d487b..f37dfab8d1 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/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, []); diff --git a/packages/svelte/src/compiler/utils/ast.js b/packages/svelte/src/compiler/utils/ast.js index 23a95a1026..32ff5a37b3 100644 --- a/packages/svelte/src/compiler/utils/ast.js +++ b/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]} + */ +export function build_pattern(id, scope) { + /** @type {Map} */ + const map = new Map(); + + /** @type {Map} */ + 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 diff --git a/packages/svelte/tests/snapshot/samples/destructured-assignments/_expected/client/index.svelte.js b/packages/svelte/tests/snapshot/samples/destructured-assignments/_expected/client/index.svelte.js index 47f297bce9..b2ef29ccaf 100644 --- a/packages/svelte/tests/snapshot/samples/destructured-assignments/_expected/client/index.svelte.js +++ b/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; } \ No newline at end of file