From 81a34aa13426c6409da348b2a48f12f919276d43 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 28 May 2025 17:33:04 -0400 Subject: [PATCH 01/17] fix: handle derived destructured iterators (#16015) * revert #15813 * failing test * tweak * tweak * WIP * WIP * WIP * WIP * WIP * working * tweak * changeset * tweak description * Update packages/svelte/src/compiler/utils/ast.js Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> --------- Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> --- .changeset/purple-dryers-mate.md | 5 + packages/svelte/src/compiler/migrate/index.js | 8 +- .../2-analyze/visitors/VariableDeclarator.js | 3 +- .../3-transform/client/visitors/EachBlock.js | 18 +- .../client/visitors/SnippetBlock.js | 15 +- .../client/visitors/VariableDeclaration.js | 154 ++++++++------- .../server/visitors/VariableDeclaration.js | 29 ++- .../phases/3-transform/shared/assignments.js | 93 +++++----- packages/svelte/src/compiler/phases/scope.js | 5 +- packages/svelte/src/compiler/utils/ast.js | 175 ++++++++---------- packages/svelte/src/internal/client/index.js | 2 +- packages/svelte/src/internal/server/index.js | 2 +- packages/svelte/src/internal/shared/utils.js | 35 ++++ .../derived-destructured-iterator/_config.js | 16 ++ .../derived-destructured-iterator/main.svelte | 16 ++ .../_expected/client/index.svelte.js | 10 +- 16 files changed, 348 insertions(+), 238 deletions(-) create mode 100644 .changeset/purple-dryers-mate.md create mode 100644 packages/svelte/tests/runtime-runes/samples/derived-destructured-iterator/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/derived-destructured-iterator/main.svelte diff --git a/.changeset/purple-dryers-mate.md b/.changeset/purple-dryers-mate.md new file mode 100644 index 0000000000..fc8e96d6d4 --- /dev/null +++ b/.changeset/purple-dryers-mate.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: handle derived destructured iterators diff --git a/packages/svelte/src/compiler/migrate/index.js b/packages/svelte/src/compiler/migrate/index.js index 2d5a4dcd9e..5ca9adb98b 100644 --- a/packages/svelte/src/compiler/migrate/index.js +++ b/packages/svelte/src/compiler/migrate/index.js @@ -603,15 +603,15 @@ const instance_script = { ); // Turn export let into props. It's really really weird because export let { x: foo, z: [bar]} = .. // means that foo and bar are the props (i.e. the leafs are the prop names), not x and z. - // const tmp = state.scope.generate('tmp'); - // const paths = extract_paths(declarator.id); + // const tmp = b.id(state.scope.generate('tmp')); + // const paths = extract_paths(declarator.id, tmp); // state.props_pre.push( - // b.declaration('const', b.id(tmp), visit(declarator.init!) as Expression) + // b.declaration('const', tmp, visit(declarator.init!) as Expression) // ); // for (const path of paths) { // const name = (path.node as Identifier).name; // const binding = state.scope.get(name)!; - // const value = path.expression!(b.id(tmp)); + // const value = path.expression; // if (binding.kind === 'bindable_prop' || binding.kind === 'rest_prop') { // state.props.push({ // local: name, diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/VariableDeclarator.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/VariableDeclarator.js index c4b4272aba..89320f3962 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/VariableDeclarator.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/VariableDeclarator.js @@ -7,6 +7,7 @@ import * as e from '../../../errors.js'; import * as w from '../../../warnings.js'; import { extract_paths } from '../../../utils/ast.js'; import { equal } from '../../../utils/assert.js'; +import * as b from '#compiler/builders'; /** * @param {VariableDeclarator} node @@ -18,7 +19,7 @@ export function VariableDeclarator(node, context) { if (context.state.analysis.runes) { const init = node.init; const rune = get_rune(init, context.state.scope); - const paths = extract_paths(node.id); + const { paths } = extract_paths(node.id, b.id('dummy')); for (const path of paths) { validate_identifier_name(context.state.scope.get(/** @type {Identifier} */ (path.node).name)); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js index f80fa0e61e..5a944b6db3 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/EachBlock.js @@ -234,13 +234,21 @@ export function EachBlock(node, context) { } else if (node.context) { const unwrapped = (flags & EACH_ITEM_REACTIVE) !== 0 ? b.call('$.get', item) : item; - for (const path of extract_paths(node.context)) { + const { inserts, paths } = extract_paths(node.context, unwrapped); + + for (const { id, value } of inserts) { + id.name = context.state.scope.generate('$$array'); + child_state.transform[id.name] = { read: get_value }; + + const expression = /** @type {Expression} */ (context.visit(b.thunk(value), child_state)); + declarations.push(b.var(id, b.call('$.derived', expression))); + } + + for (const path of paths) { const name = /** @type {Identifier} */ (path.node).name; const needs_derived = path.has_default_value; // to ensure that default value is only called once - const fn = b.thunk( - /** @type {Expression} */ (context.visit(path.expression?.(unwrapped), child_state)) - ); + const fn = b.thunk(/** @type {Expression} */ (context.visit(path.expression, child_state))); declarations.push(b.let(path.node, needs_derived ? b.call('$.derived_safe_equal', fn) : fn)); @@ -249,7 +257,7 @@ export function EachBlock(node, context) { child_state.transform[name] = { read, assign: (_, value) => { - const left = /** @type {Pattern} */ (path.update_expression(unwrapped)); + const left = /** @type {Pattern} */ (path.update_expression); return b.sequence([b.assignment('=', left, value), ...sequence]); }, mutate: (_, mutation) => { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js index a82645cd7a..0809aa21b2 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SnippetBlock.js @@ -43,14 +43,21 @@ export function SnippetBlock(node, context) { let arg_alias = `$$arg${i}`; args.push(b.id(arg_alias)); - const paths = extract_paths(argument); + const { inserts, paths } = extract_paths(argument, b.maybe_call(b.id(arg_alias))); + + for (const { id, value } of inserts) { + id.name = context.state.scope.generate('$$array'); + transform[id.name] = { read: get_value }; + + declarations.push( + b.var(id, b.call('$.derived', /** @type {Expression} */ (context.visit(b.thunk(value))))) + ); + } for (const path of paths) { const name = /** @type {Identifier} */ (path.node).name; const needs_derived = path.has_default_value; // to ensure that default value is only called once - const fn = b.thunk( - /** @type {Expression} */ (context.visit(path.expression?.(b.maybe_call(b.id(arg_alias))))) - ); + const fn = b.thunk(/** @type {Expression} */ (context.visit(path.expression))); declarations.push(b.let(path.node, needs_derived ? b.call('$.derived_safe_equal', fn) : fn)); 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 84c205d163..e717077917 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,12 +2,13 @@ /** @import { Binding } from '#compiler' */ /** @import { ComponentClientTransformState, ComponentContext } from '../types' */ import { dev } from '../../../../state.js'; -import { build_pattern, extract_paths } from '../../../../utils/ast.js'; +import { 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'; import { get_prop_source, is_prop_source, is_state_source, should_proxy } from '../utils.js'; import { is_hoisted_function } from '../../utils.js'; +import { get_value } from './shared/declarations.js'; /** * @param {VariableDeclaration} node @@ -116,7 +117,7 @@ export function VariableDeclaration(node, context) { } const args = /** @type {CallExpression} */ (init).arguments; - const value = args.length > 0 ? /** @type {Expression} */ (context.visit(args[0])) : b.void0; + const value = /** @type {Expression} */ (args[0]) ?? b.void0; // TODO do we need the void 0? can we just omit it altogether? if (rune === '$state' || rune === '$state.raw') { /** @@ -137,24 +138,34 @@ export function VariableDeclaration(node, context) { }; if (declarator.id.type === 'Identifier') { + const expression = /** @type {Expression} */ (context.visit(value)); + declarations.push( - b.declarator(declarator.id, create_state_declarator(declarator.id, value)) + b.declarator(declarator.id, create_state_declarator(declarator.id, expression)) ); } else { - const [pattern, replacements] = build_pattern(declarator.id, context.state.scope); + const tmp = b.id(context.state.scope.generate('tmp')); + const { inserts, paths } = extract_paths(declarator.id, tmp); + declarations.push( - 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 - ); - } - ) + b.declarator(tmp, value), + ...inserts.map(({ id, value }) => { + id.name = context.state.scope.generate('$$array'); + context.state.transform[id.name] = { read: get_value }; + + const expression = /** @type {Expression} */ (context.visit(b.thunk(value))); + return b.declarator(id, b.call('$.derived', expression)); + }), + ...paths.map((path) => { + const value = /** @type {Expression} */ (context.visit(path.expression)); + 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 + ); + }) ); } @@ -163,44 +174,41 @@ export function VariableDeclaration(node, context) { if (rune === '$derived' || rune === '$derived.by') { if (declarator.id.type === 'Identifier') { - declarations.push( - b.declarator( - declarator.id, - b.call('$.derived', rune === '$derived.by' ? value : b.thunk(value)) - ) - ); + let expression = /** @type {Expression} */ (context.visit(value)); + if (rune === '$derived') expression = b.thunk(expression); + + declarations.push(b.declarator(declarator.id, b.call('$.derived', expression))); } else { - const [pattern, replacements] = build_pattern(declarator.id, context.state.scope); const init = /** @type {CallExpression} */ (declarator.init); - /** @type {Identifier} */ - let id; let rhs = value; - if (rune === '$derived' && init.arguments[0].type === 'Identifier') { - id = init.arguments[0]; - } else { - id = b.id(context.state.scope.generate('$$d')); + if (rune !== '$derived' || init.arguments[0].type !== 'Identifier') { + const id = b.id(context.state.scope.generate('$$d')); rhs = b.call('$.get', id); - declarations.push( - b.declarator(id, b.call('$.derived', rune === '$derived.by' ? value : b.thunk(value))) - ); + let expression = /** @type {Expression} */ (context.visit(value)); + if (rune === '$derived') expression = b.thunk(expression); + + declarations.push(b.declarator(id, b.call('$.derived', expression))); } - for (let i = 0; i < replacements.size; i++) { - const [original, replacement] = [...replacements][i]; - declarations.push( - b.declarator( - original, - b.call( - '$.derived', - b.arrow([], b.block([b.let(pattern, rhs), b.return(replacement)])) - ) - ) - ); + const { inserts, paths } = extract_paths(declarator.id, rhs); + + for (const { id, value } of inserts) { + id.name = context.state.scope.generate('$$array'); + context.state.transform[id.name] = { read: get_value }; + + const expression = /** @type {Expression} */ (context.visit(b.thunk(value))); + declarations.push(b.declarator(id, b.call('$.derived', expression))); + } + + for (const path of paths) { + const expression = /** @type {Expression} */ (context.visit(path.expression)); + declarations.push(b.declarator(path.node, b.call('$.derived', b.thunk(expression)))); } } + continue; } } @@ -229,20 +237,29 @@ export function VariableDeclaration(node, context) { if (declarator.id.type !== 'Identifier') { // Turn export let into props. It's really really weird because export let { x: foo, z: [bar]} = .. // means that foo and bar are the props (i.e. the leafs are the prop names), not x and z. - const tmp = context.state.scope.generate('tmp'); - const paths = extract_paths(declarator.id); + const tmp = b.id(context.state.scope.generate('tmp')); + const { inserts, paths } = extract_paths(declarator.id, tmp); declarations.push( b.declarator( - b.id(tmp), + tmp, /** @type {Expression} */ (context.visit(/** @type {Expression} */ (declarator.init))) ) ); + for (const { id, value } of inserts) { + id.name = context.state.scope.generate('$$array'); + context.state.transform[id.name] = { read: get_value }; + + const expression = /** @type {Expression} */ (context.visit(b.thunk(value))); + declarations.push(b.declarator(id, b.call('$.derived', expression))); + } + for (const path of paths) { const name = /** @type {Identifier} */ (path.node).name; const binding = /** @type {Binding} */ (context.state.scope.get(name)); - const value = path.expression?.(b.id(tmp)); + const value = /** @type {Expression} */ (context.visit(path.expression)); + declarations.push( b.declarator( path.node, @@ -276,7 +293,7 @@ export function VariableDeclaration(node, context) { declarations.push( ...create_state_declarators( declarator, - context.state, + context, /** @type {Expression} */ (declarator.init && context.visit(declarator.init)) ) ); @@ -296,32 +313,41 @@ export function VariableDeclaration(node, context) { /** * Creates the output for a state declaration in legacy mode. * @param {VariableDeclarator} declarator - * @param {ComponentClientTransformState} scope + * @param {ComponentContext} context * @param {Expression} value */ -function create_state_declarators(declarator, { scope, analysis }, value) { +function create_state_declarators(declarator, context, value) { if (declarator.id.type === 'Identifier') { return [ b.declarator( declarator.id, - b.call('$.mutable_source', value, analysis.immutable ? b.true : undefined) + b.call('$.mutable_source', value, context.state.analysis.immutable ? b.true : undefined) ) ]; } - const [pattern, replacements] = build_pattern(declarator.id, scope); + const tmp = b.id(context.state.scope.generate('tmp')); + const { inserts, paths } = extract_paths(declarator.id, tmp); + return [ - 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 - ); - } - ) + b.declarator(tmp, value), + ...inserts.map(({ id, value }) => { + id.name = context.state.scope.generate('$$array'); + context.state.transform[id.name] = { read: get_value }; + + const expression = /** @type {Expression} */ (context.visit(b.thunk(value))); + return b.declarator(id, b.call('$.derived', expression)); + }), + ...paths.map((path) => { + const value = /** @type {Expression} */ (context.visit(path.expression)); + const binding = context.state.scope.get(/** @type {Identifier} */ (path.node).name); + + return b.declarator( + path.node, + binding?.kind === 'state' + ? b.call('$.mutable_source', value, context.state.analysis.immutable ? b.true : undefined) + : value + ); + }) ]; } 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 17bf516a22..1f0e6be77c 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_pattern, build_fallback, extract_paths } from '../../../../utils/ast.js'; +import { build_fallback, extract_paths } from '../../../../utils/ast.js'; import * as b from '#compiler/builders'; import { get_rune } from '../../../scope.js'; import { walk } from 'zimmerframe'; @@ -120,21 +120,29 @@ export function VariableDeclaration(node, context) { if (declarator.id.type !== 'Identifier') { // Turn export let into props. It's really really weird because export let { x: foo, z: [bar]} = .. // means that foo and bar are the props (i.e. the leafs are the prop names), not x and z. - const tmp = context.state.scope.generate('tmp'); - const paths = extract_paths(declarator.id); + const tmp = b.id(context.state.scope.generate('tmp')); + const { inserts, paths } = extract_paths(declarator.id, tmp); + declarations.push( b.declarator( - b.id(tmp), + tmp, /** @type {Expression} */ (context.visit(/** @type {Expression} */ (declarator.init))) ) ); + + for (const { id, value } of inserts) { + id.name = context.state.scope.generate('$$array'); + declarations.push(b.declarator(id, value)); + } + for (const path of paths) { - const value = path.expression?.(b.id(tmp)); + const value = path.expression; const name = /** @type {Identifier} */ (path.node).name; const binding = /** @type {Binding} */ (context.state.scope.get(name)); const prop = b.member(b.id('$$props'), b.literal(binding.prop_alias ?? name), true); declarations.push(b.declarator(path.node, build_fallback(prop, value))); } + continue; } @@ -188,10 +196,13 @@ function create_state_declarators(declarator, scope, value) { return [b.declarator(declarator.id, value)]; } - const [pattern, replacements] = build_pattern(declarator.id, scope); + const tmp = b.id(scope.generate('tmp')); + const { paths } = extract_paths(declarator.id, tmp); return [ - b.declarator(pattern, value), - // TODO inject declarator for opts, so we can use it below - ...[...replacements].map(([original, replacement]) => b.declarator(original, replacement)) + b.declarator(tmp, value), // TODO inject declarator for opts, so we can use it below + ...paths.map((path) => { + const value = path.expression; + return b.declarator(path.node, value); + }) ]; } 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 85b0869a15..175b44f4fe 100644 --- a/packages/svelte/src/compiler/phases/3-transform/shared/assignments.js +++ b/packages/svelte/src/compiler/phases/3-transform/shared/assignments.js @@ -1,8 +1,9 @@ -/** @import { AssignmentExpression, AssignmentOperator, Expression, Identifier, Node, Pattern } from 'estree' */ +/** @import { AssignmentExpression, AssignmentOperator, Expression, Node, Pattern, Statement } from 'estree' */ /** @import { Context as ClientContext } from '../client/types.js' */ /** @import { Context as ServerContext } from '../server/types.js' */ -import { build_pattern, is_expression_async } from '../../../utils/ast.js'; +import { extract_paths, is_expression_async } from '../../../utils/ast.js'; import * as b from '#compiler/builders'; +import { get_value } from '../client/visitors/shared/declarations.js'; /** * @template {ClientContext | ServerContext} Context @@ -23,23 +24,27 @@ export function visit_assignment_expression(node, context, build_assignment) { let changed = false; - const [pattern, replacements] = build_pattern(node.left, context.state.scope); - - 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)) - ) - ); - }) - ]; + const { inserts, paths } = extract_paths(node.left, rhs); + + for (const { id } of inserts) { + id.name = context.state.scope.generate('$$array'); + } + + const assignments = paths.map((path) => { + const value = path.expression; + + 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)) + ) + ); + }); if (!changed) { // No change to output -> nothing to transform -> we can keep the original assignment @@ -47,36 +52,36 @@ export function visit_assignment_expression(node, context, build_assignment) { } const is_standalone = /** @type {Node} */ (context.path.at(-1)).type.endsWith('Statement'); - const block = b.block(assignments); - if (!is_standalone) { - // this is part of an expression, we need the sequence to end with the value - block.body.push(b.return(rhs)); - } + if (inserts.length > 0 || should_cache) { + /** @type {Statement[]} */ + const statements = [ + ...inserts.map(({ id, value }) => b.var(id, value)), + ...assignments.map(b.stmt) + ]; - if (is_standalone && !should_cache) { - return block; + if (!is_standalone) { + // this is part of an expression, we need the sequence to end with the value + statements.push(b.return(rhs)); + } + + const iife = b.arrow([rhs], b.block(statements)); + + const iife_is_async = + is_expression_async(value) || + assignments.some((assignment) => is_expression_async(assignment)); + + return iife_is_async ? b.await(b.call(b.async(iife), value)) : b.call(iife, value); } - const iife = b.arrow(should_cache ? [rhs] : [], block); - - 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)) - )) - ); + const sequence = b.sequence(assignments); + + if (!is_standalone) { + // this is part of an expression, we need the sequence to end with the value + sequence.expressions.push(rhs); + } - return iife_is_async - ? b.await(b.call(b.async(iife), should_cache ? value : undefined)) - : b.call(iife, should_cache ? value : undefined); + return sequence; } 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 f37dfab8d1..75a26d487b 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -630,10 +630,9 @@ export class Scope { /** * @param {string} preferred_name - * @param {(name: string, counter: number) => string} [generator] * @returns {string} */ - generate(preferred_name, generator = (name, counter) => `${name}_${counter}`) { + generate(preferred_name) { if (this.#porous) { return /** @type {Scope} */ (this.parent).generate(preferred_name); } @@ -648,7 +647,7 @@ export class Scope { this.root.conflicts.has(name) || is_reserved(name) ) { - name = generator(preferred_name, n++); + name = `${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 32ff5a37b3..a20b6888c7 100644 --- a/packages/svelte/src/compiler/utils/ast.js +++ b/packages/svelte/src/compiler/utils/ast.js @@ -1,8 +1,7 @@ -/** @import { AST, Scope } from '#compiler' */ +/** @import { AST } 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. @@ -129,49 +128,6 @@ 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 @@ -271,40 +227,50 @@ export function extract_identifiers_from_destructuring(node, nodes = []) { * @property {ESTree.Identifier | ESTree.MemberExpression} node The node the destructuring path end in. Can be a member expression only for assignment expressions * @property {boolean} is_rest `true` if this is a `...rest` destructuring * @property {boolean} has_default_value `true` if this has a fallback value like `const { foo = 'bar } = ..` - * @property {(expression: ESTree.Expression) => ESTree.Identifier | ESTree.MemberExpression | ESTree.CallExpression | ESTree.AwaitExpression} expression Returns an expression which walks the path starting at the given expression. + * @property {ESTree.Expression} expression The value of the current path * This will be a call expression if a rest element or default is involved — e.g. `const { foo: { bar: baz = 42 }, ...rest } = quux` — since we can't represent `baz` or `rest` purely as a path * Will be an await expression in case of an async default value (`const { foo = await bar } = ...`) - * @property {(expression: ESTree.Expression) => ESTree.Identifier | ESTree.MemberExpression | ESTree.CallExpression | ESTree.AwaitExpression} update_expression Like `expression` but without default values. + * @property {ESTree.Expression} update_expression Like `expression` but without default values. */ /** * Extracts all destructured assignments from a pattern. + * For each `id` in the returned `inserts`, make sure to adjust the `name`. * @param {ESTree.Node} param - * @returns {DestructuredAssignment[]} + * @param {ESTree.Expression} initial + * @returns {{ inserts: Array<{ id: ESTree.Identifier, value: ESTree.Expression }>, paths: DestructuredAssignment[] }} */ -export function extract_paths(param) { - return _extract_paths( - [], - param, - (node) => /** @type {ESTree.Identifier | ESTree.MemberExpression} */ (node), - (node) => /** @type {ESTree.Identifier | ESTree.MemberExpression} */ (node), - false - ); +export function extract_paths(param, initial) { + /** + * When dealing with array destructuring patterns (`let [a, b, c] = $derived(blah())`) + * we need an intermediate declaration that creates an array, since `blah()` could + * return a non-array-like iterator + * @type {Array<{ id: ESTree.Identifier, value: ESTree.Expression }>} + */ + const inserts = []; + + /** @type {DestructuredAssignment[]} */ + const paths = []; + + _extract_paths(paths, inserts, param, initial, initial, false); + + return { inserts, paths }; } /** - * @param {DestructuredAssignment[]} assignments + * @param {DestructuredAssignment[]} paths + * @param {Array<{ id: ESTree.Identifier, value: ESTree.Expression }>} inserts * @param {ESTree.Node} param - * @param {DestructuredAssignment['expression']} expression - * @param {DestructuredAssignment['update_expression']} update_expression + * @param {ESTree.Expression} expression + * @param {ESTree.Expression} update_expression * @param {boolean} has_default_value * @returns {DestructuredAssignment[]} */ -function _extract_paths(assignments = [], param, expression, update_expression, has_default_value) { +function _extract_paths(paths, inserts, param, expression, update_expression, has_default_value) { switch (param.type) { case 'Identifier': case 'MemberExpression': - assignments.push({ + paths.push({ node: param, is_rest: false, has_default_value, @@ -316,28 +282,25 @@ function _extract_paths(assignments = [], param, expression, update_expression, case 'ObjectPattern': for (const prop of param.properties) { if (prop.type === 'RestElement') { - /** @type {DestructuredAssignment['expression']} */ - const rest_expression = (object) => { - /** @type {ESTree.Expression[]} */ - const props = []; - - for (const p of param.properties) { - if (p.type === 'Property' && p.key.type !== 'PrivateIdentifier') { - if (p.key.type === 'Identifier' && !p.computed) { - props.push(b.literal(p.key.name)); - } else if (p.key.type === 'Literal') { - props.push(b.literal(String(p.key.value))); - } else { - props.push(b.call('String', p.key)); - } + /** @type {ESTree.Expression[]} */ + const props = []; + + for (const p of param.properties) { + if (p.type === 'Property' && p.key.type !== 'PrivateIdentifier') { + if (p.key.type === 'Identifier' && !p.computed) { + props.push(b.literal(p.key.name)); + } else if (p.key.type === 'Literal') { + props.push(b.literal(String(p.key.value))); + } else { + props.push(b.call('String', p.key)); } } + } - return b.call('$.exclude_from_object', expression(object), b.array(props)); - }; + const rest_expression = b.call('$.exclude_from_object', expression, b.array(props)); if (prop.argument.type === 'Identifier') { - assignments.push({ + paths.push({ node: prop.argument, is_rest: true, has_default_value, @@ -346,7 +309,8 @@ function _extract_paths(assignments = [], param, expression, update_expression, }); } else { _extract_paths( - assignments, + paths, + inserts, prop.argument, rest_expression, rest_expression, @@ -354,11 +318,15 @@ function _extract_paths(assignments = [], param, expression, update_expression, ); } } else { - /** @type {DestructuredAssignment['expression']} */ - const object_expression = (object) => - b.member(expression(object), prop.key, prop.computed || prop.key.type !== 'Identifier'); + const object_expression = b.member( + expression, + prop.key, + prop.computed || prop.key.type !== 'Identifier' + ); + _extract_paths( - assignments, + paths, + inserts, prop.value, object_expression, object_expression, @@ -369,16 +337,27 @@ function _extract_paths(assignments = [], param, expression, update_expression, break; - case 'ArrayPattern': + case 'ArrayPattern': { + // we create an intermediate declaration to convert iterables to arrays if necessary. + // the consumer is responsible for setting the name of the identifier + const id = b.id('#'); + + const value = b.call( + '$.to_array', + expression, + param.elements.at(-1)?.type === 'RestElement' ? undefined : b.literal(param.elements.length) + ); + + inserts.push({ id, value }); + for (let i = 0; i < param.elements.length; i += 1) { const element = param.elements[i]; if (element) { if (element.type === 'RestElement') { - /** @type {DestructuredAssignment['expression']} */ - const rest_expression = (object) => - b.call(b.member(expression(object), 'slice'), b.literal(i)); + const rest_expression = b.call(b.member(id, 'slice'), b.literal(i)); + if (element.argument.type === 'Identifier') { - assignments.push({ + paths.push({ node: element.argument, is_rest: true, has_default_value, @@ -387,7 +366,8 @@ function _extract_paths(assignments = [], param, expression, update_expression, }); } else { _extract_paths( - assignments, + paths, + inserts, element.argument, rest_expression, rest_expression, @@ -395,10 +375,11 @@ function _extract_paths(assignments = [], param, expression, update_expression, ); } } else { - /** @type {DestructuredAssignment['expression']} */ - const array_expression = (object) => b.member(expression(object), b.literal(i), true); + const array_expression = b.member(id, b.literal(i), true); + _extract_paths( - assignments, + paths, + inserts, element, array_expression, array_expression, @@ -409,13 +390,13 @@ function _extract_paths(assignments = [], param, expression, update_expression, } break; + } case 'AssignmentPattern': { - /** @type {DestructuredAssignment['expression']} */ - const fallback_expression = (object) => build_fallback(expression(object), param.right); + const fallback_expression = build_fallback(expression, param.right); if (param.left.type === 'Identifier') { - assignments.push({ + paths.push({ node: param.left, is_rest: false, has_default_value: true, @@ -423,14 +404,14 @@ function _extract_paths(assignments = [], param, expression, update_expression, update_expression }); } else { - _extract_paths(assignments, param.left, fallback_expression, update_expression, true); + _extract_paths(paths, inserts, param.left, fallback_expression, update_expression, true); } break; } } - return assignments; + return paths; } /** diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 226d79a65f..62cb3e6513 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -154,7 +154,7 @@ export { } from './dom/operations.js'; export { attr, clsx } from '../shared/attributes.js'; export { snapshot } from '../shared/clone.js'; -export { noop, fallback } from '../shared/utils.js'; +export { noop, fallback, to_array } from '../shared/utils.js'; export { invalid_default_snippet, validate_dynamic_element_tag, diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index 29e09fe4dd..85c059e09b 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -504,7 +504,7 @@ export { assign_payload, copy_payload } from './payload.js'; export { snapshot } from '../shared/clone.js'; -export { fallback } from '../shared/utils.js'; +export { fallback, to_array } from '../shared/utils.js'; export { invalid_default_snippet, diff --git a/packages/svelte/src/internal/shared/utils.js b/packages/svelte/src/internal/shared/utils.js index 5e7f3152d8..10f8597520 100644 --- a/packages/svelte/src/internal/shared/utils.js +++ b/packages/svelte/src/internal/shared/utils.js @@ -81,3 +81,38 @@ export function fallback(value, fallback, lazy = false) { : /** @type {V} */ (fallback) : value; } + +/** + * When encountering a situation like `let [a, b, c] = $derived(blah())`, + * we need to stash an intermediate value that `a`, `b`, and `c` derive + * from, in case it's an iterable + * @template T + * @param {ArrayLike | Iterable} value + * @param {number} [n] + * @returns {Array} + */ +export function to_array(value, n) { + // return arrays unchanged + if (Array.isArray(value)) { + return value; + } + + // if value is not iterable, or `n` is unspecified (indicates a rest + // element, which means we're not concerned about unbounded iterables) + // convert to an array with `Array.from` + if (n === undefined || !(Symbol.iterator in value)) { + return Array.from(value); + } + + // otherwise, populate an array with `n` values + + /** @type {T[]} */ + const array = []; + + for (const element of value) { + array.push(element); + if (array.length === n) break; + } + + return array; +} diff --git a/packages/svelte/tests/runtime-runes/samples/derived-destructured-iterator/_config.js b/packages/svelte/tests/runtime-runes/samples/derived-destructured-iterator/_config.js new file mode 100644 index 0000000000..7f8d1e000d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/derived-destructured-iterator/_config.js @@ -0,0 +1,16 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + html: `

a: 1

b: 2

c: 3

`, + + test({ assert, target }) { + const button = target.querySelector('button'); + + flushSync(() => button?.click()); + assert.htmlEqual( + target.innerHTML, + `

a: 2

b: 3

c: 4

` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/derived-destructured-iterator/main.svelte b/packages/svelte/tests/runtime-runes/samples/derived-destructured-iterator/main.svelte new file mode 100644 index 0000000000..8c8629a72c --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/derived-destructured-iterator/main.svelte @@ -0,0 +1,16 @@ + + + + +

a: {a}

+

b: {b}

+

c: {c}

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 b2ef29ccaf..e9cf5b573d 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,12 +7,12 @@ let c = 3; let d = 4; export function update(array) { - { - let [$$1, $$2] = array; + ((array) => { + var $$array = $.to_array(array, 2); - $.set(a, $$1, true); - $.set(b, $$2, true); - }; + $.set(a, $$array[0], true); + $.set(b, $$array[1], true); + })(array); [c, d] = array; } \ No newline at end of file From b8d15135cffd395beabb27ca7efd8ab402cf1048 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 28 May 2025 17:33:59 -0400 Subject: [PATCH 02/17] fix: stable attachments (#15961) * fix: stable attachments * WIP * WIP * WIP * WIP * WIP * WIP * fix * unused * unused * unused * changeset * ugh * remove nonsensical comment, this is unused in spread * rename * Update packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> --------- Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> --- .changeset/soft-jobs-sip.md | 5 ++ .../client/visitors/RegularElement.js | 49 +++--------- .../client/visitors/SvelteElement.js | 13 +-- .../client/visitors/shared/element.js | 79 ++++++++----------- .../client/visitors/shared/utils.js | 11 +-- .../client/dom/elements/attributes.js | 68 +++++++++++++++- .../client/dom/elements/bindings/select.js | 18 ++--- packages/svelte/src/internal/client/index.js | 1 + .../attachment-spread-stable/Component.svelte | 5 ++ .../attachment-spread-stable/_config.js | 16 ++++ .../attachment-spread-stable/main.svelte | 17 ++++ 11 files changed, 173 insertions(+), 109 deletions(-) create mode 100644 .changeset/soft-jobs-sip.md create mode 100644 packages/svelte/tests/runtime-runes/samples/attachment-spread-stable/Component.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/attachment-spread-stable/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/attachment-spread-stable/main.svelte diff --git a/.changeset/soft-jobs-sip.md b/.changeset/soft-jobs-sip.md new file mode 100644 index 0000000000..fcee398d45 --- /dev/null +++ b/.changeset/soft-jobs-sip.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: avoid rerunning attachments when unrelated spread attributes change diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index c1806b5128..e823792993 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -17,7 +17,7 @@ import { build_getter } from '../utils.js'; import { get_attribute_name, build_attribute_value, - build_set_attributes, + build_attribute_effect, build_set_class, build_set_style } from './shared/element.js'; @@ -201,37 +201,7 @@ export function RegularElement(node, context) { const node_id = context.state.node; if (has_spread) { - const attributes_id = b.id(context.state.scope.generate('attributes')); - - build_set_attributes( - attributes, - class_directives, - style_directives, - context, - node, - node_id, - attributes_id - ); - - // If value binding exists, that one takes care of calling $.init_select - if (node.name === 'select' && !bindings.has('value')) { - context.state.init.push( - b.stmt(b.call('$.init_select', node_id, b.thunk(b.member(attributes_id, 'value')))) - ); - - context.state.update.push( - b.if( - b.binary('in', b.literal('value'), attributes_id), - b.block([ - // This ensures a one-way street to the DOM in case it's . We need it in addition to $.init_select - // because the select value is not reflected as an attribute, so the - // mutation observer wouldn't notice. - b.stmt(b.call('$.select_option', node_id, b.member(attributes_id, 'value'))) - ]) - ) - ); - } + build_attribute_effect(attributes, class_directives, style_directives, context, node, node_id); } else { /** If true, needs `__value` for inputs */ const needs_special_value_handling = @@ -290,7 +260,8 @@ export function RegularElement(node, context) { const { value, has_state } = build_attribute_value( attribute.value, context, - (value, metadata) => (metadata.has_call ? get_expression_id(context.state, value) : value) + (value, metadata) => + metadata.has_call ? get_expression_id(context.state.expressions, value) : value ); const update = build_element_attribute_update(node, node_id, name, value, attributes); @@ -482,10 +453,11 @@ function setup_select_synchronization(value_binding, context) { /** * @param {AST.ClassDirective[]} class_directives + * @param {Expression[]} expressions * @param {ComponentContext} context * @return {ObjectExpression | Identifier} */ -export function build_class_directives_object(class_directives, context) { +export function build_class_directives_object(class_directives, expressions, context) { let properties = []; let has_call_or_state = false; @@ -497,15 +469,16 @@ export function build_class_directives_object(class_directives, context) { const directives = b.object(properties); - return has_call_or_state ? get_expression_id(context.state, directives) : directives; + return has_call_or_state ? get_expression_id(expressions, directives) : directives; } /** * @param {AST.StyleDirective[]} style_directives + * @param {Expression[]} expressions * @param {ComponentContext} context * @return {ObjectExpression | ArrayExpression}} */ -export function build_style_directives_object(style_directives, context) { +export function build_style_directives_object(style_directives, expressions, context) { let normal_properties = []; let important_properties = []; @@ -514,7 +487,7 @@ export function build_style_directives_object(style_directives, context) { directive.value === true ? build_getter({ name: directive.name, type: 'Identifier' }, context.state) : build_attribute_value(directive.value, context, (value, metadata) => - metadata.has_call ? get_expression_id(context.state, value) : value + metadata.has_call ? get_expression_id(expressions, value) : value ).value; const property = b.init(directive.name, expression); @@ -653,7 +626,7 @@ function build_element_special_value_attribute(element, node_id, attribute, cont ? // if is a select with value we will also invoke `init_select` which need a reference before the template effect so we memoize separately is_select_with_value ? memoize_expression(state, value) - : get_expression_id(state, value) + : get_expression_id(state.expressions, value) : value ); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js index 7e97035e00..7381553dbe 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js @@ -5,7 +5,11 @@ import { dev, locator } from '../../../../state.js'; import { is_text_attribute } from '../../../../utils/ast.js'; import * as b from '#compiler/builders'; import { determine_namespace_for_children } from '../../utils.js'; -import { build_attribute_value, build_set_attributes, build_set_class } from './shared/element.js'; +import { + build_attribute_value, + build_attribute_effect, + build_set_class +} from './shared/element.js'; import { build_render_statement } from './shared/utils.js'; /** @@ -80,18 +84,15 @@ export function SvelteElement(node, context) { ) { build_set_class(node, element_id, attributes[0], class_directives, inner_context, false); } else if (attributes.length) { - const attributes_id = b.id(context.state.scope.generate('attributes')); - // Always use spread because we don't know whether the element is a custom element or not, // therefore we need to do the "how to set an attribute" logic at runtime. - build_set_attributes( + build_attribute_effect( attributes, class_directives, style_directives, inner_context, node, - element_id, - attributes_id + element_id ); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js index a093a0bf4a..67de25b770 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js @@ -16,28 +16,30 @@ import { build_template_chunk, get_expression_id } from './utils.js'; * @param {ComponentContext} context * @param {AST.RegularElement | AST.SvelteElement} element * @param {Identifier} element_id - * @param {Identifier} attributes_id */ -export function build_set_attributes( +export function build_attribute_effect( attributes, class_directives, style_directives, context, element, - element_id, - attributes_id + element_id ) { - let is_dynamic = false; - /** @type {ObjectExpression['properties']} */ const values = []; + /** @type {Expression[]} */ + const expressions = []; + + /** @param {Expression} value */ + function memoize(value) { + return b.id(`$${expressions.push(value) - 1}`); + } + for (const attribute of attributes) { if (attribute.type === 'Attribute') { - const { value, has_state } = build_attribute_value( - attribute.value, - context, - (value, metadata) => (metadata.has_call ? get_expression_id(context.state, value) : value) + const { value } = build_attribute_value(attribute.value, context, (value, metadata) => + metadata.has_call ? memoize(value) : value ); if ( @@ -51,16 +53,11 @@ export function build_set_attributes( } else { values.push(b.init(attribute.name, value)); } - - is_dynamic ||= has_state; } else { - // objects could contain reactive getters -> play it safe and always assume spread attributes are reactive - is_dynamic = true; - let value = /** @type {Expression} */ (context.visit(attribute)); if (attribute.metadata.expression.has_call) { - value = get_expression_id(context.state, value); + value = memoize(value); } values.push(b.spread(value)); @@ -72,12 +69,9 @@ export function build_set_attributes( b.prop( 'init', b.array([b.id('$.CLASS')]), - build_class_directives_object(class_directives, context) + build_class_directives_object(class_directives, expressions, context) ) ); - - is_dynamic ||= - class_directives.find((directive) => directive.metadata.expression.has_state) !== null; } if (style_directives.length) { @@ -85,31 +79,28 @@ export function build_set_attributes( b.prop( 'init', b.array([b.id('$.STYLE')]), - build_style_directives_object(style_directives, context) + build_style_directives_object(style_directives, expressions, context) ) ); - - is_dynamic ||= style_directives.some((directive) => directive.metadata.expression.has_state); } - const call = b.call( - '$.set_attributes', - element_id, - is_dynamic ? attributes_id : b.null, - b.object(values), - element.metadata.scoped && - context.state.analysis.css.hash !== '' && - b.literal(context.state.analysis.css.hash), - is_ignored(element, 'hydration_attribute_changed') && b.true + context.state.init.push( + b.stmt( + b.call( + '$.attribute_effect', + element_id, + b.arrow( + expressions.map((_, i) => b.id(`$${i}`)), + b.object(values) + ), + expressions.length > 0 && b.array(expressions.map((expression) => b.thunk(expression))), + element.metadata.scoped && + context.state.analysis.css.hash !== '' && + b.literal(context.state.analysis.css.hash), + is_ignored(element, 'hydration_attribute_changed') && b.true + ) + ) ); - - if (is_dynamic) { - context.state.init.push(b.let(attributes_id)); - const update = b.stmt(b.assignment('=', attributes_id, call)); - context.state.update.push(update); - } else { - context.state.init.push(b.stmt(call)); - } } /** @@ -167,7 +158,7 @@ export function build_set_class(element, node_id, attribute, class_directives, c value = b.call('$.clsx', value); } - return metadata.has_call ? get_expression_id(context.state, value) : value; + return metadata.has_call ? get_expression_id(context.state.expressions, value) : value; }); /** @type {Identifier | undefined} */ @@ -180,7 +171,7 @@ export function build_set_class(element, node_id, attribute, class_directives, c let next; if (class_directives.length) { - next = build_class_directives_object(class_directives, context); + next = build_class_directives_object(class_directives, context.state.expressions, context); has_state ||= class_directives.some((d) => d.metadata.expression.has_state); if (has_state) { @@ -235,7 +226,7 @@ export function build_set_class(element, node_id, attribute, class_directives, c */ export function build_set_style(node_id, attribute, style_directives, context) { let { value, has_state } = build_attribute_value(attribute.value, context, (value, metadata) => - metadata.has_call ? get_expression_id(context.state, value) : value + metadata.has_call ? get_expression_id(context.state.expressions, value) : value ); /** @type {Identifier | undefined} */ @@ -248,7 +239,7 @@ export function build_set_style(node_id, attribute, style_directives, context) { let next; if (style_directives.length) { - next = build_style_directives_object(style_directives, context); + next = build_style_directives_object(style_directives, context.state.expressions, context); has_state ||= style_directives.some((d) => d.metadata.expression.has_state); if (has_state) { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js index 80b472ac37..ebf88e878f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js @@ -21,12 +21,12 @@ export function memoize_expression(state, value) { } /** - * - * @param {ComponentClientTransformState} state + * Pushes `value` into `expressions` and returns a new id + * @param {Expression[]} expressions * @param {Expression} value */ -export function get_expression_id(state, value) { - return b.id(`$${state.expressions.push(value) - 1}`); +export function get_expression_id(expressions, value) { + return b.id(`$${expressions.push(value) - 1}`); } /** @@ -40,7 +40,8 @@ export function build_template_chunk( values, visit, state, - memoize = (value, metadata) => (metadata.has_call ? get_expression_id(state, value) : value) + memoize = (value, metadata) => + metadata.has_call ? get_expression_id(state.expressions, value) : value ) { /** @type {Expression[]} */ const expressions = []; diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index 3d1acbd31c..6cfc123cc2 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -1,3 +1,4 @@ +/** @import { Effect } from '#client' */ import { DEV } from 'esm-env'; import { hydrating, set_hydrating } from '../hydration.js'; import { get_descriptors, get_prototype_of } from '../../../shared/utils.js'; @@ -10,6 +11,7 @@ import { is_capture_event, is_delegated, normalize_attribute } from '../../../.. import { active_effect, active_reaction, + get, set_active_effect, set_active_reaction } from '../../runtime.js'; @@ -18,6 +20,9 @@ import { clsx } from '../../../shared/attributes.js'; import { set_class } from './class.js'; import { set_style } from './style.js'; import { ATTACHMENT_KEY, NAMESPACE_HTML } from '../../../../constants.js'; +import { block, branch, destroy_effect } from '../../reactivity/effects.js'; +import { derived } from '../../reactivity/deriveds.js'; +import { init_select, select_option } from './bindings/select.js'; export const CLASS = Symbol('class'); export const STYLE = Symbol('style'); @@ -447,13 +452,68 @@ export function set_attributes(element, prev, next, css_hash, skip_warning = fal set_hydrating(true); } - for (let symbol of Object.getOwnPropertySymbols(next)) { - if (symbol.description === ATTACHMENT_KEY) { - attach(element, () => next[symbol]); + return current; +} + +/** + * @param {Element & ElementCSSInlineStyle} element + * @param {(...expressions: any) => Record} fn + * @param {Array<() => any>} thunks + * @param {string} [css_hash] + * @param {boolean} [skip_warning] + */ +export function attribute_effect( + element, + fn, + thunks = [], + css_hash, + skip_warning = false, + d = derived +) { + const deriveds = thunks.map(d); + + /** @type {Record | undefined} */ + var prev = undefined; + + /** @type {Record} */ + var effects = {}; + + var is_select = element.nodeName === 'SELECT'; + var inited = false; + + block(() => { + var next = fn(...deriveds.map(get)); + + set_attributes(element, prev, next, css_hash, skip_warning); + + if (inited && is_select) { + select_option(/** @type {HTMLSelectElement} */ (element), next.value, false); + } + + for (let symbol of Object.getOwnPropertySymbols(effects)) { + if (!next[symbol]) destroy_effect(effects[symbol]); } + + for (let symbol of Object.getOwnPropertySymbols(next)) { + var n = next[symbol]; + + if (symbol.description === ATTACHMENT_KEY && (!prev || n !== prev[symbol])) { + if (effects[symbol]) destroy_effect(effects[symbol]); + effects[symbol] = branch(() => attach(element, () => n)); + } + } + + prev = next; + }); + + if (is_select) { + init_select( + /** @type {HTMLSelectElement} */ (element), + () => /** @type {Record} */ (prev).value + ); } - return current; + inited = true; } /** diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/select.js b/packages/svelte/src/internal/client/dom/elements/bindings/select.js index 20f95af5ec..e3263c65af 100644 --- a/packages/svelte/src/internal/client/dom/elements/bindings/select.js +++ b/packages/svelte/src/internal/client/dom/elements/bindings/select.js @@ -25,10 +25,14 @@ export function select_option(select, value, mounting) { } // Otherwise, update the selection - return select_options(select, value); + for (var option of select.options) { + option.selected = value.includes(get_option_value(option)); + } + + return; } - for (var option of select.options) { + for (option of select.options) { var option_value = get_option_value(option); if (is(option_value, value)) { option.selected = true; @@ -136,16 +140,6 @@ export function bind_select_value(select, get, set = get) { init_select(select); } -/** - * @param {HTMLSelectElement} select - * @param {unknown[]} value - */ -function select_options(select, value) { - for (var option of select.options) { - option.selected = value.includes(get_option_value(option)); - } -} - /** @param {HTMLOptionElement} option */ function get_option_value(option) { // __value only exists if the