diff --git a/.changeset/nine-ants-invite.md b/.changeset/nine-ants-invite.md new file mode 100644 index 0000000000..fdf33c27a9 --- /dev/null +++ b/.changeset/nine-ants-invite.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: invalidate signals following ++/-- inside each block diff --git a/.changeset/perfect-hairs-matter.md b/.changeset/perfect-hairs-matter.md new file mode 100644 index 0000000000..8ec02f7507 --- /dev/null +++ b/.changeset/perfect-hairs-matter.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +feat: better code generation for destructuring assignments diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js index 05ba0d486f..102091c78d 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js @@ -187,9 +187,7 @@ function get_delegated_event(event_name, handler, context) { // Bail out if we reference anything from the EachBlock (for now) that mutates in non-runes mode, (((!context.state.analysis.runes && binding.kind === 'each') || // or any normal not reactive bindings that are mutated. - binding.kind === 'normal' || - // or any reactive imports (those are rewritten) (can only happen in legacy mode) - binding.kind === 'legacy_reactive_import') && + binding.kind === 'normal') && binding.mutated)) ) { return unhoisted; diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/EachBlock.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/EachBlock.js index 2e09c756e3..ebf20d5aa6 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/EachBlock.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/EachBlock.js @@ -1,5 +1,6 @@ /** @import { EachBlock } from '#compiler' */ /** @import { Context } from '../types' */ +/** @import { Scope } from '../../scope' */ import * as e from '../../../errors.js'; import { validate_block_not_empty, validate_opening_tag } from './shared/utils.js'; @@ -25,5 +26,13 @@ export function EachBlock(node, context) { node.key.type !== 'Identifier' || !node.index || node.key.name !== node.index; } - context.next(); + // evaluate expression in parent scope + context.visit(node.expression, { + ...context.state, + scope: /** @type {Scope} */ (context.state.scope.parent) + }); + + context.visit(node.body); + if (node.key) context.visit(node.key); + if (node.fallback) context.visit(node.fallback); } diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js index c00d521610..2e4595ec08 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js @@ -80,16 +80,8 @@ export function Identifier(node, context) { binding.declaration_kind !== 'function') || binding.declaration_kind === 'import') ) { - if (binding.declaration_kind === 'import') { - if ( - binding.mutated && - // TODO could be more fine-grained - not every mention in the template implies a state binding - (context.state.reactive_statement || context.state.ast_type === 'template') && - parent.type === 'MemberExpression' - ) { - binding.kind = 'legacy_reactive_import'; - } - } else if ( + if ( + binding.declaration_kind !== 'import' && binding.mutated && // TODO could be more fine-grained - not every mention in the template implies a state binding (context.state.reactive_statement || context.state.ast_type === 'template') diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/component.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/component.js index 347a59488a..e6bb3c067e 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/component.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/component.js @@ -1,7 +1,8 @@ -/** @import { Component, SvelteComponent, SvelteSelf } from '#compiler' */ +/** @import { Comment, Component, Fragment, SvelteComponent, SvelteSelf } from '#compiler' */ /** @import { Context } from '../../types' */ import * as e from '../../../../errors.js'; import { get_attribute_expression, is_expression_attribute } from '../../../../utils/ast.js'; +import { determine_slot } from '../../../../utils/slot.js'; import { validate_attribute, validate_attribute_name, @@ -60,9 +61,45 @@ export function visit_component(node, context) { } } - context.next({ - ...context.state, - parent_element: null, - component_slots: new Set() - }); + // If the component has a slot attribute — `<Foo slot="whatever" .../>` — + // then `let:` directives apply to other attributes, instead of just the + // top-level contents of the component. Yes, this is very weird. + const default_state = determine_slot(node) + ? context.state + : { ...context.state, scope: node.metadata.scopes.default }; + + for (const attribute of node.attributes) { + context.visit(attribute, attribute.type === 'LetDirective' ? default_state : context.state); + } + + /** @type {Comment[]} */ + let comments = []; + + /** @type {Record<string, Fragment['nodes']>} */ + const nodes = { default: [] }; + + for (const child of node.fragment.nodes) { + if (child.type === 'Comment') { + comments.push(child); + continue; + } + + const slot_name = determine_slot(child) ?? 'default'; + (nodes[slot_name] ??= []).push(...comments, child); + + if (slot_name !== 'default') comments = []; + } + + const component_slots = new Set(); + + for (const slot_name in nodes) { + const state = { + ...context.state, + scope: node.metadata.scopes[slot_name], + parent_element: null, + component_slots + }; + + context.visit({ ...node.fragment, nodes: nodes[slot_name] }, state); + } } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js index 6ebb77f2e9..c4af60f24b 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/transform-client.js @@ -4,7 +4,6 @@ /** @import { Visitors, ComponentClientTransformState, ClientTransformState } from './types' */ import { walk } from 'zimmerframe'; import * as b from '../../../utils/builders.js'; -import { set_scope } from '../../scope.js'; import { build_getter } from './utils.js'; import { render_stylesheet } from '../css/index.js'; import { dev, filename } from '../../../state.js'; @@ -15,6 +14,7 @@ import { Attribute } from './visitors/Attribute.js'; import { AwaitBlock } from './visitors/AwaitBlock.js'; import { BinaryExpression } from './visitors/BinaryExpression.js'; import { BindDirective } from './visitors/BindDirective.js'; +import { BlockStatement } from './visitors/BlockStatement.js'; import { BreakStatement } from './visitors/BreakStatement.js'; import { CallExpression } from './visitors/CallExpression.js'; import { ClassBody } from './visitors/ClassBody.js'; @@ -37,6 +37,7 @@ import { LabeledStatement } from './visitors/LabeledStatement.js'; import { LetDirective } from './visitors/LetDirective.js'; import { MemberExpression } from './visitors/MemberExpression.js'; import { OnDirective } from './visitors/OnDirective.js'; +import { Program } from './visitors/Program.js'; import { RegularElement } from './visitors/RegularElement.js'; import { RenderTag } from './visitors/RenderTag.js'; import { SlotElement } from './visitors/SlotElement.js'; @@ -58,7 +59,23 @@ import { VariableDeclaration } from './visitors/VariableDeclaration.js'; /** @type {Visitors} */ const visitors = { - _: set_scope, + _: function set_scope(node, { next, state }) { + const scope = state.scopes.get(node); + + if (scope && scope !== state.scope) { + const transform = { ...state.transform }; + + for (const [name, binding] of scope.declarations) { + if (binding.kind === 'normal') { + delete transform[name]; + } + } + + next({ ...state, transform, scope }); + } else { + next(); + } + }, AnimateDirective, ArrowFunctionExpression, AssignmentExpression, @@ -66,6 +83,7 @@ const visitors = { AwaitBlock, BinaryExpression, BindDirective, + BlockStatement, BreakStatement, CallExpression, ClassBody, @@ -88,6 +106,7 @@ const visitors = { LetDirective, MemberExpression, OnDirective, + Program, RegularElement, RenderTag, SlotElement, @@ -123,6 +142,7 @@ export function client_component(analysis, options) { is_instance: false, hoisted: [b.import_all('$', 'svelte/internal/client')], node: /** @type {any} */ (null), // populated by the root node + legacy_reactive_imports: [], legacy_reactive_statements: new Map(), metadata: { context: { @@ -136,8 +156,7 @@ export function client_component(analysis, options) { preserve_whitespace: options.preserveWhitespace, public_state: new Map(), private_state: new Map(), - getters: {}, - setters: {}, + transform: {}, in_constructor: false, // these are set inside the `Fragment` visitor, and cannot be used until then @@ -155,6 +174,7 @@ export function client_component(analysis, options) { const instance_state = { ...state, + transform: { ...state.transform }, scope: analysis.instance.scope, scopes: analysis.instance.scopes, is_instance: true @@ -167,21 +187,17 @@ export function client_component(analysis, options) { const template = /** @type {ESTree.Program} */ ( walk( /** @type {SvelteNode} */ (analysis.template.ast), - { ...state, scope: analysis.instance.scope, scopes: analysis.template.scopes }, + { + ...state, + transform: instance_state.transform, + scope: analysis.instance.scope, + scopes: analysis.template.scopes + }, visitors ) ); - // Very very dirty way of making import statements reactive in legacy mode if needed - if (!analysis.runes) { - for (const [name, binding] of analysis.module.scope.declarations) { - if (binding.kind === 'legacy_reactive_import') { - instance.body.unshift( - b.var('$$_import_' + name, b.call('$.reactive_import', b.thunk(b.id(name)))) - ); - } - } - } + instance.body.unshift(...state.legacy_reactive_imports); /** @type {ESTree.Statement[]} */ const store_setup = []; @@ -623,11 +639,9 @@ export function client_module(analysis, options) { options, scope: analysis.module.scope, scopes: analysis.module.scopes, - legacy_reactive_statements: new Map(), public_state: new Map(), private_state: new Map(), - getters: {}, - setters: {}, + transform: {}, in_constructor: false }; diff --git a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts index 0284c82d1e..cae1a10940 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts +++ b/packages/svelte/src/compiler/phases/3-transform/client/types.d.ts @@ -5,7 +5,8 @@ import type { Identifier, PrivateIdentifier, Expression, - AssignmentExpression + AssignmentExpression, + UpdateExpression } from 'estree'; import type { Namespace, SvelteNode, ValidatedCompileOptions } from '#compiler'; import type { TransformState } from '../types.js'; @@ -22,19 +23,18 @@ export interface ClientTransformState extends TransformState { */ readonly in_constructor: boolean; - /** The $: calls, which will be ordered in the end */ - readonly legacy_reactive_statements: Map<LabeledStatement, Statement>; - /** - * A map of `[name, node]` pairs, where `Identifier` nodes matching `name` - * will be replaced with `node` (e.g. `x` -> `$.get(x)`) - */ - readonly getters: Record<string, Expression | ((id: Identifier) => Expression)>; - /** - * Counterpart to `getters` - */ - readonly setters: Record< + readonly transform: Record< string, - (assignment: AssignmentExpression, context: Context) => Expression + { + /** turn `foo` into e.g. `$.get(foo)` */ + read: (id: Identifier) => Expression; + /** turn `foo = bar` into e.g. `$.set(foo, bar)` */ + assign?: (node: Identifier, value: Expression) => Expression; + /** turn `foo.bar = baz` into e.g. `$.mutate(foo, $.get(foo).bar = baz);` */ + mutate?: (node: Identifier, mutation: AssignmentExpression) => Expression; + /** turn `foo++` into e.g. `$.update(foo)` */ + update?: (node: UpdateExpression) => Expression; + } >; } @@ -79,6 +79,12 @@ export interface ComponentClientTransformState extends ClientTransformState { /** The anchor node for the current context */ readonly node: Identifier; + + /** Imports that should be re-evaluated in legacy mode following a mutation */ + readonly legacy_reactive_imports: Statement[]; + + /** The $: calls, which will be ordered in the end */ + readonly legacy_reactive_statements: Map<LabeledStatement, Statement>; } export interface StateField { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/utils.js index 57ede67bb3..d5e6420d5d 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -1,59 +1,17 @@ -/** @import { ArrowFunctionExpression, AssignmentExpression, BinaryOperator, Expression, FunctionDeclaration, FunctionExpression, Identifier, MemberExpression, Node, Pattern, PrivateIdentifier, Statement } from 'estree' */ +/** @import { ArrowFunctionExpression, Expression, FunctionDeclaration, FunctionExpression, Identifier, Pattern, PrivateIdentifier, Statement } from 'estree' */ /** @import { Binding, SvelteNode } from '#compiler' */ /** @import { ClientTransformState, ComponentClientTransformState, ComponentContext } from './types.js' */ /** @import { Scope } from '../../scope.js' */ import * as b from '../../../utils/builders.js'; -import { - extract_identifiers, - extract_paths, - is_expression_async, - is_simple_expression, - object -} from '../../../utils/ast.js'; +import { extract_identifiers, is_simple_expression } from '../../../utils/ast.js'; import { PROPS_IS_LAZY_INITIAL, PROPS_IS_IMMUTABLE, PROPS_IS_RUNES, PROPS_IS_UPDATED } from '../../../../constants.js'; -import { is_ignored, dev } from '../../../state.js'; - -/** - * @template {ClientTransformState} State - * @param {AssignmentExpression} node - * @param {import('zimmerframe').Context<SvelteNode, State>} context - * @returns - */ -export function get_assignment_value(node, { state, visit }) { - if (node.left.type === 'Identifier') { - const operator = node.operator; - return operator === '=' - ? /** @type {Expression} */ (visit(node.right)) - : // turn something like x += 1 into x = x + 1 - b.binary( - /** @type {BinaryOperator} */ (operator.slice(0, -1)), - build_getter(node.left, state), - /** @type {Expression} */ (visit(node.right)) - ); - } else if ( - node.left.type === 'MemberExpression' && - node.left.object.type === 'ThisExpression' && - node.left.property.type === 'PrivateIdentifier' && - state.private_state.has(node.left.property.name) - ) { - const operator = node.operator; - return operator === '=' - ? /** @type {Expression} */ (visit(node.right)) - : // turn something like x += 1 into x = x + 1 - b.binary( - /** @type {BinaryOperator} */ (operator.slice(0, -1)), - /** @type {Expression} */ (visit(node.left)), - /** @type {Expression} */ (visit(node.right)) - ); - } else { - return /** @type {Expression} */ (visit(node.right)); - } -} +import { dev } from '../../../state.js'; +import { get_value } from './visitors/shared/declarations.js'; /** * @param {Binding} binding @@ -73,392 +31,18 @@ export function is_state_source(binding, state) { * @returns {Expression} */ export function build_getter(node, state) { - const binding = state.scope.get(node.name); - - if (binding === null || node === binding.node) { - // No associated binding or the declaration itself which shouldn't be transformed - return node; - } + if (Object.hasOwn(state.transform, node.name)) { + const binding = state.scope.get(node.name); - if (Object.hasOwn(state.getters, node.name)) { - const getter = state.getters[node.name]; - return typeof getter === 'function' ? getter(node) : getter; - } - - if (binding.node.name === '$$props') { - // Special case for $$props which only exists in the old world - return b.id('$$sanitized_props'); - } - - if (binding.kind === 'store_sub') { - return b.call(node); - } - - if (binding.kind === 'prop' || binding.kind === 'bindable_prop') { - if (is_prop_source(binding, state)) { - return b.call(node); + // don't transform the declaration itself + if (node !== binding?.node) { + return state.transform[node.name].read(node); } - - if (binding.prop_alias) { - const key = b.key(binding.prop_alias); - return b.member(b.id('$$props'), key, key.type === 'Literal'); - } - return b.member(b.id('$$props'), node); - } - - if (binding.kind === 'legacy_reactive_import') { - return b.call('$$_import_' + node.name); - } - - if ( - is_state_source(binding, state) || - binding.kind === 'derived' || - binding.kind === 'legacy_reactive' - ) { - return b.call('$.get', node); } return node; } -/** - * @template {ClientTransformState} State - * @param {AssignmentExpression} node - * @param {import('zimmerframe').Context<SvelteNode, State>} context - * @param {() => any} fallback - * @param {boolean | null} [prefix] - If the assignment is a transformed update expression, set this. Else `null` - * @param {{skip_proxy_and_freeze?: boolean}} [options] - * @returns {Expression} - */ -export function build_setter(node, context, fallback, prefix, options) { - const { state, visit } = context; - - const assignee = node.left; - if ( - assignee.type === 'ArrayPattern' || - assignee.type === 'ObjectPattern' || - assignee.type === 'RestElement' - ) { - // Turn assignment into an IIFE, so that `$.set` calls etc don't produce invalid code - const tmp_id = context.state.scope.generate('tmp'); - - /** @type {AssignmentExpression[]} */ - const original_assignments = []; - - /** @type {Expression[]} */ - const assignments = []; - - const paths = extract_paths(assignee); - - for (const path of paths) { - const value = path.expression?.(b.id(tmp_id)); - const assignment = b.assignment('=', path.node, value); - original_assignments.push(assignment); - assignments.push(build_setter(assignment, context, () => assignment, prefix, options)); - } - - if (assignments.every((assignment, i) => assignment === original_assignments[i])) { - // No change to output -> nothing to transform -> we can keep the original assignment - return fallback(); - } - - const rhs_expression = /** @type {Expression} */ (visit(node.right)); - - const iife_is_async = - is_expression_async(rhs_expression) || - assignments.some((assignment) => is_expression_async(assignment)); - - const iife = b.arrow( - [], - b.block([ - b.const(tmp_id, rhs_expression), - b.stmt(b.sequence(assignments)), - // return because it could be used in a nested expression where the value is needed. - // example: { foo: ({ bar } = { bar: 1 })} - b.return(b.id(tmp_id)) - ]) - ); - - if (iife_is_async) { - return b.await(b.call(b.async(iife))); - } else { - return b.call(iife); - } - } - - if (assignee.type !== 'Identifier' && assignee.type !== 'MemberExpression') { - throw new Error(`Unexpected assignment type ${assignee.type}`); - } - - // Handle class private/public state assignment cases - if (assignee.type === 'MemberExpression') { - if ( - assignee.object.type === 'ThisExpression' && - assignee.property.type === 'PrivateIdentifier' - ) { - const private_state = context.state.private_state.get(assignee.property.name); - const value = get_assignment_value(node, context); - if (private_state !== undefined) { - if (state.in_constructor) { - // See if we should wrap value in $.proxy - if ( - context.state.analysis.runes && - !options?.skip_proxy_and_freeze && - should_proxy_or_freeze(value, context.state.scope) - ) { - const assignment = fallback(); - if (assignment.type === 'AssignmentExpression') { - assignment.right = - private_state.kind === 'frozen_state' - ? b.call('$.freeze', value) - : build_proxy_reassignment(value, private_state.id); - return assignment; - } - } - } else { - return b.call( - '$.set', - assignee, - context.state.analysis.runes && - !options?.skip_proxy_and_freeze && - should_proxy_or_freeze(value, context.state.scope) - ? private_state.kind === 'frozen_state' - ? b.call('$.freeze', value) - : build_proxy_reassignment(value, private_state.id) - : value - ); - } - } - } else if ( - assignee.object.type === 'ThisExpression' && - assignee.property.type === 'Identifier' && - state.in_constructor - ) { - const public_state = context.state.public_state.get(assignee.property.name); - const value = get_assignment_value(node, context); - // See if we should wrap value in $.proxy - if ( - context.state.analysis.runes && - public_state !== undefined && - !options?.skip_proxy_and_freeze && - should_proxy_or_freeze(value, context.state.scope) - ) { - const assignment = fallback(); - if (assignment.type === 'AssignmentExpression') { - assignment.right = - public_state.kind === 'frozen_state' - ? b.call('$.freeze', value) - : build_proxy_reassignment(value, public_state.id); - return assignment; - } - } - } - } - - const left = object(assignee); - - if (left === null) { - return fallback(); - } - - const binding = state.scope.get(left.name); - - if (!binding) return fallback(); - - if (Object.hasOwn(state.setters, left.name)) { - const setter = state.setters[left.name]; - // @ts-expect-error - return setter(node, context); - } - - if (binding.kind === 'legacy_reactive_import') { - return b.call( - '$$_import_' + binding.node.name, - b.assignment( - node.operator, - /** @type {Pattern} */ (visit(node.left)), - /** @type {Expression} */ (visit(node.right)) - ) - ); - } - - /** - * @param {any} serialized - * @returns - */ - function maybe_skip_ownership_validation(serialized) { - if (is_ignored(node, 'ownership_invalid_mutation')) { - return b.call('$.skip_ownership_validation', b.thunk(serialized)); - } - - return serialized; - } - - if (binding.kind === 'derived') { - return maybe_skip_ownership_validation(fallback()); - } - - const is_store = binding.kind === 'store_sub'; - const left_name = is_store ? left.name.slice(1) : left.name; - - if ( - binding.kind !== 'state' && - binding.kind !== 'frozen_state' && - binding.kind !== 'prop' && - binding.kind !== 'bindable_prop' && - binding.kind !== 'each' && - binding.kind !== 'legacy_reactive' && - !is_store - ) { - // TODO error if it's a computed (or rest prop)? or does that already happen elsewhere? - return fallback(); - } - - const value = get_assignment_value(node, context); - - const serialize = () => { - if (left === node.left) { - const is_initial_proxy = - binding.initial !== null && - should_proxy_or_freeze(/**@type {Expression}*/ (binding.initial), context.state.scope); - if ((binding.kind === 'prop' || binding.kind === 'bindable_prop') && !is_initial_proxy) { - return b.call(left, value); - } else if (is_store) { - return b.call('$.store_set', build_getter(b.id(left_name), state), value); - } else { - let call; - if (binding.kind === 'state') { - call = b.call( - '$.set', - b.id(left_name), - context.state.analysis.runes && - !options?.skip_proxy_and_freeze && - should_proxy_or_freeze(value, context.state.scope) - ? build_proxy_reassignment(value, left_name) - : value - ); - } else if (binding.kind === 'frozen_state') { - call = b.call( - '$.set', - b.id(left_name), - context.state.analysis.runes && - !options?.skip_proxy_and_freeze && - should_proxy_or_freeze(value, context.state.scope) - ? b.call('$.freeze', value) - : value - ); - } else if ( - (binding.kind === 'prop' || binding.kind === 'bindable_prop') && - is_initial_proxy - ) { - call = b.call( - left, - context.state.analysis.runes && - !options?.skip_proxy_and_freeze && - should_proxy_or_freeze(value, context.state.scope) && - binding.kind === 'bindable_prop' - ? build_proxy_reassignment(value, left_name) - : value - ); - } else { - call = b.call('$.set', b.id(left_name), value); - } - - if (state.scope.get(`$${left.name}`)?.kind === 'store_sub') { - return b.call('$.store_unsub', call, b.literal(`$${left.name}`), b.id('$$stores')); - } else { - return call; - } - } - } else { - if (is_store) { - // If we are assigning to a store property, we need to ensure we don't - // capture the read for the store as part of the member expression to - // keep consistency with how store $ shorthand reads work in Svelte 4. - /** - * - * @param {Expression | Pattern} node - * @returns {Expression} - */ - function visit_node(node) { - if (node.type === 'MemberExpression') { - return { - ...node, - object: visit_node(/** @type {Expression} */ (node.object)), - property: /** @type {MemberExpression} */ (visit(node)).property - }; - } - if (node.type === 'Identifier') { - const binding = state.scope.get(node.name); - - if (binding !== null && binding.kind === 'store_sub') { - return b.call('$.untrack', b.thunk(/** @type {Expression} */ (visit(node)))); - } - } - return /** @type {Expression} */ (visit(node)); - } - - return maybe_skip_ownership_validation( - b.call( - '$.store_mutate', - build_getter(b.id(left_name), state), - b.assignment(node.operator, /** @type {Pattern}} */ (visit_node(node.left)), value), - b.call('$.untrack', b.id('$' + left_name)) - ) - ); - } else if ( - !state.analysis.runes || - // this condition can go away once legacy mode is gone; only necessary for interop with legacy parent bindings - (binding.mutated && binding.kind === 'bindable_prop') - ) { - if (binding.kind === 'bindable_prop') { - return maybe_skip_ownership_validation( - b.call( - left, - b.assignment(node.operator, /** @type {Pattern} */ (visit(node.left)), value), - b.true - ) - ); - } else { - return maybe_skip_ownership_validation( - b.call( - '$.mutate', - b.id(left_name), - b.assignment(node.operator, /** @type {Pattern} */ (visit(node.left)), value) - ) - ); - } - } else if ( - node.right.type === 'Literal' && - prefix != null && - (node.operator === '+=' || node.operator === '-=') - ) { - return maybe_skip_ownership_validation( - b.update( - node.operator === '+=' ? '++' : '--', - /** @type {Expression} */ (visit(node.left)), - prefix - ) - ); - } else { - return maybe_skip_ownership_validation( - b.assignment( - node.operator, - /** @type {Pattern} */ (visit(node.left)), - /** @type {Expression} */ (visit(node.right)) - ) - ); - } - } - }; - - if (value.type === 'BinaryExpression' && /** @type {any} */ (value.operator) === '??') { - return b.logical('??', build_getter(b.id(left_name), state), serialize()); - } - - return serialize(); -} - /** * @param {Expression} value * @param {PrivateIdentifier | string} proxy_reference @@ -507,7 +91,7 @@ function get_hoisted_params(node, context) { binding = /** @type {Binding} */ (scope.get(binding.node.name.slice(1))); } - const expression = context.state.getters[reference]; + let expression = context.state.transform[reference]?.read(b.id(binding.node.name)); if ( // If it's a destructured derived binding, then we can extract the derived signal reference and use that. @@ -706,6 +290,7 @@ export function with_loc(target, source) { */ export function create_derived_block_argument(node, context) { if (node.type === 'Identifier') { + context.state.transform[node.name] = { read: get_value }; return { id: node, declarations: null }; } @@ -723,6 +308,8 @@ export function create_derived_block_argument(node, context) { const declarations = [b.var(value, create_derived(context.state, b.thunk(block)))]; for (const id of identifiers) { + context.state.transform[id.name] = { read: get_value }; + declarations.push( b.var(id, create_derived(context.state, b.thunk(b.member(b.call('$.get', value), id)))) ); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js index fefc11bf1f..afd2d50ba4 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js @@ -1,11 +1,129 @@ -/** @import { AssignmentExpression } from 'estree' */ -/** @import { Context } from '../types' */ -import { build_setter } from '../utils.js'; +/** @import { AssignmentExpression, AssignmentOperator, Expression, Pattern } from 'estree' */ +/** @import { Context } from '../types.js' */ +import * as b from '../../../../utils/builders.js'; +import { build_assignment_value } from '../../../../utils/ast.js'; +import { is_ignored } from '../../../../state.js'; +import { build_proxy_reassignment, should_proxy_or_freeze } from '../utils.js'; +import { visit_assignment_expression } from '../../shared/assignments.js'; /** * @param {AssignmentExpression} node * @param {Context} context */ export function AssignmentExpression(node, context) { - return build_setter(node, context, context.next); + return visit_assignment_expression(node, context, build_assignment); +} + +/** + * @param {AssignmentOperator} operator + * @param {Pattern} left + * @param {Expression} right + * @param {Context} context + * @returns {Expression | null} + */ +export function build_assignment(operator, left, right, context) { + // Handle class private/public state assignment cases + if ( + context.state.analysis.runes && + left.type === 'MemberExpression' && + left.object.type === 'ThisExpression' + ) { + if (left.property.type === 'PrivateIdentifier') { + const private_state = context.state.private_state.get(left.property.name); + + if (private_state !== undefined) { + let transformed = false; + let value = /** @type {Expression} */ ( + context.visit(build_assignment_value(operator, left, right)) + ); + + if (should_proxy_or_freeze(value, context.state.scope)) { + transformed = true; + value = + private_state.kind === 'frozen_state' + ? b.call('$.freeze', value) + : build_proxy_reassignment(value, private_state.id); + } + + if (!context.state.in_constructor) { + return b.call('$.set', left, value); + } else if (transformed) { + return b.assignment(operator, /** @type {Pattern} */ (context.visit(left)), value); + } + } + } else if (left.property.type === 'Identifier' && context.state.in_constructor) { + const public_state = context.state.public_state.get(left.property.name); + + if (public_state !== undefined && should_proxy_or_freeze(right, context.state.scope)) { + const value = /** @type {Expression} */ (context.visit(right)); + + return b.assignment( + operator, + /** @type {Pattern} */ (context.visit(left)), + public_state.kind === 'frozen_state' + ? b.call('$.freeze', value) + : build_proxy_reassignment(value, public_state.id) + ); + } + } + } + + let object = left; + + while (object.type === 'MemberExpression') { + // @ts-expect-error + object = object.object; + } + + if (object.type !== 'Identifier') { + return null; + } + + const binding = context.state.scope.get(object.name); + if (!binding) return null; + + const transform = Object.hasOwn(context.state.transform, object.name) + ? context.state.transform[object.name] + : null; + + // reassignment + if (object === left && transform?.assign) { + let value = /** @type {Expression} */ ( + context.visit(build_assignment_value(operator, left, right)) + ); + + // special case — if an element binding, we know it's a primitive + const path = context.path.map((node) => node.type); + const is_primitive = path.at(-1) === 'BindDirective' && path.at(-2) === 'RegularElement'; + + if ( + !is_primitive && + binding.kind !== 'prop' && + context.state.analysis.runes && + should_proxy_or_freeze(value, context.state.scope) + ) { + value = + binding.kind === 'frozen_state' + ? b.call('$.freeze', value) + : build_proxy_reassignment(value, object.name); + } + + return transform.assign(object, value); + } + + /** @type {Expression} */ + let mutation = b.assignment( + operator, + /** @type {Pattern} */ (context.visit(left)), + /** @type {Expression} */ (context.visit(right)) + ); + + // mutation + if (transform?.mutate) { + mutation = transform.mutate(object, mutation); + } + + return is_ignored(left, 'ownership_invalid_mutation') + ? b.call('$.skip_ownership_validation', b.thunk(mutation)) + : mutation; } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AwaitBlock.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AwaitBlock.js index 21f16f3449..f956a5cf30 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AwaitBlock.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AwaitBlock.js @@ -1,4 +1,4 @@ -/** @import { BlockStatement, Expression, Pattern } from 'estree' */ +/** @import { BlockStatement, Expression, Pattern, Statement } from 'estree' */ /** @import { AwaitBlock } from '#compiler' */ /** @import { ComponentContext } from '../types' */ import * as b from '../../../../utils/builders.js'; @@ -15,39 +15,29 @@ export function AwaitBlock(node, context) { let catch_block; if (node.then) { + const argument = node.value && create_derived_block_argument(node.value, context); + /** @type {Pattern[]} */ const args = [b.id('$$anchor')]; - const block = /** @type {BlockStatement} */ (context.visit(node.then)); - - if (node.value) { - const argument = create_derived_block_argument(node.value, context); + if (argument) args.push(argument.id); - args.push(argument.id); - - if (argument.declarations !== null) { - block.body.unshift(...argument.declarations); - } - } + const declarations = argument?.declarations ?? []; + const block = /** @type {BlockStatement} */ (context.visit(node.then)); - then_block = b.arrow(args, block); + then_block = b.arrow(args, b.block([...declarations, ...block.body])); } if (node.catch) { + const argument = node.error && create_derived_block_argument(node.error, context); + /** @type {Pattern[]} */ const args = [b.id('$$anchor')]; - const block = /** @type {BlockStatement} */ (context.visit(node.catch)); - - if (node.error) { - const argument = create_derived_block_argument(node.error, context); + if (argument) args.push(argument.id); - args.push(argument.id); - - if (argument.declarations !== null) { - block.body.unshift(...argument.declarations); - } - } + const declarations = argument?.declarations ?? []; + const block = /** @type {BlockStatement} */ (context.visit(node.catch)); - catch_block = b.arrow(args, block); + catch_block = b.arrow(args, b.block([...declarations, ...block.body])); } context.state.init.push( diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js index 8b9064d59a..81bafcc18a 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/BindDirective.js @@ -5,7 +5,6 @@ import { dev, is_ignored } from '../../../../state.js'; import { is_text_attribute } from '../../../../utils/ast.js'; import * as b from '../../../../utils/builders.js'; import { binding_properties } from '../../../bindings.js'; -import { build_setter } from '../utils.js'; import { build_attribute_value } from './shared/element.js'; import { build_bind_this, validate_binding } from './shared/utils.js'; @@ -38,18 +37,10 @@ export function BindDirective(node, context) { } const getter = b.thunk(/** @type {Expression} */ (context.visit(expression))); - const assignment = b.assignment('=', expression, b.id('$$value')); + const setter = b.arrow( [b.id('$$value')], - build_setter( - assignment, - context, - () => /** @type {Expression} */ (context.visit(assignment)), - null, - { - skip_proxy_and_freeze: true - } - ) + /** @type {Expression} */ (context.visit(b.assignment('=', expression, b.id('$$value')))) ); /** @type {CallExpression} */ diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/BlockStatement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/BlockStatement.js new file mode 100644 index 0000000000..502fbd471e --- /dev/null +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/BlockStatement.js @@ -0,0 +1,12 @@ +/** @import { BlockStatement } from 'estree' */ +/** @import { ComponentContext } from '../types' */ +import { add_state_transformers } from './shared/declarations.js'; + +/** + * @param {BlockStatement} node + * @param {ComponentContext} context + */ +export function BlockStatement(node, context) { + add_state_transformers(context); + context.next(); +} diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ConstTag.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ConstTag.js index 10623ce3f5..bfff38338b 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/ConstTag.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/ConstTag.js @@ -5,6 +5,7 @@ import { dev } from '../../../../state.js'; import { extract_identifiers } from '../../../../utils/ast.js'; import * as b from '../../../../utils/builders.js'; import { create_derived } from '../utils.js'; +import { get_value } from './shared/declarations.js'; /** * @param {ConstTag} node @@ -24,7 +25,7 @@ export function ConstTag(node, context) { ) ); - context.state.getters[declaration.id.name] = b.call('$.get', declaration.id); + context.state.transform[declaration.id.name] = { read: get_value }; // we need to eagerly evaluate the expression in order to hit any // 'Cannot access x before initialization' errors @@ -35,15 +36,15 @@ export function ConstTag(node, context) { const identifiers = extract_identifiers(declaration.id); const tmp = b.id(context.state.scope.generate('computed_const')); - const getters = { ...context.state.getters }; + const transform = { ...context.state.transform }; // Make all identifiers that are declared within the following computed regular // variables, as they are not signals in that context yet for (const node of identifiers) { - getters[node.name] = node; + delete transform[node.name]; } - const child_state = { ...context.state, getters }; + const child_state = { ...context.state, transform }; // TODO optimise the simple `{ x } = y` case — we can just return `y` // instead of destructuring it only to return a new object @@ -67,7 +68,9 @@ export function ConstTag(node, context) { } for (const node of identifiers) { - context.state.getters[node.name] = b.member(b.call('$.get', tmp), node); + context.state.transform[node.name] = { + read: (node) => b.member(b.call('$.get', tmp), node) + }; } } } 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 a82677817a..fa19509c06 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 @@ -1,6 +1,7 @@ -/** @import { AssignmentExpression, BlockStatement, Expression, Identifier, MemberExpression, Pattern, Statement } from 'estree' */ +/** @import { BlockStatement, Expression, Identifier, Pattern, Statement } from 'estree' */ /** @import { Binding, EachBlock } from '#compiler' */ -/** @import { ComponentContext, Context } from '../types' */ +/** @import { ComponentContext } from '../types' */ +/** @import { Scope } from '../../../scope' */ import { EACH_INDEX_REACTIVE, EACH_IS_ANIMATED, @@ -12,7 +13,8 @@ import { import { dev } from '../../../../state.js'; import { extract_paths, object } from '../../../../utils/ast.js'; import * as b from '../../../../utils/builders.js'; -import { get_assignment_value, build_getter, build_setter, with_loc } from '../utils.js'; +import { build_getter, with_loc } from '../utils.js'; +import { get_value } from './shared/declarations.js'; /** * @param {EachBlock} node @@ -20,7 +22,15 @@ import { get_assignment_value, build_getter, build_setter, with_loc } from '../u */ export function EachBlock(node, context) { const each_node_meta = node.metadata; - const collection = /** @type {Expression} */ (context.visit(node.expression)); + + // expression should be evaluated in the parent scope, not the scope + // created by the each block itself + const collection = /** @type {Expression} */ ( + context.visit(node.expression, { + ...context.state, + scope: /** @type {Scope} */ (context.state.scope.parent) + }) + ); if (!each_node_meta.is_controlled) { context.state.template.push('<!>'); @@ -110,54 +120,13 @@ export function EachBlock(node, context) { const child_state = { ...context.state, - getters: { ...context.state.getters }, - setters: { ...context.state.setters } + transform: { ...context.state.transform } }; /** The state used when generating the key function, if necessary */ const key_state = { ...context.state, - getters: { ...context.state.getters } - }; - - /** - * @param {Pattern} expression_for_id - * @returns {(assignment: AssignmentExpression, context: Context) => Expression} - */ - const create_mutation = (expression_for_id) => { - return (assignment, context) => { - if (assignment.left.type !== 'Identifier' && assignment.left.type !== 'MemberExpression') { - // build_setter turns other patterns into IIFEs and separates the assignments - // into separate expressions, at which point this is called again with an identifier or member expression - return build_setter(assignment, context, () => assignment); - } - - const left = object(assignment.left); - const value = get_assignment_value(assignment, context); - const invalidate = b.call( - '$.invalidate_inner_signals', - b.thunk(b.sequence(indirect_dependencies)) - ); - const invalidate_store = store_to_invalidate - ? b.call('$.invalidate_store', b.id('$$stores'), b.literal(store_to_invalidate)) - : undefined; - - const sequence = []; - if (!context.state.analysis.runes) sequence.push(invalidate); - if (invalidate_store) sequence.push(invalidate_store); - - if (left === assignment.left) { - const assign = b.assignment('=', expression_for_id, value); - sequence.unshift(assign); - return b.sequence(sequence); - } else { - const original_left = /** @type {MemberExpression} */ (assignment.left); - const left = /** @type {Pattern} */ (context.visit(original_left)); - const assign = b.assignment(assignment.operator, left, value); - sequence.unshift(assign); - return b.sequence(sequence); - } - }; + transform: { ...context.state.transform } }; // We need to generate a unique identifier in case there's a bind:group below @@ -170,37 +139,59 @@ export function EachBlock(node, context) { const item_with_loc = with_loc(item, id); return b.call('$.unwrap', item_with_loc); }; - child_state.getters[item.name] = getter; if (node.index) { - child_state.getters[node.index] = (id) => { - const index_with_loc = with_loc(index, id); - return (flags & EACH_INDEX_REACTIVE) === 0 ? index_with_loc : b.call('$.get', index_with_loc); + child_state.transform[node.index] = { + read: (id) => { + const index_with_loc = with_loc(index, id); + return (flags & EACH_INDEX_REACTIVE) === 0 + ? index_with_loc + : b.call('$.get', index_with_loc); + } }; - key_state.getters[node.index] = b.id(node.index); + delete key_state.transform[node.index]; } /** @type {Statement[]} */ const declarations = []; + const invalidate = b.call( + '$.invalidate_inner_signals', + b.thunk(b.sequence(indirect_dependencies)) + ); + + const invalidate_store = store_to_invalidate + ? b.call('$.invalidate_store', b.id('$$stores'), b.literal(store_to_invalidate)) + : undefined; + + /** @type {Expression[]} */ + const sequence = []; + if (!context.state.analysis.runes) sequence.push(invalidate); + if (invalidate_store) sequence.push(invalidate_store); + if (node.context.type === 'Identifier') { - child_state.setters[node.context.name] = create_mutation( - b.member( - each_node_meta.array_name ? b.call(each_node_meta.array_name) : collection, - index, - true - ) - ); + child_state.transform[node.context.name] = { + read: getter, + assign: (_, value) => { + const left = b.member( + each_node_meta.array_name ? b.call(each_node_meta.array_name) : collection, + index, + true + ); + + return b.sequence([b.assignment('=', left, value), ...sequence]); + }, + mutate: (_, mutation) => b.sequence([mutation, ...sequence]) + }; - key_state.getters[node.context.name] = node.context; + delete key_state.transform[node.context.name]; } else { const unwrapped = getter(binding.node); const paths = extract_paths(node.context); for (const path of paths) { const name = /** @type {Identifier} */ (path.node).name; - const binding = /** @type {Binding} */ (context.state.scope.get(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)) @@ -208,19 +199,26 @@ export function EachBlock(node, context) { declarations.push(b.let(path.node, needs_derived ? b.call('$.derived_safe_equal', fn) : fn)); - const getter = needs_derived ? b.call('$.get', b.id(name)) : b.call(name); - child_state.getters[name] = getter; - child_state.setters[name] = create_mutation( - /** @type {Pattern} */ (path.update_expression(unwrapped)) - ); + const read = needs_derived ? get_value : b.call; + + child_state.transform[name] = { + read, + assign: (node, value) => { + const left = /** @type {Pattern} */ (path.update_expression(unwrapped)); + return b.sequence([b.assignment('=', left, value), ...sequence]); + }, + mutate: (node, mutation) => { + return b.sequence([mutation, ...sequence]); + } + }; // we need to eagerly evaluate the expression in order to hit any // 'Cannot access x before initialization' errors if (dev) { - declarations.push(b.stmt(getter)); + declarations.push(b.stmt(read(b.id(name)))); } - key_state.getters[name] = path.node; + delete key_state.transform[name]; } } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js index 1696d9213e..e7312d15aa 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Fragment.js @@ -66,7 +66,7 @@ export function Fragment(node, context) { after_update: [], template: [], locations: [], - getters: { ...context.state.getters }, + transform: { ...context.state.transform }, metadata: { context: { template_needs_import_node: false, diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/LabeledStatement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/LabeledStatement.js index 262e44dfcb..87f56262a8 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/LabeledStatement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/LabeledStatement.js @@ -34,7 +34,7 @@ export function LabeledStatement(node, context) { const sequence = []; for (const binding of reactive_statement.dependencies) { - if (binding.kind === 'normal') continue; + if (binding.kind === 'normal' && binding.declaration_kind !== 'import') continue; const name = binding.node.name; let serialized = build_getter(b.id(name), context.state); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/LetDirective.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/LetDirective.js index 2423c6e50e..c455ea3219 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/LetDirective.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/LetDirective.js @@ -16,10 +16,9 @@ export function LetDirective(node, context) { const bindings = context.state.scope.get_bindings(node); for (const binding of bindings) { - context.state.getters[binding.node.name] = b.member( - b.call('$.get', b.id(name)), - b.id(binding.node.name) - ); + context.state.transform[binding.node.name] = { + read: (node) => b.member(b.call('$.get', b.id(name)), node) + }; } return b.const( @@ -42,8 +41,13 @@ export function LetDirective(node, context) { ) ); } else { + const name = node.expression === null ? node.name : node.expression.name; + context.state.transform[name] = { + read: (node) => b.call('$.get', node) + }; + return b.const( - node.expression === null ? node.name : node.expression.name, + name, create_derived(context.state, b.thunk(b.member(b.id('$$slotProps'), b.id(node.name)))) ); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Program.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Program.js new file mode 100644 index 0000000000..c222d74d6e --- /dev/null +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Program.js @@ -0,0 +1,123 @@ +/** @import { Expression, MemberExpression, Program } from 'estree' */ +/** @import { ComponentContext } from '../types' */ +import { build_getter, is_prop_source } from '../utils.js'; +import * as b from '../../../../utils/builders.js'; +import { add_state_transformers } from './shared/declarations.js'; + +/** + * @param {Program} _ + * @param {ComponentContext} context + */ +export function Program(_, context) { + if (!context.state.analysis.runes) { + context.state.transform['$$props'] = { + read: (node) => ({ ...node, name: '$$sanitized_props' }) + }; + + for (const [name, binding] of context.state.scope.declarations) { + if (binding.declaration_kind === 'import' && binding.mutated) { + const id = b.id('$$_import_' + name); + + context.state.transform[name] = { + read: (_) => b.call(id), + mutate: (_, mutation) => b.call(id, mutation) + }; + + context.state.legacy_reactive_imports.push( + b.var(id, b.call('$.reactive_import', b.thunk(b.id(name)))) + ); + } + } + } + + for (const [name, binding] of context.state.scope.declarations) { + if (binding.kind === 'store_sub') { + const store = /** @type {Expression} */ (context.visit(b.id(name.slice(1)))); + + context.state.transform[name] = { + read: b.call, + assign: (_, value) => b.call('$.store_set', store, value), + mutate: (node, mutation) => { + // We need to untrack the store read, for consistency with Svelte 4 + const untracked = b.call('$.untrack', node); + + /** + * + * @param {Expression} n + * @returns {Expression} + */ + function replace(n) { + if (n.type === 'MemberExpression') { + return { + ...n, + object: replace(/** @type {Expression} */ (n.object)), + property: n.property + }; + } + + return untracked; + } + + return b.call( + '$.store_mutate', + store, + b.assignment( + mutation.operator, + /** @type {MemberExpression} */ ( + replace(/** @type {MemberExpression} */ (mutation.left)) + ), + mutation.right + ), + untracked + ); + }, + update: (node) => { + return b.call( + node.prefix ? '$.update_pre_store' : '$.update_store', + build_getter(b.id(name.slice(1)), context.state), + b.call(node.argument), + node.operator === '--' && b.literal(-1) + ); + } + }; + } + + if (binding.kind === 'prop' || binding.kind === 'bindable_prop') { + if (is_prop_source(binding, context.state)) { + context.state.transform[name] = { + read: b.call, + assign: (node, value) => b.call(node, value), + mutate: (node, value) => { + if (binding.kind === 'bindable_prop') { + // only necessary for interop with legacy parent bindings + return b.call(node, value, b.true); + } + + return value; + }, + update: (node) => { + return b.call( + node.prefix ? '$.update_pre_prop' : '$.update_prop', + node.argument, + node.operator === '--' && b.literal(-1) + ); + } + }; + } else if (binding.prop_alias) { + const key = b.key(binding.prop_alias); + + context.state.transform[name] = { + read: (_) => b.member(b.id('$$props'), key, key.type === 'Literal') + }; + } else { + context.state.transform[name] = { + read: (node) => b.member(b.id('$$props'), node) + }; + } + } + } + + add_state_transformers(context); + + context.next(); +} 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 94864fecef..890713f22a 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 @@ -100,6 +100,13 @@ export function RegularElement(node, context) { metadata.context.template_needs_import_node = true; } + // visit let directives first, to set state + for (const attribute of node.attributes) { + if (attribute.type === 'LetDirective') { + lets.push(/** @type {ExpressionStatement} */ (context.visit(attribute))); + } + } + for (const attribute of node.attributes) { if (attribute.type === 'Attribute') { attributes.push(attribute); @@ -136,8 +143,6 @@ export function RegularElement(node, context) { class_directives.push(attribute); } else if (attribute.type === 'StyleDirective') { style_directives.push(attribute); - } else if (attribute.type === 'LetDirective') { - lets.push(/** @type {ExpressionStatement} */ (context.visit(attribute))); } else if (attribute.type === 'OnDirective') { const handler = /** @type {Expression} */ (context.visit(attribute)); const has_action_directive = node.attributes.find((a) => a.type === 'UseDirective'); @@ -145,7 +150,7 @@ export function RegularElement(node, context) { context.state.after_update.push( b.stmt(has_action_directive ? b.call('$.effect', b.thunk(handler)) : handler) ); - } else { + } else if (attribute.type !== 'LetDirective') { if (attribute.type === 'BindDirective') { if (attribute.name === 'group' || attribute.name === 'checked') { needs_special_value_handling = true; 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 cdb7455447..b62a58c8d7 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 @@ -4,6 +4,7 @@ import { dev } from '../../../../state.js'; import { extract_paths } from '../../../../utils/ast.js'; import * as b from '../../../../utils/builders.js'; +import { get_value } from './shared/declarations.js'; /** * @param {SnippetBlock} node @@ -20,8 +21,8 @@ export function SnippetBlock(node, context) { /** @type {Statement[]} */ const declarations = []; - const getters = { ...context.state.getters }; - const child_state = { ...context.state, getters }; + const transform = { ...context.state.transform }; + const child_state = { ...context.state, transform }; for (let i = 0; i < node.parameters.length; i++) { const argument = node.parameters[i]; @@ -35,7 +36,8 @@ export function SnippetBlock(node, context) { right: b.id('$.noop') }); - getters[argument.name] = b.call(argument); + transform[argument.name] = { read: b.call }; + continue; } @@ -53,12 +55,14 @@ export function SnippetBlock(node, context) { declarations.push(b.let(path.node, needs_derived ? b.call('$.derived_safe_equal', fn) : fn)); - getters[name] = needs_derived ? b.call('$.get', b.id(name)) : b.call(name); + transform[name] = { + read: needs_derived ? get_value : b.call + }; // we need to eagerly evaluate the expression in order to hit any // 'Cannot access x before initialization' errors if (dev) { - declarations.push(b.stmt(getters[name])); + declarations.push(b.stmt(transform[name].read(b.id(name)))); } } } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js index 5f8685eb3b..91383a5679 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/UpdateExpression.js @@ -1,8 +1,8 @@ -/** @import { Expression, Pattern, Statement, UpdateExpression } from 'estree' */ +/** @import { Expression, Node, Pattern, Statement, UpdateExpression } from 'estree' */ /** @import { Context } from '../types' */ import { is_ignored } from '../../../../state.js'; +import { object } from '../../../../utils/ast.js'; import * as b from '../../../../utils/builders.js'; -import { build_getter, build_setter } from '../utils.js'; /** * @param {UpdateExpression} node @@ -11,45 +11,6 @@ import { build_getter, build_setter } from '../utils.js'; export function UpdateExpression(node, context) { const argument = node.argument; - if (argument.type === 'Identifier') { - const binding = context.state.scope.get(argument.name); - const is_store = binding?.kind === 'store_sub'; - const name = is_store ? argument.name.slice(1) : argument.name; - - // use runtime functions for smaller output - if ( - binding?.kind === 'state' || - binding?.kind === 'frozen_state' || - binding?.kind === 'each' || - binding?.kind === 'legacy_reactive' || - binding?.kind === 'prop' || - binding?.kind === 'bindable_prop' || - is_store - ) { - /** @type {Expression[]} */ - const args = []; - - let fn = '$.update'; - if (node.prefix) fn += '_pre'; - - if (is_store) { - fn += '_store'; - args.push(build_getter(b.id(name), context.state), b.call('$' + name)); - } else { - if (binding.kind === 'prop' || binding.kind === 'bindable_prop') fn += '_prop'; - args.push(b.id(name)); - } - - if (node.operator === '--') { - args.push(b.literal(-1)); - } - - return b.call(fn, ...args); - } - - return context.next(); - } - if ( argument.type === 'MemberExpression' && argument.object.type === 'ThisExpression' && @@ -68,43 +29,41 @@ export function UpdateExpression(node, context) { return b.call(fn, ...args); } - /** @param {any} serialized */ - function maybe_skip_ownership_validation(serialized) { - if (is_ignored(node, 'ownership_invalid_mutation')) { - return b.call('$.skip_ownership_validation', b.thunk(serialized)); - } - - return serialized; + if (argument.type !== 'Identifier' && argument.type !== 'MemberExpression') { + throw new Error('An impossible state was reached'); } - // turn it into an IIFE assignment expression: i++ -> (() => { const $$value = i; i+=1; return $$value; }) - const assignment = b.assignment( - node.operator === '++' ? '+=' : '-=', - /** @type {Pattern} */ (argument), - b.literal(1) - ); + const left = object(argument); + if (left === null) return context.next(); - const serialized_assignment = build_setter(assignment, context, () => assignment, node.prefix); + if (left === argument) { + const transform = context.state.transform; + const update = transform[left.name]?.update; - const value = /** @type {Expression} */ (context.visit(argument)); - - if (serialized_assignment === assignment) { - // No change to output -> nothing to transform -> we can keep the original update expression - return maybe_skip_ownership_validation(context.next()); + if (update && Object.hasOwn(transform, left.name)) { + return update(node); + } } - if (context.state.analysis.runes) { - return maybe_skip_ownership_validation(serialized_assignment); - } + const assignment = /** @type {Expression} */ ( + context.visit( + b.assignment( + node.operator === '++' ? '+=' : '-=', + /** @type {Pattern} */ (argument), + b.literal(1) + ) + ) + ); - /** @type {Statement[]} */ - let statements; - if (node.prefix) { - statements = [b.stmt(serialized_assignment), b.return(value)]; - } else { - const tmp_id = context.state.scope.generate('$$value'); - statements = [b.const(tmp_id, value), b.stmt(serialized_assignment), b.return(b.id(tmp_id))]; - } + const parent = /** @type {Node} */ (context.path.at(-1)); + const is_standalone = parent.type === 'ExpressionStatement'; // TODO and possibly others, but not e.g. the `test` of a WhileStatement + + const update = + node.prefix || is_standalone + ? assignment + : b.binary(node.operator === '++' ? '-' : '+', assignment, b.literal(1)); - return maybe_skip_ownership_validation(b.call(b.thunk(b.block(statements)))); + return is_ignored(node, 'ownership_invalid_mutation') + ? b.call('$.skip_ownership_validation', b.thunk(update)) + : update; } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js index 6416e26318..44f943ddec 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js @@ -1,14 +1,14 @@ /** @import { BlockStatement, Expression, ExpressionStatement, Identifier, MemberExpression, Property, Statement } from 'estree' */ -/** @import { Attribute, Component, SvelteComponent, SvelteSelf, TemplateNode, Text } from '#compiler' */ +/** @import { Component, SvelteComponent, SvelteSelf, TemplateNode } from '#compiler' */ /** @import { ComponentContext } from '../../types.js' */ import { dev, is_ignored } from '../../../../../state.js'; import { get_attribute_chunks } from '../../../../../utils/ast.js'; import * as b from '../../../../../utils/builders.js'; -import { is_element_node } from '../../../../nodes.js'; -import { create_derived, build_setter } from '../../utils.js'; +import { create_derived } from '../../utils.js'; import { build_bind_this, validate_binding } from '../shared/utils.js'; import { build_attribute_value } from '../shared/element.js'; import { build_event_handler } from './events.js'; +import { determine_slot } from '../../../../../utils/slot.js'; /** * @param {Component | SvelteComponent | SvelteSelf} node @@ -24,6 +24,15 @@ export function build_component(node, component_name, context, anchor = context. /** @type {ExpressionStatement[]} */ const lets = []; + /** @type {Record<string, typeof context.state>} */ + const states = { + default: { + ...context.state, + scope: node.metadata.scopes.default, + transform: { ...context.state.transform } + } + }; + /** @type {Record<string, TemplateNode[]>} */ const children = {}; @@ -45,7 +54,7 @@ export function build_component(node, component_name, context, anchor = context. * If this component has a slot property, it is a named slot within another component. In this case * the slot scope applies to the component itself, too, and not just its children. */ - let slot_scope_applies_to_itself = false; + let slot_scope_applies_to_itself = !!determine_slot(node); /** * Components may have a children prop and also have child nodes. In this case, we assume @@ -66,9 +75,20 @@ export function build_component(node, component_name, context, anchor = context. props_and_spreads.push(props); } } + + if (slot_scope_applies_to_itself) { + for (const attribute of node.attributes) { + if (attribute.type === 'LetDirective') { + lets.push(/** @type {ExpressionStatement} */ (context.visit(attribute))); + } + } + } + for (const attribute of node.attributes) { if (attribute.type === 'LetDirective') { - lets.push(/** @type {ExpressionStatement} */ (context.visit(attribute))); + if (!slot_scope_applies_to_itself) { + lets.push(/** @type {ExpressionStatement} */ (context.visit(attribute, states.default))); + } } else if (attribute.type === 'OnDirective') { if (!attribute.expression) { context.state.analysis.needs_props = true; @@ -174,9 +194,7 @@ export function build_component(node, component_name, context, anchor = context. const assignment = b.assignment('=', attribute.expression, b.id('$$value')); push_prop( - b.set(attribute.name, [ - b.stmt(build_setter(assignment, context, () => context.visit(assignment))) - ]) + b.set(attribute.name, [b.stmt(/** @type {Expression} */ (context.visit(assignment)))]) ); } } @@ -214,19 +232,7 @@ export function build_component(node, component_name, context, anchor = context. continue; } - let slot_name = 'default'; - - if (is_element_node(child)) { - const attribute = /** @type {Attribute | undefined} */ ( - child.attributes.find( - (attribute) => attribute.type === 'Attribute' && attribute.name === 'slot' - ) - ); - - if (attribute !== undefined) { - slot_name = /** @type {Text[]} */ (attribute.value)[0].data; - } - } + let slot_name = determine_slot(child) ?? 'default'; (children[slot_name] ||= []).push(child); } @@ -242,12 +248,15 @@ export function build_component(node, component_name, context, anchor = context. // @ts-expect-error nodes: children[slot_name] }, - { - ...context.state, - scope: - context.state.scopes.get(slot_name === 'default' ? children[slot_name][0] : node) ?? - context.state.scope - } + slot_name === 'default' + ? slot_scope_applies_to_itself + ? context.state + : states.default + : { + ...context.state, + scope: node.metadata.scopes[slot_name], + transform: { ...context.state.transform } + } ) ); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js new file mode 100644 index 0000000000..5a03fd9be4 --- /dev/null +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/declarations.js @@ -0,0 +1,53 @@ +/** @import { BinaryOperator, Expression, Identifier } from 'estree' */ +/** @import { ComponentContext, Context } from '../../types' */ +import { build_proxy_reassignment, is_state_source, should_proxy_or_freeze } from '../../utils.js'; +import * as b from '../../../../../utils/builders.js'; + +/** + * Turns `foo` into `$.get(foo)` + * @param {Identifier} node + */ +export function get_value(node) { + return b.call('$.get', node); +} + +/** + * + * @param {Context | ComponentContext} context + */ +export function add_state_transformers(context) { + for (const [name, binding] of context.state.scope.declarations) { + if ( + is_state_source(binding, context.state) || + binding.kind === 'derived' || + binding.kind === 'legacy_reactive' + ) { + context.state.transform[name] = { + read: get_value, + assign: (node, value) => { + let call = b.call('$.set', node, value); + + if (context.state.scope.get(`$${node.name}`)?.kind === 'store_sub') { + call = b.call('$.store_unsub', call, b.literal(`$${node.name}`), b.id('$$stores')); + } + + return call; + }, + mutate: (node, mutation) => { + if (context.state.analysis.runes) { + return mutation; + } + + return b.call('$.mutate', node, mutation); + }, + update: (node) => { + return b.call( + node.prefix ? '$.update_pre' : '$.update', + node.argument, + node.operator === '--' && b.literal(-1) + ); + } + }; + } + } +} 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 28d6684a57..7d5c61c056 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 @@ -101,7 +101,7 @@ export function build_attribute_value(value, context) { } return { - has_state: chunk.metadata.expression.has_call, + has_state: chunk.metadata.expression.has_state, has_call: chunk.metadata.expression.has_call, value: /** @type {Expression} */ (context.visit(chunk.expression)) }; 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 874ebe3131..b11c2ed224 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 @@ -145,8 +145,10 @@ export function build_bind_this(expression, value, { state, visit }) { /** @type {Expression[]} */ const values = []; - /** @type {typeof state.getters} */ - const getters = {}; + /** @type {string[]} */ + const seen = []; + + const transform = { ...state.transform }; // Pass in each context variables to the get/set functions, so that we can null out old values on teardown. // Note that we only do this for each context variables, the consequence is that the value might be stale in @@ -154,7 +156,8 @@ export function build_bind_this(expression, value, { state, visit }) { // variables, but that was the same case in Svelte 4, too. Once legacy mode is gone completely, we can revisit this. walk(expression, null, { Identifier(node, { path }) { - if (Object.hasOwn(getters, node.name)) return; + if (seen.includes(node.name)) return; + seen.push(node.name); const parent = /** @type {Expression} */ (path.at(-1)); if (!is_reference(node, parent)) return; @@ -166,14 +169,21 @@ export function build_bind_this(expression, value, { state, visit }) { if (owner.type === 'EachBlock' && scope === binding.scope) { ids.push(node); values.push(/** @type {Expression} */ (visit(node))); - getters[node.name] = node; + + if (transform[node.name]) { + transform[node.name] = { + ...transform[node.name], + read: (node) => node + }; + } + break; } } } }); - const child_state = { ...state, getters: { ...state.getters, ...getters } }; + const child_state = { ...state, transform }; const get = /** @type {Expression} */ (visit(expression, child_state)); const set = /** @type {Expression} */ ( diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js index c349f64b3d..e92d54c3b7 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/AssignmentExpression.js @@ -1,59 +1,16 @@ -/** @import { AssignmentExpression, AssignmentOperator, BinaryOperator, Expression, Node, Pattern } from 'estree' */ +/** @import { AssignmentExpression, AssignmentOperator, Expression, Pattern } from 'estree' */ /** @import { SvelteNode } from '#compiler' */ /** @import { Context, ServerTransformState } from '../types.js' */ import * as b from '../../../../utils/builders.js'; -import { extract_paths } from '../../../../utils/ast.js'; -import { build_getter } from './shared/utils.js'; +import { build_assignment_value } from '../../../../utils/ast.js'; +import { visit_assignment_expression } from '../../shared/assignments.js'; /** * @param {AssignmentExpression} node * @param {Context} context */ export function AssignmentExpression(node, context) { - const parent = /** @type {Node} */ (context.path.at(-1)); - const is_standalone = parent.type.endsWith('Statement'); - - if ( - node.left.type === 'ArrayPattern' || - node.left.type === 'ObjectPattern' || - node.left.type === 'RestElement' - ) { - const value = /** @type {Expression} */ (context.visit(node.right)); - const should_cache = value.type !== 'Identifier'; - const rhs = should_cache ? b.id('$$value') : value; - - let changed = false; - - const assignments = extract_paths(node.left).map((path) => { - const value = path.expression?.(rhs); - - let assignment = build_assignment('=', path.node, value, context); - if (assignment !== null) changed = true; - - return assignment ?? b.assignment('=', path.node, value); - }); - - if (!changed) { - // No change to output -> nothing to transform -> we can keep the original assignment - return context.next(); - } - - 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); - } - - if (should_cache) { - // the right hand side is a complex expression, wrap in an IIFE to cache it - return b.call(b.arrow([rhs], sequence), value); - } - - return sequence; - } - - return build_assignment(node.operator, node.left, node.right, context) || context.next(); + return visit_assignment_expression(node, context, build_assignment); } /** @@ -83,16 +40,9 @@ function build_assignment(operator, left, right, context) { } if (object === left) { - let value = /** @type {Expression} */ (context.visit(right)); - - if (operator !== '=') { - // turn `x += 1` into `x = x + 1` - value = b.binary( - /** @type {BinaryOperator} */ (operator.slice(0, -1)), - build_getter(left, context.state), - value - ); - } + let value = /** @type {Expression} */ ( + context.visit(build_assignment_value(operator, left, right)) + ); return b.call('$.store_set', b.id(name), value); } diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js index 60b3498510..941088228a 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/component.js @@ -20,6 +20,15 @@ export function build_inline_component(node, expression, context) { /** @type {Record<string, LetDirective[]>} */ const lets = { default: [] }; + /** + * Children in the default slot are evaluated in the component scope, + * children in named slots are evaluated in the parent scope + */ + const child_state = { + ...context.state, + scope: node.metadata.scopes.default + }; + /** @type {Record<string, TemplateNode[]>} */ const children = {}; @@ -144,12 +153,12 @@ export function build_inline_component(node, expression, context) { // @ts-expect-error nodes: children[slot_name] }, - { - ...context.state, - scope: - context.state.scopes.get(slot_name === 'default' ? children[slot_name][0] : node) ?? - context.state.scope - } + slot_name === 'default' + ? child_state + : { + ...context.state, + scope: node.metadata.scopes[slot_name] + } ) ); diff --git a/packages/svelte/src/compiler/phases/3-transform/shared/assignments.js b/packages/svelte/src/compiler/phases/3-transform/shared/assignments.js new file mode 100644 index 0000000000..ce7595a06e --- /dev/null +++ b/packages/svelte/src/compiler/phases/3-transform/shared/assignments.js @@ -0,0 +1,77 @@ +/** @import { AssignmentExpression, AssignmentOperator, Expression, 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 * as b from '../../../utils/builders.js'; + +/** + * @template {ClientContext | ServerContext} Context + * @param {AssignmentExpression} node + * @param {Context} context + * @param {(operator: AssignmentOperator, left: Pattern, right: Expression, context: Context) => Expression | null} build_assignment + * @returns + */ +export function visit_assignment_expression(node, context, build_assignment) { + if ( + node.left.type === 'ArrayPattern' || + node.left.type === 'ObjectPattern' || + node.left.type === 'RestElement' + ) { + const value = /** @type {Expression} */ (context.visit(node.right)); + const should_cache = value.type !== 'Identifier'; + const rhs = should_cache ? b.id('$$value') : value; + + let changed = false; + + const assignments = extract_paths(node.left).map((path) => { + const value = path.expression?.(rhs); + + 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 + return context.next(); + } + + const is_standalone = /** @type {Node} */ (context.path.at(-1)).type.endsWith('Statement'); + 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); + } + + if (should_cache) { + // the right hand side is a complex expression, wrap in an IIFE to cache it + const iife = b.arrow([rhs], sequence); + + 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); + } + + return sequence; + } + + if (node.left.type !== 'Identifier' && node.left.type !== 'MemberExpression') { + throw new Error(`Unexpected assignment type ${node.left.type}`); + } + + return ( + build_assignment(node.operator, node.left, node.right, context) ?? + /** @type {Expression} */ (context.next()) + ); +} diff --git a/packages/svelte/src/compiler/phases/3-transform/utils.js b/packages/svelte/src/compiler/phases/3-transform/utils.js index 8a1a00d862..f71c4120f9 100644 --- a/packages/svelte/src/compiler/phases/3-transform/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/utils.js @@ -425,7 +425,7 @@ export function transform_inspect_rune(node, context) { const { state, visit } = context; const as_fn = state.options.generate === 'client'; - if (!dev) return b.unary('void', b.literal(0)); + if (!dev) return b.empty; if (node.callee.type === 'MemberExpression') { const raw_inspect_args = /** @type {CallExpression} */ (node.callee.object).arguments; diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index 5eba0d3cf2..633f326080 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -1,9 +1,8 @@ /** @import { ClassDeclaration, Expression, FunctionDeclaration, Identifier, ImportDeclaration, MemberExpression, Node, Pattern, VariableDeclarator } from 'estree' */ /** @import { Context, Visitor } from 'zimmerframe' */ -/** @import { AnimateDirective, Binding, DeclarationKind, EachBlock, ElementLike, LetDirective, SvelteNode, TransitionDirective, UseDirective } from '#compiler' */ +/** @import { AnimateDirective, Binding, Component, DeclarationKind, EachBlock, ElementLike, LetDirective, SvelteComponent, SvelteNode, SvelteSelf, TransitionDirective, UseDirective } from '#compiler' */ import is_reference from 'is-reference'; import { walk } from 'zimmerframe'; -import { is_element_node } from './nodes.js'; import * as b from '../utils/builders.js'; import * as e from '../errors.js'; import { @@ -13,6 +12,7 @@ import { unwrap_pattern } from '../utils/ast.js'; import { is_reserved, is_rune } from '../../utils.js'; +import { determine_slot } from '../utils/slot.js'; export class Scope { /** @type {ScopeRoot} */ @@ -290,52 +290,47 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { * @type {Visitor<ElementLike, State, SvelteNode>} */ const SvelteFragment = (node, { state, next }) => { - const [scope] = analyze_let_directives(node, state.scope); + const scope = state.scope.child(); scopes.set(node, scope); next({ scope }); }; /** - * @param {ElementLike} node - * @param {Scope} parent + * @type {Visitor<Component | SvelteComponent | SvelteSelf, State, SvelteNode>} */ - function analyze_let_directives(node, parent) { - const scope = parent.child(); - let is_default_slot = true; + const Component = (node, context) => { + node.metadata.scopes = { + default: context.state.scope.child() + }; + + const default_state = determine_slot(node) + ? context.state + : { scope: node.metadata.scopes.default }; for (const attribute of node.attributes) { if (attribute.type === 'LetDirective') { - /** @type {Binding[]} */ - const bindings = []; - scope.declarators.set(attribute, bindings); + context.visit(attribute, default_state); + } else { + context.visit(attribute); + } + } - // attach the scope to the directive itself, as well as the - // contents to which it applies - scopes.set(attribute, scope); + for (const child of node.fragment.nodes) { + let state = default_state; - if (attribute.expression) { - for (const id of extract_identifiers_from_destructuring(attribute.expression)) { - const binding = scope.declare(id, 'derived', 'const'); - bindings.push(binding); - } - } else { - /** @type {Identifier} */ - const id = { - name: attribute.name, - type: 'Identifier', - start: attribute.start, - end: attribute.end - }; - const binding = scope.declare(id, 'derived', 'const'); - bindings.push(binding); - } - } else if (attribute.type === 'Attribute' && attribute.name === 'slot') { - is_default_slot = false; + const slot_name = determine_slot(child); + + if (slot_name !== null) { + node.metadata.scopes[slot_name] = context.state.scope.child(); + + state = { + scope: node.metadata.scopes[slot_name] + }; } - } - return /** @type {const} */ ([scope, is_default_slot]); - } + context.visit(child, state); + } + }; /** * @type {Visitor<AnimateDirective | TransitionDirective | UseDirective, State, SvelteNode>} @@ -384,48 +379,37 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { SvelteElement: SvelteFragment, RegularElement: SvelteFragment, - Component(node, { state, visit, path }) { - state.scope.reference(b.id(node.name), path); + LetDirective(node, context) { + const scope = context.state.scope; - // let:x is super weird: - // - for the default slot, its scope only applies to children that are not slots themselves - // - for named slots, its scope applies to the component itself, too - const [scope, is_default_slot] = analyze_let_directives(node, state.scope); - if (is_default_slot) { - for (const attribute of node.attributes) { - visit(attribute); - } - } else { - scopes.set(node, scope); + /** @type {Binding[]} */ + const bindings = []; + scope.declarators.set(node, bindings); - for (const attribute of node.attributes) { - visit(attribute, { ...state, scope }); + if (node.expression) { + for (const id of extract_identifiers_from_destructuring(node.expression)) { + const binding = scope.declare(id, 'derived', 'const'); + bindings.push(binding); } + } else { + /** @type {Identifier} */ + const id = { + name: node.name, + type: 'Identifier', + start: node.start, + end: node.end + }; + const binding = scope.declare(id, 'derived', 'const'); + bindings.push(binding); } + }, - for (const child of node.fragment.nodes) { - if ( - is_element_node(child) && - child.attributes.some( - (attribute) => attribute.type === 'Attribute' && attribute.name === 'slot' - ) - ) { - // <div slot="..."> inherits the scope above the component unless the component is a named slot itself, because slots are hella weird - scopes.set(child, is_default_slot ? state.scope : scope); - visit(child, { scope: is_default_slot ? state.scope : scope }); - } else { - if (child.type === 'ExpressionTag') { - // expression tag is a special case — we don't visit it directly, but via process_children, - // so we need to set the scope on the expression rather than the tag itself - scopes.set(child.expression, scope); - } else { - scopes.set(child, scope); - } - - visit(child, { scope }); - } - } + Component: (node, context) => { + context.state.scope.reference(b.id(node.name), context.path); + Component(node, context); }, + SvelteSelf: Component, + SvelteComponent: Component, // updates AssignmentExpression(node, { state, next }) { @@ -532,7 +516,6 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { references_within.add(id); } } - scopes.set(node.expression, state.scope); // context and children are a new scope const scope = state.scope.child(); diff --git a/packages/svelte/src/compiler/types/index.d.ts b/packages/svelte/src/compiler/types/index.d.ts index 47dc9d7eb6..f7d111a267 100644 --- a/packages/svelte/src/compiler/types/index.d.ts +++ b/packages/svelte/src/compiler/types/index.d.ts @@ -271,7 +271,6 @@ export interface Binding { * - `snippet`: A snippet parameter * - `store_sub`: A $store value * - `legacy_reactive`: A `$:` declaration - * - `legacy_reactive_import`: An imported binding that is mutated inside the component */ kind: | 'normal' @@ -284,8 +283,7 @@ export interface Binding { | 'each' | 'snippet' | 'store_sub' - | 'legacy_reactive' - | 'legacy_reactive_import'; + | 'legacy_reactive'; declaration_kind: DeclarationKind; /** * What the value was initialized with. diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index 86341d07d0..20fdb25ee4 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -16,6 +16,7 @@ import type { ChainExpression, SimpleCallExpression } from 'estree'; +import type { Scope } from '../phases/scope'; export interface BaseNode { type: string; @@ -275,6 +276,7 @@ interface BaseElement extends BaseNode { export interface Component extends BaseElement { type: 'Component'; metadata: { + scopes: Record<string, Scope>; dynamic: boolean; }; } @@ -311,6 +313,9 @@ export interface SvelteComponent extends BaseElement { type: 'SvelteComponent'; name: 'svelte:component'; expression: Expression; + metadata: { + scopes: Record<string, Scope>; + }; } interface SvelteDocument extends BaseElement { @@ -356,6 +361,9 @@ export interface SvelteOptionsRaw extends BaseElement { export interface SvelteSelf extends BaseElement { type: 'SvelteSelf'; name: 'svelte:self'; + metadata: { + scopes: Record<string, Scope>; + }; } interface SvelteWindow extends BaseElement { diff --git a/packages/svelte/src/compiler/utils/ast.js b/packages/svelte/src/compiler/utils/ast.js index 95b5d2fb2d..47bbdb945f 100644 --- a/packages/svelte/src/compiler/utils/ast.js +++ b/packages/svelte/src/compiler/utils/ast.js @@ -563,3 +563,15 @@ export function build_fallback(expression, fallback) { ? b.await(b.call('$.fallback', expression, b.thunk(fallback, true), b.true)) : b.call('$.fallback', expression, b.thunk(fallback), b.true); } + +/** + * @param {ESTree.AssignmentOperator} operator + * @param {ESTree.Identifier | ESTree.MemberExpression} left + * @param {ESTree.Expression} right + */ +export function build_assignment_value(operator, left, right) { + return operator === '=' + ? right + : // turn something like x += 1 into x = x + 1 + b.binary(/** @type {ESTree.BinaryOperator} */ (operator.slice(0, -1)), left, right); +} diff --git a/packages/svelte/src/compiler/utils/slot.js b/packages/svelte/src/compiler/utils/slot.js new file mode 100644 index 0000000000..1c05ec8ee6 --- /dev/null +++ b/packages/svelte/src/compiler/utils/slot.js @@ -0,0 +1,20 @@ +/** @import { SvelteNode } from '#compiler' */ +import { is_element_node } from '../phases/nodes.js'; +import { is_text_attribute } from './ast.js'; + +/** + * @param {SvelteNode} node + */ +export function determine_slot(node) { + if (!is_element_node(node)) return null; + + for (const attribute of node.attributes) { + if (attribute.type !== 'Attribute') continue; + if (attribute.name !== 'slot') continue; + if (!is_text_attribute(attribute)) continue; + + return /** @type {string} */ (attribute.value[0].data); + } + + return null; +} diff --git a/packages/svelte/tests/runtime-legacy/samples/each-blocks-update/_config.js b/packages/svelte/tests/runtime-legacy/samples/each-blocks-update/_config.js new file mode 100644 index 0000000000..a00605a634 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/each-blocks-update/_config.js @@ -0,0 +1,27 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + html: ` + <button>1</button> + <button>2</button> + <button>3</button> + <p>1, 2, 3</p> + `, + + test({ assert, target }) { + let buttons = target.querySelectorAll('button'); + + flushSync(() => buttons[2].click()); + + assert.htmlEqual( + target.innerHTML, + ` + <button>1</button> + <button>2</button> + <button>4</button> + <p>1, 2, 4</p> + ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/each-blocks-update/main.svelte b/packages/svelte/tests/runtime-legacy/samples/each-blocks-update/main.svelte new file mode 100644 index 0000000000..0abded02ff --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/each-blocks-update/main.svelte @@ -0,0 +1,9 @@ +<script> + let arr = [1, 2, 3]; +</script> + +{#each arr as n} + <button on:click={() => n++}>{n}</button> +{/each} + +<p>{arr.join(', ')}</p> 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 new file mode 100644 index 0000000000..9400b52718 --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/destructured-assignments/_expected/client/index.svelte.js @@ -0,0 +1,12 @@ +/* index.svelte.js generated by Svelte VERSION */ +import * as $ from "svelte/internal/client"; + +let a = $.source(1); +let b = $.source(2); + +export function update(array) { + ( + $.set(a, $.proxy(array[0])), + $.set(b, $.proxy(array[1])) + ); +} \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/destructured-assignments/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/destructured-assignments/_expected/server/index.svelte.js new file mode 100644 index 0000000000..846ed48458 --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/destructured-assignments/_expected/server/index.svelte.js @@ -0,0 +1,9 @@ +/* index.svelte.js generated by Svelte VERSION */ +import * as $ from "svelte/internal/server"; + +let a = 1; +let b = 2; + +export function update(array) { + [a, b] = array; +} \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/destructured-assignments/index.svelte.js b/packages/svelte/tests/snapshot/samples/destructured-assignments/index.svelte.js new file mode 100644 index 0000000000..9c0da7558a --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/destructured-assignments/index.svelte.js @@ -0,0 +1,6 @@ +let a = $state(1); +let b = $state(2); + +export function update(array) { + [a, b] = array; +} diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 969f89222d..df012254f6 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -920,7 +920,6 @@ declare module 'svelte/compiler' { * - `snippet`: A snippet parameter * - `store_sub`: A $store value * - `legacy_reactive`: A `$:` declaration - * - `legacy_reactive_import`: An imported binding that is mutated inside the component */ kind: | 'normal' @@ -933,8 +932,7 @@ declare module 'svelte/compiler' { | 'each' | 'snippet' | 'store_sub' - | 'legacy_reactive' - | 'legacy_reactive_import'; + | 'legacy_reactive'; declaration_kind: DeclarationKind; /** * What the value was initialized with. @@ -1733,6 +1731,7 @@ declare module 'svelte/compiler' { interface Component extends BaseElement { type: 'Component'; metadata: { + scopes: Record<string, Scope>; dynamic: boolean; }; } @@ -1769,6 +1768,9 @@ declare module 'svelte/compiler' { type: 'SvelteComponent'; name: 'svelte:component'; expression: Expression; + metadata: { + scopes: Record<string, Scope>; + }; } interface SvelteDocument extends BaseElement { @@ -1814,6 +1816,9 @@ declare module 'svelte/compiler' { interface SvelteSelf extends BaseElement { type: 'SvelteSelf'; name: 'svelte:self'; + metadata: { + scopes: Record<string, Scope>; + }; } interface SvelteWindow extends BaseElement {