From 203462a9d9bc4ca1ca9ff81dfc8908d4b9189200 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Tue, 14 Nov 2023 13:17:35 +0100 Subject: [PATCH] add event delegation to spread_attributes, add event attributes to spread also fixes an edge case bug with event hoistability --- .../compiler/phases/1-parse/state/element.js | 4 +- .../src/compiler/phases/2-analyze/index.js | 92 +++++++++++++++---- .../phases/3-transform/client/types.d.ts | 2 - .../3-transform/client/visitors/template.js | 46 ++++------ .../svelte/src/compiler/phases/constants.js | 26 ------ .../svelte/src/compiler/types/template.d.ts | 8 ++ packages/svelte/src/constants.js | 29 ++++++ packages/svelte/src/internal/client/render.js | 23 ++--- .../Button.svelte | 7 -- .../_config.js | 16 ---- .../main.svelte | 7 -- .../Button.svelte | 7 -- .../_config.js | 59 +++++++++++- .../main.svelte | 22 ++++- 14 files changed, 221 insertions(+), 127 deletions(-) delete mode 100644 packages/svelte/tests/runtime-runes/samples/event-attribute-spread-collision-2/Button.svelte delete mode 100644 packages/svelte/tests/runtime-runes/samples/event-attribute-spread-collision-2/_config.js delete mode 100644 packages/svelte/tests/runtime-runes/samples/event-attribute-spread-collision-2/main.svelte delete mode 100644 packages/svelte/tests/runtime-runes/samples/event-attribute-spread-collision/Button.svelte diff --git a/packages/svelte/src/compiler/phases/1-parse/state/element.js b/packages/svelte/src/compiler/phases/1-parse/state/element.js index 8e69a92c6b..51eac9ca53 100644 --- a/packages/svelte/src/compiler/phases/1-parse/state/element.js +++ b/packages/svelte/src/compiler/phases/1-parse/state/element.js @@ -127,7 +127,9 @@ export default function tag(parser) { attributes: [], fragment: create_fragment(true), metadata: { - svg: false + svg: false, + has_spread: false, + can_delegate_events: null }, parent: null } diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index c1511e3355..c0729211a8 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -11,7 +11,7 @@ import { object } from '../../utils/ast.js'; import * as b from '../../utils/builders.js'; -import { DelegatedEvents, ReservedKeywords, Runes, SVGElements } from '../constants.js'; +import { ReservedKeywords, Runes, SVGElements } from '../constants.js'; import { Scope, ScopeRoot, create_scopes, get_rune, set_scope } from '../scope.js'; import { merge } from '../visitors.js'; import Stylesheet from './css/Stylesheet.js'; @@ -20,6 +20,7 @@ import { warn } from '../../warnings.js'; import check_graph_for_cycles from './utils/check_graph_for_cycles.js'; import { regex_starts_with_newline } from '../patterns.js'; import { create_attribute, is_element_node } from '../nodes.js'; +import { DelegatedEvents } from '../../../constants.js'; /** * @param {import('#compiler').Script | null} script @@ -58,7 +59,7 @@ function get_component_name(filename) { } /** - * @param {Pick} node + * @param {Pick & { type: string }} node * @param {import('./types').Context} context * @returns {null | import('#compiler').DelegatedEvent} */ @@ -70,16 +71,13 @@ function get_delegated_event(node, context) { if (!handler || node.modifiers.includes('capture') || !DelegatedEvents.includes(event_name)) { return null; } - // If we are not working with a RegularElement/SlotElement, then bail-out. + // If we are not working with a RegularElement, then bail-out. const element = context.path.at(-1); - if (element == null || (element.type !== 'RegularElement' && element.type !== 'SlotElement')) { + if (element?.type !== 'RegularElement') { return null; } - // If we have multiple OnDirectives of the same type, bail-out. - if ( - element.attributes.filter((attr) => attr.type === 'OnDirective' && attr.name === event_name) - .length > 1 - ) { + // If element says we can't delegate because we have multiple OnDirectives of the same type, bail-out. + if (!element.metadata.can_delegate_events) { return null; } @@ -89,6 +87,11 @@ function get_delegated_event(node, context) { let target_function = null; let binding = null; + if (node.type === 'Attribute' && element.metadata.has_spread) { + // event attribute becomes part of the dynamic spread array + return non_hoistable; + } + if (handler.type === 'ArrowFunctionExpression' || handler.type === 'FunctionExpression') { target_function = handler; } else if (handler.type === 'Identifier') { @@ -110,7 +113,11 @@ function get_delegated_event(node, context) { : null; if (element) { - if (element.type !== 'RegularElement' && element.type !== 'SlotElement') { + if ( + element.type !== 'RegularElement' || + !determine_element_spread_and_delegatable(element).metadata.can_delegate_events || + (element.metadata.has_spread && node.type === 'Attribute') + ) { return non_hoistable; } } else if (parent.type !== 'FunctionDeclaration' && parent.type !== 'VariableDeclarator') { @@ -772,16 +779,15 @@ const common_visitors = { let name = node.name.slice(2); - if ( - name.endsWith('capture') && - name !== 'ongotpointercapture' && - name !== 'onlostpointercapture' - ) { + if (is_capture_event(name)) { name = name.slice(0, -7); modifiers.push('capture'); } - const delegated_event = get_delegated_event({ name, expression, modifiers }, context); + const delegated_event = get_delegated_event( + { type: node.type, name, expression, modifiers }, + context + ); if (delegated_event !== null) { if (delegated_event.type === 'hoistable') { @@ -950,6 +956,8 @@ const common_visitors = { node.metadata.svg = true; } + determine_element_spread_and_delegatable(node); + // Special case: Move the children of