diff --git a/.changeset/smart-cars-know.md b/.changeset/smart-cars-know.md new file mode 100644 index 0000000000..5b87239e41 --- /dev/null +++ b/.changeset/smart-cars-know.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: avoid recreating handlers for component events diff --git a/.changeset/wild-pumas-count.md b/.changeset/wild-pumas-count.md new file mode 100644 index 0000000000..5d34731ef2 --- /dev/null +++ b/.changeset/wild-pumas-count.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: call correct event handler for properties of non-reactive objects 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 824f784a6b..85dfc6c67e 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js @@ -40,8 +40,8 @@ export function Attribute(node, context) { const delegated_event = get_delegated_event(node.name.slice(2), expression, context); if (delegated_event !== null) { - if (delegated_event.type === 'hoistable') { - delegated_event.function.metadata.hoistable = true; + if (delegated_event.hoisted) { + delegated_event.function.metadata.hoisted = true; } node.metadata.delegated = delegated_event; @@ -50,6 +50,9 @@ export function Attribute(node, context) { } } +/** @type {DelegatedEvent} */ +const unhoisted = { hoisted: false }; + /** * Checks if given event attribute can be delegated/hoisted and returns the corresponding info if so * @param {string} event_name @@ -58,26 +61,24 @@ export function Attribute(node, context) { * @returns {null | DelegatedEvent} */ function get_delegated_event(event_name, handler, context) { - // Handle delegated event handlers. Bail-out if not a delegated event. + // Handle delegated event handlers. Bail out if not a delegated event. if (!handler || !is_delegated(event_name)) { return null; } - // If we are not working with a RegularElement, then bail-out. + // If we are not working with a RegularElement, then bail out. const element = context.path.at(-1); if (element?.type !== 'RegularElement') { return null; } - /** @type {DelegatedEvent} */ - const non_hoistable = { type: 'non-hoistable' }; /** @type {FunctionExpression | FunctionDeclaration | ArrowFunctionExpression | null} */ let target_function = null; let binding = null; if (element.metadata.has_spread) { // event attribute becomes part of the dynamic spread array - return non_hoistable; + return unhoisted; } if (handler.type === 'ArrowFunctionExpression' || handler.type === 'FunctionExpression') { @@ -86,14 +87,14 @@ function get_delegated_event(event_name, handler, context) { binding = context.state.scope.get(handler.name); if (context.state.analysis.module.scope.references.has(handler.name)) { - // If a binding with the same name is referenced in the module scope (even if not declared there), bail-out - return non_hoistable; + // If a binding with the same name is referenced in the module scope (even if not declared there), bail out + return unhoisted; } if (binding != null) { for (const { path } of binding.references) { const parent = path.at(-1); - if (parent == null) return non_hoistable; + if (parent === undefined) return unhoisted; const grandparent = path.at(-2); @@ -120,17 +121,17 @@ function get_delegated_event(event_name, handler, context) { element.metadata.has_spread || !is_delegated(event_name) ) { - return non_hoistable; + return unhoisted; } } else if (parent.type !== 'FunctionDeclaration' && parent.type !== 'VariableDeclarator') { - return non_hoistable; + return unhoisted; } } } - // If the binding is exported, bail-out + // If the binding is exported, bail out if (context.state.analysis.exports.find((node) => node.name === handler.name)) { - return non_hoistable; + return unhoisted; } if (binding !== null && binding.initial !== null && !binding.mutated && !binding.is_called) { @@ -146,27 +147,25 @@ function get_delegated_event(event_name, handler, context) { } } - // If we can't find a function, bail-out - if (target_function == null) return non_hoistable; - // If the function is marked as non-hoistable, bail-out - if (target_function.metadata.hoistable === 'impossible') return non_hoistable; - // If the function has more than one arg, then bail-out - if (target_function.params.length > 1) return non_hoistable; + // If we can't find a function, or the function has multiple parameters, bail out + if (target_function == null || target_function.params.length > 1) { + return unhoisted; + } const visited_references = new Set(); const scope = target_function.metadata.scope; for (const [reference] of scope.references) { - // Bail-out if the arguments keyword is used - if (reference === 'arguments') return non_hoistable; - // Bail-out if references a store subscription - if (scope.get(`$${reference}`)?.kind === 'store_sub') return non_hoistable; + // Bail out if the arguments keyword is used + if (reference === 'arguments') return unhoisted; + // Bail out if references a store subscription + if (scope.get(`$${reference}`)?.kind === 'store_sub') return unhoisted; const binding = scope.get(reference); const local_binding = context.state.scope.get(reference); // If we are referencing a binding that is shadowed in another scope then bail out. if (local_binding !== null && binding !== null && local_binding.node !== binding.node) { - return non_hoistable; + return unhoisted; } // If we have multiple references to the same store using $ prefix, bail out. @@ -175,17 +174,17 @@ function get_delegated_event(event_name, handler, context) { binding.kind === 'store_sub' && visited_references.has(reference.slice(1)) ) { - return non_hoistable; + return unhoisted; } - // If we reference the index within an each block, then bail-out. - if (binding !== null && binding.initial?.type === 'EachBlock') return non_hoistable; + // If we reference the index within an each block, then bail out. + if (binding !== null && binding.initial?.type === 'EachBlock') return unhoisted; if ( binding !== null && - // Bail-out if the the binding is a rest param + // Bail out if the the binding is a rest param (binding.declaration_kind === 'rest_param' || - // Bail-out if we reference anything from the EachBlock (for now) that mutates in non-runes mode, + // 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' || @@ -193,12 +192,12 @@ function get_delegated_event(event_name, handler, context) { binding.kind === 'legacy_reactive_import') && binding.mutated)) ) { - return non_hoistable; + return unhoisted; } visited_references.add(reference); } - return { type: 'hoistable', function: target_function }; + return { hoisted: true, function: target_function }; } /** diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/function.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/function.js index da1f06d7f6..c6151992bf 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/function.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/shared/function.js @@ -8,9 +8,8 @@ export function visit_function(node, context) { // TODO retire this in favour of a more general solution based on bindings node.metadata = { - // module context -> already hoisted - hoistable: context.state.ast_type === 'module' ? 'impossible' : false, - hoistable_params: [], + hoisted: false, + hoisted_params: [], scope: context.state.scope }; 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 58fae325ef..57ede67bb3 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/utils.js @@ -481,7 +481,7 @@ export function build_proxy_reassignment(value, proxy_reference) { * @param {ComponentContext} context * @returns {Pattern[]} */ -function get_hoistable_params(node, context) { +function get_hoisted_params(node, context) { const scope = context.state.scope; /** @type {Identifier[]} */ @@ -549,15 +549,15 @@ function get_hoistable_params(node, context) { * @param {ComponentContext} context * @returns {Pattern[]} */ -export function build_hoistable_params(node, context) { - const hoistable_params = get_hoistable_params(node, context); - node.metadata.hoistable_params = hoistable_params; +export function build_hoisted_params(node, context) { + const hoisted_params = get_hoisted_params(node, context); + node.metadata.hoisted_params = hoisted_params; /** @type {Pattern[]} */ const params = []; if (node.params.length === 0) { - if (hoistable_params.length > 0) { + if (hoisted_params.length > 0) { // For the event object params.push(b.id('_')); } @@ -567,7 +567,7 @@ export function build_hoistable_params(node, context) { } } - params.push(...hoistable_params); + params.push(...hoisted_params); return params; } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Attribute.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Attribute.js index 1fcf110216..3cab0d7eec 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/Attribute.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/Attribute.js @@ -1,7 +1,7 @@ /** @import { Attribute } from '#compiler' */ /** @import { ComponentContext } from '../types' */ import { is_event_attribute } from '../../../../utils/ast.js'; -import { build_event_attribute } from './shared/element.js'; +import { visit_event_attribute } from './shared/events.js'; /** * @param {Attribute} node @@ -9,6 +9,6 @@ import { build_event_attribute } from './shared/element.js'; */ export function Attribute(node, context) { if (is_event_attribute(node)) { - build_event_attribute(node, context); + visit_event_attribute(node, context); } } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/FunctionDeclaration.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/FunctionDeclaration.js index f223d748ca..ed8fefc6ba 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/FunctionDeclaration.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/FunctionDeclaration.js @@ -1,6 +1,6 @@ /** @import { FunctionDeclaration } from 'estree' */ /** @import { ComponentContext } from '../types' */ -import { build_hoistable_params } from '../utils.js'; +import { build_hoisted_params } from '../utils.js'; import * as b from '../../../../utils/builders.js'; /** @@ -8,21 +8,13 @@ import * as b from '../../../../utils/builders.js'; * @param {ComponentContext} context */ export function FunctionDeclaration(node, context) { - const metadata = node.metadata; - const state = { ...context.state, in_constructor: false }; - if (metadata?.hoistable === true) { - const params = build_hoistable_params(node, context); + if (node.metadata?.hoisted === true) { + const params = build_hoisted_params(node, context); + const body = context.visit(node.body, state); - context.state.hoisted.push( - /** @type {FunctionDeclaration} */ ({ - ...node, - id: node.id !== null ? context.visit(node.id, state) : null, - params, - body: context.visit(node.body, state) - }) - ); + context.state.hoisted.push(/** @type {FunctionDeclaration} */ ({ ...node, params, body })); return b.empty; } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/OnDirective.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/OnDirective.js index dc7da67e16..9bd22b7eb4 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/OnDirective.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/OnDirective.js @@ -1,11 +1,38 @@ -/** @import { OnDirective } from '#compiler' */ +/** @import { OnDirective, SvelteNode } from '#compiler' */ /** @import { ComponentContext } from '../types' */ -import { build_event } from './shared/element.js'; +import * as b from '../../../../utils/builders.js'; +import { build_event, build_event_handler } from './shared/events.js'; + +const modifiers = [ + 'stopPropagation', + 'stopImmediatePropagation', + 'preventDefault', + 'self', + 'trusted', + 'once' +]; /** * @param {OnDirective} node * @param {ComponentContext} context */ export function OnDirective(node, context) { - build_event(node, node.metadata.expression, context); + if (!node.expression) { + context.state.analysis.needs_props = true; + } + + let handler = build_event_handler(node.expression, node.metadata.expression, context); + + for (const modifier of modifiers) { + if (node.modifiers.includes(modifier)) { + handler = b.call('$.' + modifier, handler); + } + } + + const capture = node.modifiers.includes('capture'); + const passive = + node.modifiers.includes('passive') || + (node.modifiers.includes('nonpassive') ? false : undefined); + + return build_event(node.name, context.state.node, handler, capture, passive); } 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 13ebb41d5b..2063027782 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 @@ -24,11 +24,11 @@ import { get_attribute_name, build_attribute_value, build_class_directives, - build_event_attribute, build_style_directives } from './shared/element.js'; import { process_children } from './shared/fragment.js'; import { build_render_statement, build_update, build_update_assignment } from './shared/utils.js'; +import { visit_event_attribute } from './shared/events.js'; /** * @param {RegularElement} node @@ -138,6 +138,13 @@ export function RegularElement(node, context) { 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'); + + context.state.after_update.push( + b.stmt(has_action_directive ? b.call('$.effect', b.thunk(handler)) : handler) + ); } else { if (attribute.type === 'BindDirective') { if (attribute.name === 'group' || attribute.name === 'checked') { @@ -214,7 +221,7 @@ export function RegularElement(node, context) { ) { might_need_event_replaying = true; } - build_event_attribute(attribute, context); + visit_event_attribute(attribute, context); continue; } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBody.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBody.js index 6c7637adee..c046b21534 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBody.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteBody.js @@ -1,14 +1,11 @@ /** @import { SvelteBody } from '#compiler' */ /** @import { ComponentContext } from '../types' */ -import * as b from '../../../../utils/builders.js'; +import { visit_special_element } from './shared/special_element.js'; /** * @param {SvelteBody} node * @param {ComponentContext} context */ export function SvelteBody(node, context) { - context.next({ - ...context.state, - node: b.id('$.document.body') - }); + visit_special_element(node, '$.document.body', context); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteDocument.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteDocument.js index e11c53fffb..bff32fa570 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteDocument.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteDocument.js @@ -1,14 +1,11 @@ /** @import { SvelteDocument } from '#compiler' */ /** @import { ComponentContext } from '../types' */ -import * as b from '../../../../utils/builders.js'; +import { visit_special_element } from './shared/special_element.js'; /** * @param {SvelteDocument} node * @param {ComponentContext} context */ export function SvelteDocument(node, context) { - context.next({ - ...context.state, - node: b.id('$.document') - }); + visit_special_element(node, '$.document', context); } 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 72174d1015..90e74259f1 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 @@ -71,6 +71,9 @@ export function SvelteElement(node, context) { 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, inner_context.state)); + inner_context.state.after_update.push(b.stmt(handler)); } else { context.visit(attribute, inner_context.state); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteWindow.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteWindow.js index 57ac3ee47b..a29fec60bf 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteWindow.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteWindow.js @@ -1,14 +1,12 @@ +/** @import { Expression } from 'estree' */ /** @import { SvelteWindow } from '#compiler' */ /** @import { ComponentContext } from '../types' */ -import * as b from '../../../../utils/builders.js'; +import { visit_special_element } from './shared/special_element.js'; /** * @param {SvelteWindow} node * @param {ComponentContext} context */ export function SvelteWindow(node, context) { - context.next({ - ...context.state, - node: b.id('$.window') - }); + visit_special_element(node, '$.window', context); } 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 5dfcec4847..1f024ee450 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 @@ -13,7 +13,7 @@ import { is_state_source, should_proxy_or_freeze } from '../utils.js'; -import { is_hoistable_function } from '../../utils.js'; +import { is_hoisted_function } from '../../utils.js'; /** * @param {VariableDeclaration} node @@ -36,11 +36,11 @@ export function VariableDeclaration(node, context) { rune === '$state.snapshot' || rune === '$state.is' ) { - if (init != null && is_hoistable_function(init)) { - const hoistable_function = context.visit(init); + if (init != null && is_hoisted_function(init)) { context.state.hoisted.push( - b.declaration('const', declarator.id, /** @type {Expression} */ (hoistable_function)) + b.declaration('const', declarator.id, /** @type {Expression} */ (context.visit(init))) ); + continue; } declarations.push(/** @type {VariableDeclarator} */ (context.visit(declarator))); @@ -219,11 +219,9 @@ export function VariableDeclaration(node, context) { if (!has_state && !has_props) { const init = declarator.init; - if (init != null && is_hoistable_function(init)) { - const hoistable_function = context.visit(init); - + if (init != null && is_hoisted_function(init)) { context.state.hoisted.push( - b.declaration('const', declarator.id, /** @type {Expression} */ (hoistable_function)) + b.declaration('const', declarator.id, /** @type {Expression} */ (context.visit(init))) ); continue; 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 d498ef10ea..ab1abc82da 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 @@ -6,8 +6,9 @@ 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 { build_bind_this, build_event_handler, build_validate_binding } from '../shared/utils.js'; +import { build_bind_this, build_validate_binding } from '../shared/utils.js'; import { build_attribute_value } from '../shared/element.js'; +import { build_event_handler } from './events.js'; /** * @param {Component | SvelteComponent | SvelteSelf} node @@ -69,12 +70,21 @@ export function build_component(node, component_name, context, anchor = context. if (attribute.type === 'LetDirective') { lets.push(/** @type {ExpressionStatement} */ (context.visit(attribute))); } else if (attribute.type === 'OnDirective') { - events[attribute.name] ||= []; - let handler = build_event_handler(attribute, null, context); + if (!attribute.expression) { + context.state.analysis.needs_props = true; + } + + let handler = build_event_handler( + attribute.expression, + attribute.metadata.expression, + context + ); + if (attribute.modifiers.includes('once')) { handler = b.call('$.once', handler); } - events[attribute.name].push(handler); + + (events[attribute.name] ||= []).push(handler); } else if (attribute.type === 'SpreadAttribute') { const expression = /** @type {Expression} */ (context.visit(attribute)); if (attribute.metadata.expression.has_state) { 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 06563cd033..28d6684a57 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 @@ -1,15 +1,10 @@ /** @import { Expression, Identifier } from 'estree' */ -/** @import { Attribute, ClassDirective, DelegatedEvent, ExpressionMetadata, ExpressionTag, Namespace, OnDirective, RegularElement, StyleDirective, SvelteElement, SvelteNode } from '#compiler' */ +/** @import { Attribute, ClassDirective, ExpressionMetadata, Namespace, RegularElement, StyleDirective, SvelteElement } from '#compiler' */ /** @import { ComponentContext } from '../../types' */ -import { - is_capture_event, - is_passive_event, - normalize_attribute -} from '../../../../../../utils.js'; -import { get_attribute_expression } from '../../../../../utils/ast.js'; +import { normalize_attribute } from '../../../../../../utils.js'; import * as b from '../../../../../utils/builders.js'; import { build_getter } from '../../utils.js'; -import { build_event_handler, build_template_literal, build_update } from './utils.js'; +import { build_template_literal, build_update } from './utils.js'; /** * Serializes each style directive into something like `$.set_style(element, style_property, value)` @@ -131,143 +126,3 @@ export function get_attribute_name(element, attribute, context) { return attribute.name; } - -/** - * @param {Attribute & { value: ExpressionTag | [ExpressionTag] }} node - * @param {ComponentContext} context - */ -export function build_event_attribute(node, context) { - /** @type {string[]} */ - const modifiers = []; - - let event_name = node.name.slice(2); - if (is_capture_event(event_name)) { - event_name = event_name.slice(0, -7); - modifiers.push('capture'); - } - - build_event( - { - name: event_name, - expression: get_attribute_expression(node), - modifiers, - delegated: node.metadata.delegated - }, - !Array.isArray(node.value) && node.value?.type === 'ExpressionTag' - ? node.value.metadata.expression - : null, - context - ); -} - -/** - * Serializes an event handler function of the `on:` directive or an attribute starting with `on` - * @param {{name: string;modifiers: string[];expression: Expression | null;delegated?: DelegatedEvent | null;}} node - * @param {null | ExpressionMetadata} metadata - * @param {ComponentContext} context - */ -export function build_event(node, metadata, context) { - const state = context.state; - - /** @type {Expression} */ - let expression; - - if (node.expression) { - let handler = build_event_handler(node, metadata, context); - const event_name = node.name; - const delegated = node.delegated; - - if (delegated != null) { - let delegated_assignment; - - if (!state.events.has(event_name)) { - state.events.add(event_name); - } - // Hoist function if we can, otherwise we leave the function as is - if (delegated.type === 'hoistable') { - if (delegated.function === node.expression) { - const func_name = context.state.scope.root.unique('on_' + event_name); - state.hoisted.push(b.var(func_name, handler)); - handler = func_name; - } - if (node.modifiers.includes('once')) { - handler = b.call('$.once', handler); - } - const hoistable_params = /** @type {Expression[]} */ ( - delegated.function.metadata.hoistable_params - ); - // When we hoist a function we assign an array with the function and all - // hoisted closure params. - const args = [handler, ...hoistable_params]; - delegated_assignment = b.array(args); - } else { - if (node.modifiers.includes('once')) { - handler = b.call('$.once', handler); - } - delegated_assignment = handler; - } - - state.init.push( - b.stmt( - b.assignment( - '=', - b.member(context.state.node, b.id('__' + event_name)), - delegated_assignment - ) - ) - ); - return; - } - - if (node.modifiers.includes('once')) { - handler = b.call('$.once', handler); - } - - const args = [ - b.literal(event_name), - context.state.node, - handler, - b.literal(node.modifiers.includes('capture')) - ]; - - if (node.modifiers.includes('passive')) { - args.push(b.literal(true)); - } else if (node.modifiers.includes('nonpassive')) { - args.push(b.literal(false)); - } else if ( - is_passive_event(node.name) && - /** @type {OnDirective} */ (node).type !== 'OnDirective' - ) { - // For on:something events we don't apply passive behaviour to match Svelte 4. - args.push(b.literal(true)); - } - - // Events need to run in order with bindings/actions - expression = b.call('$.event', ...args); - } else { - expression = b.call( - '$.event', - b.literal(node.name), - state.node, - build_event_handler(node, metadata, context) - ); - } - - const parent = /** @type {SvelteNode} */ (context.path.at(-1)); - const has_action_directive = - parent.type === 'RegularElement' && parent.attributes.find((a) => a.type === 'UseDirective'); - const statement = b.stmt( - has_action_directive ? b.call('$.effect', b.thunk(expression)) : expression - ); - - if ( - parent.type === 'SvelteDocument' || - parent.type === 'SvelteWindow' || - parent.type === 'SvelteBody' - ) { - // These nodes are above the component tree, and its events should run parent first - state.before_init.push(statement); - } else { - state.after_update.push(statement); - } -} diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/events.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/events.js new file mode 100644 index 0000000000..9510715a91 --- /dev/null +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/events.js @@ -0,0 +1,144 @@ +/** @import { Expression } from 'estree' */ +/** @import { Attribute, ExpressionMetadata, ExpressionTag, OnDirective, SvelteNode } from '#compiler' */ +/** @import { ComponentContext } from '../../types' */ +import { is_capture_event, is_passive_event } from '../../../../../../utils.js'; +import * as b from '../../../../../utils/builders.js'; + +/** + * @param {Attribute} node + * @param {ComponentContext} context + */ +export function visit_event_attribute(node, context) { + let capture = false; + + let event_name = node.name.slice(2); + if (is_capture_event(event_name)) { + event_name = event_name.slice(0, -7); + capture = true; + } + + // we still need to support the weird `onclick="{() => {...}}" form + const tag = Array.isArray(node.value) + ? /** @type {ExpressionTag} */ (node.value[0]) + : /** @type {ExpressionTag} */ (node.value); + + let handler = build_event_handler(tag.expression, tag.metadata.expression, context); + + if (node.metadata.delegated) { + let delegated_assignment; + + if (!context.state.events.has(event_name)) { + context.state.events.add(event_name); + } + + // Hoist function if we can, otherwise we leave the function as is + if (node.metadata.delegated.hoisted) { + if (node.metadata.delegated.function === tag.expression) { + const func_name = context.state.scope.root.unique('on_' + event_name); + context.state.hoisted.push(b.var(func_name, handler)); + handler = func_name; + } + + const hoisted_params = /** @type {Expression[]} */ ( + node.metadata.delegated.function.metadata.hoisted_params + ); + + // When we hoist a function we assign an array with the function and all + // hoisted closure params. + const args = [handler, ...hoisted_params]; + delegated_assignment = b.array(args); + } else { + delegated_assignment = handler; + } + + context.state.init.push( + b.stmt( + b.assignment( + '=', + b.member(context.state.node, b.id('__' + event_name)), + delegated_assignment + ) + ) + ); + } else { + const statement = b.stmt( + build_event(event_name, context.state.node, handler, capture, undefined) + ); + + const type = /** @type {SvelteNode} */ (context.path.at(-1)).type; + + if (type === 'SvelteDocument' || type === 'SvelteWindow' || type === 'SvelteBody') { + // These nodes are above the component tree, and its events should run parent first + context.state.init.push(statement); + } else { + context.state.after_update.push(statement); + } + } +} + +/** + * Creates a `$.event(...)` call for non-delegated event handlers + * @param {string} event_name + * @param {Expression} node + * @param {Expression} handler + * @param {boolean} capture + * @param {boolean | undefined} passive + */ +export function build_event(event_name, node, handler, capture, passive) { + return b.call( + '$.event', + b.literal(event_name), + node, + handler, + capture && b.true, + passive === undefined ? undefined : b.literal(passive) + ); +} + +/** + * Creates an event handler + * @param {Expression | null} node + * @param {ExpressionMetadata} metadata + * @param {ComponentContext} context + * @returns {Expression} + */ +export function build_event_handler(node, metadata, context) { + if (node === null) { + // bubble event + return b.function( + null, + [b.id('$$arg')], + b.block([b.stmt(b.call('$.bubble_event.call', b.this, b.id('$$props'), b.id('$$arg')))]) + ); + } + + let handler = /** @type {Expression} */ (context.visit(node)); + + // inline handler + if (handler.type === 'ArrowFunctionExpression' || handler.type === 'FunctionExpression') { + return handler; + } + + // function declared in the script + if ( + handler.type === 'Identifier' && + context.state.scope.get(handler.name)?.declaration_kind !== 'import' + ) { + return handler; + } + + if (metadata.has_call) { + // memoize where necessary + const id = b.id(context.state.scope.generate('event_handler')); + + context.state.init.push(b.var(id, b.call('$.derived', b.thunk(handler)))); + handler = b.call('$.get', id); + } + + // wrap the handler in a function, so the expression is re-evaluated for each event + return b.function( + null, + [b.rest(b.id('$$args'))], + b.block([b.stmt(b.call(b.member(handler, b.id('apply'), false, true), b.this, b.id('$$args')))]) + ); +} diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/function.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/function.js index 9939621890..e66cdcb091 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/function.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/function.js @@ -1,6 +1,6 @@ /** @import { ArrowFunctionExpression, FunctionExpression, Node } from 'estree' */ /** @import { ComponentContext } from '../../types' */ -import { build_hoistable_params } from '../../utils.js'; +import { build_hoisted_params } from '../../utils.js'; /** * @param {ArrowFunctionExpression | FunctionExpression} node @@ -20,8 +20,8 @@ export const visit_function = (node, context) => { state = { ...context.state, in_constructor: false }; } - if (metadata?.hoistable === true) { - const params = build_hoistable_params(node, context); + if (metadata?.hoisted === true) { + const params = build_hoisted_params(node, context); return /** @type {FunctionExpression} */ ({ ...node, diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/special_element.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/special_element.js new file mode 100644 index 0000000000..6a0be55949 --- /dev/null +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/special_element.js @@ -0,0 +1,23 @@ +/** @import { Expression } from 'estree' */ +/** @import { SvelteBody, SvelteDocument, SvelteWindow } from '#compiler' */ +/** @import { ComponentContext } from '../../types' */ +import { is_event_attribute } from '../../../../../utils/ast.js'; +import * as b from '../../../../../utils/builders.js'; + +/** + * + * @param {SvelteBody | SvelteDocument | SvelteWindow} node + * @param {string} id + * @param {ComponentContext} context + */ +export function visit_special_element(node, id, context) { + const state = { ...context.state, node: b.id(id) }; + + for (const attribute of node.attributes) { + if (attribute.type === 'OnDirective') { + context.state.init.push(b.stmt(/** @type {Expression} */ (context.visit(attribute, state)))); + } else { + context.visit(attribute, 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 b6b11c6909..04b7b36186 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 @@ -1,5 +1,5 @@ /** @import { Expression, ExpressionStatement, Identifier, MemberExpression, Statement, Super, TemplateElement, TemplateLiteral } from 'estree' */ -/** @import { BindDirective, ExpressionMetadata, ExpressionTag, OnDirective, SvelteNode, Text } from '#compiler' */ +/** @import { BindDirective, DelegatedEvent, ExpressionMetadata, ExpressionTag, OnDirective, SvelteNode, Text } from '#compiler' */ /** @import { ComponentClientTransformState, ComponentContext } from '../../types' */ import { walk } from 'zimmerframe'; import { object } from '../../../../../utils/ast.js'; @@ -132,115 +132,6 @@ export function build_update_assignment(state, id, init, value, update) { ); } -/** - * Serializes the event handler function of the `on:` directive - * @param {Pick} node - * @param {null | ExpressionMetadata} metadata - * @param {ComponentContext} context - */ -export function build_event_handler(node, metadata, { state, visit }) { - /** @type {Expression} */ - let handler; - - if (node.expression) { - handler = node.expression; - - // Event handlers can be dynamic (source/store/prop/conditional etc) - const dynamic_handler = () => - b.function( - null, - [b.rest(b.id('$$args'))], - b.block([ - b.return( - b.call( - b.member(/** @type {Expression} */ (visit(handler)), b.id('apply'), false, true), - b.this, - b.id('$$args') - ) - ) - ]) - ); - - if ( - metadata?.has_call && - !( - (handler.type === 'ArrowFunctionExpression' || handler.type === 'FunctionExpression') && - handler.metadata.hoistable - ) - ) { - // Create a derived dynamic event handler - const id = b.id(state.scope.generate('event_handler')); - - state.init.push( - b.var(id, b.call('$.derived', b.thunk(/** @type {Expression} */ (visit(handler))))) - ); - - handler = b.function( - null, - [b.rest(b.id('$$args'))], - b.block([ - b.return( - b.call( - b.member(b.call('$.get', id), b.id('apply'), false, true), - b.this, - b.id('$$args') - ) - ) - ]) - ); - } else if (handler.type === 'Identifier' || handler.type === 'MemberExpression') { - const id = object(handler); - const binding = id === null ? null : state.scope.get(id.name); - if ( - binding !== null && - (binding.kind === 'state' || - binding.kind === 'frozen_state' || - binding.declaration_kind === 'import' || - binding.kind === 'legacy_reactive' || - binding.kind === 'derived' || - binding.kind === 'prop' || - binding.kind === 'bindable_prop' || - binding.kind === 'store_sub') - ) { - handler = dynamic_handler(); - } else { - handler = /** @type {Expression} */ (visit(handler)); - } - } else if (handler.type === 'ConditionalExpression' || handler.type === 'LogicalExpression') { - handler = dynamic_handler(); - } else { - handler = /** @type {Expression} */ (visit(handler)); - } - } else { - state.analysis.needs_props = true; - - // Function + .call to preserve "this" context as much as possible - handler = b.function( - null, - [b.id('$$arg')], - b.block([b.stmt(b.call('$.bubble_event.call', b.this, b.id('$$props'), b.id('$$arg')))]) - ); - } - - if (node.modifiers.includes('stopPropagation')) { - handler = b.call('$.stopPropagation', handler); - } - if (node.modifiers.includes('stopImmediatePropagation')) { - handler = b.call('$.stopImmediatePropagation', handler); - } - if (node.modifiers.includes('preventDefault')) { - handler = b.call('$.preventDefault', handler); - } - if (node.modifiers.includes('self')) { - handler = b.call('$.self', handler); - } - if (node.modifiers.includes('trusted')) { - handler = b.call('$.trusted', handler); - } - - return handler; -} - /** * Serializes `bind:this` for components and elements. * @param {Identifier | MemberExpression} expression diff --git a/packages/svelte/src/compiler/phases/3-transform/utils.js b/packages/svelte/src/compiler/phases/3-transform/utils.js index 2b35fa67d7..188ae6739f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/utils.js @@ -20,13 +20,13 @@ import { dev } from '../../state.js'; * @param {Node} node * @returns {boolean} */ -export function is_hoistable_function(node) { +export function is_hoisted_function(node) { if ( node.type === 'ArrowFunctionExpression' || node.type === 'FunctionExpression' || node.type === 'FunctionDeclaration' ) { - return node.metadata?.hoistable === true; + return node.metadata?.hoisted === true; } return false; } diff --git a/packages/svelte/src/compiler/phases/types.d.ts b/packages/svelte/src/compiler/phases/types.d.ts index a223834d06..96c0f65eb7 100644 --- a/packages/svelte/src/compiler/phases/types.d.ts +++ b/packages/svelte/src/compiler/phases/types.d.ts @@ -82,24 +82,24 @@ export interface ComponentAnalysis extends Analysis { declare module 'estree' { interface ArrowFunctionExpression { metadata: { - hoistable: boolean | 'impossible'; - hoistable_params: Pattern[]; + hoisted: boolean; + hoisted_params: Pattern[]; scope: Scope; }; } interface FunctionExpression { metadata: { - hoistable: boolean | 'impossible'; - hoistable_params: Pattern[]; + hoisted: boolean; + hoisted_params: Pattern[]; scope: Scope; }; } interface FunctionDeclaration { metadata: { - hoistable: boolean | 'impossible'; - hoistable_params: Pattern[]; + hoisted: boolean; + hoisted_params: Pattern[]; scope: Scope; }; } diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index 373822a873..86341d07d0 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -214,10 +214,10 @@ export interface OnDirective extends BaseNode { export type DelegatedEvent = | { - type: 'hoistable'; + hoisted: true; function: ArrowFunctionExpression | FunctionExpression | FunctionDeclaration; } - | { type: 'non-hoistable' }; + | { hoisted: false }; /** A `style:` directive */ export interface StyleDirective extends BaseNode { diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index cf2e7cc11d..9fd238c866 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -252,7 +252,7 @@ export function legacy_pre_effect(deps, fn) { deps(); // If this legacy pre effect has already run before the end of the reset, then - // bail-out to emulate the same behavior. + // bail out to emulate the same behavior. if (token.ran) return; token.ran = true; diff --git a/packages/svelte/tests/runtime-runes/samples/dynamic-element-event-handler-2/_config.js b/packages/svelte/tests/runtime-runes/samples/dynamic-event-handler-2/_config.js similarity index 100% rename from packages/svelte/tests/runtime-runes/samples/dynamic-element-event-handler-2/_config.js rename to packages/svelte/tests/runtime-runes/samples/dynamic-event-handler-2/_config.js diff --git a/packages/svelte/tests/runtime-runes/samples/dynamic-element-event-handler-2/main.svelte b/packages/svelte/tests/runtime-runes/samples/dynamic-event-handler-2/main.svelte similarity index 100% rename from packages/svelte/tests/runtime-runes/samples/dynamic-element-event-handler-2/main.svelte rename to packages/svelte/tests/runtime-runes/samples/dynamic-event-handler-2/main.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/dynamic-event-handler-3/Button.svelte b/packages/svelte/tests/runtime-runes/samples/dynamic-event-handler-3/Button.svelte new file mode 100644 index 0000000000..8fcd7c705d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/dynamic-event-handler-3/Button.svelte @@ -0,0 +1,3 @@ + diff --git a/packages/svelte/tests/runtime-runes/samples/dynamic-event-handler-3/_config.js b/packages/svelte/tests/runtime-runes/samples/dynamic-event-handler-3/_config.js new file mode 100644 index 0000000000..b87e8c4ce1 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/dynamic-event-handler-3/_config.js @@ -0,0 +1,34 @@ +import { test } from '../../test'; + +export default test({ + test({ assert, logs, target }) { + const [b1, b2, b3] = target.querySelectorAll('button'); + + b2?.click(); + b2?.click(); + b3?.click(); + b3?.click(); + + b1?.click(); + + b2?.click(); + b2?.click(); + b3?.click(); + b3?.click(); + + assert.deepEqual(logs, [ + 'creating handler (1)', + 1, + 2, + 'creating handler (1)', + 3, + 4, + 'creating handler (2)', + 6, + 8, + 'creating handler (2)', + 10, + 12 + ]); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/dynamic-event-handler-3/main.svelte b/packages/svelte/tests/runtime-runes/samples/dynamic-event-handler-3/main.svelte new file mode 100644 index 0000000000..2f7bb13043 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/dynamic-event-handler-3/main.svelte @@ -0,0 +1,27 @@ + + + + + + + diff --git a/packages/svelte/tests/runtime-runes/samples/dynamic-element-event-handler/_config.js b/packages/svelte/tests/runtime-runes/samples/dynamic-event-handler/_config.js similarity index 100% rename from packages/svelte/tests/runtime-runes/samples/dynamic-element-event-handler/_config.js rename to packages/svelte/tests/runtime-runes/samples/dynamic-event-handler/_config.js diff --git a/packages/svelte/tests/runtime-runes/samples/dynamic-element-event-handler/main.svelte b/packages/svelte/tests/runtime-runes/samples/dynamic-event-handler/main.svelte similarity index 100% rename from packages/svelte/tests/runtime-runes/samples/dynamic-element-event-handler/main.svelte rename to packages/svelte/tests/runtime-runes/samples/dynamic-event-handler/main.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/event-attribute-not-reactive/_config.js b/packages/svelte/tests/runtime-runes/samples/event-attribute-not-reactive/_config.js new file mode 100644 index 0000000000..3dfeaa2952 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/event-attribute-not-reactive/_config.js @@ -0,0 +1,19 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + test({ assert, target }) { + const [btn1, btn2, btn3] = target.querySelectorAll('button'); + + flushSync(() => btn3.click()); + assert.htmlEqual(/** @type {string} */ (btn3.textContent), 'clicks: 1'); + + flushSync(() => btn2.click()); + flushSync(() => btn3.click()); + assert.htmlEqual(/** @type {string} */ (btn3.textContent), 'clicks: 0'); + + flushSync(() => btn1.click()); + flushSync(() => btn3.click()); + assert.htmlEqual(/** @type {string} */ (btn3.textContent), 'clicks: 1'); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/event-attribute-not-reactive/main.svelte b/packages/svelte/tests/runtime-runes/samples/event-attribute-not-reactive/main.svelte new file mode 100644 index 0000000000..9de44f23d9 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/event-attribute-not-reactive/main.svelte @@ -0,0 +1,19 @@ + + + + + diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 8212c52cc0..969f89222d 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -1672,10 +1672,10 @@ declare module 'svelte/compiler' { type DelegatedEvent = | { - type: 'hoistable'; + hoisted: true; function: ArrowFunctionExpression | FunctionExpression | FunctionDeclaration; } - | { type: 'non-hoistable' }; + | { hoisted: false }; /** A `style:` directive */ interface StyleDirective extends BaseNode {