diff --git a/.changeset/serious-moles-yell.md b/.changeset/serious-moles-yell.md new file mode 100644 index 000000000000..08f33dc05975 --- /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 84044e4dedca..84c205d16325 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 1f2bd3e2b1db..9482aad79a40 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 @@ -2,7 +2,7 @@ /** @import { Binding } from '#compiler' */ /** @import { Context } 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'; @@ -181,13 +181,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 3e6bb0c4c605..85b0869a1539 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 8297f174d3de..2123349dcd20 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -28,7 +28,7 @@ const globals = { 'Math.max': [NUMBER, Math.max], 'Math.random': [NUMBER], 'Math.floor': [NUMBER, Math.floor], - // @ts-expect-error + // @ts-ignore 'Math.f16round': [NUMBER, Math.f16round], 'Math.round': [NUMBER, Math.round], 'Math.abs': [NUMBER, Math.abs], @@ -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 108f4eff6414..3114c8463269 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,43 @@ 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) { + const nodes = unwrap_pattern(id); + /** @type {Map} */ + const map = new Map(); + /** @type {Map} */ + const ident_map = new Map(); + let counter = 0; + for (const node of nodes) { + map.set(node, b.id(scope.generate(`$$${++counter}`, (_, counter) => `$$${counter}`))); + if (node.type === 'Identifier') { + ident_map.set(node.name, /** @type {ESTree.Identifier} */ (map.get(node)).name); + } + } + const pattern = walk(id, null, { + _(node, { path, next }) { + if ( + (node.type === 'Identifier' && + is_reference(node, /** @type {ESTree.Pattern} */ (path.at(-1))) && + ident_map.has(node.name)) || + (node.type === 'MemberExpression' && map.has(node)) + ) { + return ( + map.get(node) ?? + b.id(/** @type {string} */ (ident_map.get(/** @type {ESTree.Identifier} */ (node).name))) + ); + } + return 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 47f297bce9c7..b2ef29ccafb5 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