add event delegation to spread_attributes, add event attributes to spread

also fixes an edge case bug with event hoistability

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
 			  }

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<import('#compiler').OnDirective, 'expression'| 'name' | 'modifiers'>} node
+ * @param {Pick<import('#compiler').OnDirective, 'expression'| 'name' | 'modifiers'> & { 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 <textarea> into a value attribute if they are dynamic
 		if (
 			context.state.options.namespace !== 'foreign' &&
@@ -1005,6 +1013,58 @@ const common_visitors = {
 	}
 };
 
+/**
+ * Check if events on this element can theoretically be delegated. They can if there's no
+ * possibility of an OnDirective and an event attribute on the same element, and if there's
+ * no OnDirectives of the same type (the latter is a bit too strict because `on:click on:click on:keyup`
+ * means that `on:keyup` can be delegated but we gloss over this edge case).
+ * @param {import('#compiler').RegularElement} node
+ */
+function determine_element_spread_and_delegatable(node) {
+	if (typeof node.metadata.can_delegate_events === 'boolean') {
+		return node; // did this already
+	}
+
+	let events = new Map();
+	let has_spread = false;
+	let has_on = false;
+	for (const attribute of node.attributes) {
+		if (
+			attribute.type === 'OnDirective' ||
+			(attribute.type === 'Attribute' && is_event_attribute(attribute))
+		) {
+			let event_name = attribute.name;
+			if (attribute.type === 'Attribute') {
+				if (is_capture_event(event_name)) {
+					event_name = event_name.slice(0, -7);
+				}
+				event_name = event_name.slice(2);
+			}
+			events.set(event_name, (events.get(event_name) || 0) + 1);
+			if (!has_on && attribute.type === 'OnDirective') {
+				has_on = true;
+			}
+		} else if (!has_spread && attribute.type === 'SpreadAttribute') {
+			has_spread = true;
+		}
+	}
+	node.metadata.can_delegate_events =
+		!(has_spread && has_on) && ![...events.values()].some((count) => count > 1);
+	node.metadata.has_spread = has_spread;
+
+	return node;
+}
+
+/**
+ * @param {string} name
+ * @returns boolean
+ */
+function is_capture_event(name) {
+	return (
+		name.endsWith('capture') && name !== 'ongotpointercapture' && name !== 'onlostpointercapture'
+	);
+}
+
 /**
  * @param {Map<import('estree').LabeledStatement, import('../types.js').ReactiveStatement>} unsorted_reactive_declarations
  */

packages/svelte/src/compiler/phases/3-transform/client/types.d.ts

@@ -42,8 +42,6 @@ export interface ComponentClientTransformState extends ClientTransformState {
 		/** Used if condition for singular prop is false (see comment above) */
 		grouped: Statement;
 	}[];
-	/** Stuff that happens just before the render effect (events) */
-	readonly before_update: Statement[];
 	/** Stuff that happens after the render effect (bindings, actions) */
 	readonly after_update: Statement[];
 	/** The HTML template string */

packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js

@@ -954,7 +954,6 @@ function create_block(parent, name, nodes, context) {
 		init: [],
 		update: [],
 		update_effects: [],
-		before_update: [],
 		after_update: [],
 		template: [],
 		metadata: {
@@ -1036,8 +1035,6 @@ function create_block(parent, name, nodes, context) {
 		}
 	}
 
-	body.push(...state.before_update);
-
 	if (state.update.length > 0 || state.update_effects.length > 0) {
 		/** @type {import('estree').Statement | undefined} */
 		let update;
@@ -1223,7 +1220,7 @@ function serialize_event(node, context) {
 				delegated_assignment = handler;
 			}
 
-			state.before_update.push(
+			state.after_update.push(
 				b.stmt(
 					b.assignment(
 						'=',
@@ -1731,7 +1728,6 @@ export const template_visitors = {
 		const is_custom_element = is_custom_element_node(node);
 		let needs_input_reset = false;
 		let needs_content_reset = false;
-		let has_spread = false;
 
 		/** @type {import('#compiler').BindDirective | null} */
 		let value_binding = null;
@@ -1751,9 +1747,6 @@ export const template_visitors = {
 
 		for (const attribute of node.attributes) {
 			if (attribute.type === 'Attribute') {
-				if (is_event_attribute(attribute)) {
-					serialize_event_attribute(attribute, context);
-				} else {
 				attributes.push(attribute);
 				if (
 					(attribute.name === 'value' || attribute.name === 'checked') &&
@@ -1768,10 +1761,8 @@ export const template_visitors = {
 				) {
 					is_content_editable = true;
 				}
-				}
 			} else if (attribute.type === 'SpreadAttribute') {
 				attributes.push(attribute);
-				has_spread = true;
 				needs_input_reset = true;
 				needs_content_reset = true;
 			} else if (attribute.type === 'ClassDirective') {
@@ -1832,7 +1823,7 @@ export const template_visitors = {
 
 		// Then do attributes
 		let is_attributes_reactive = false;
-		if (has_spread) {
+		if (node.metadata.has_spread) {
 			const spread_id = serialize_element_spread_attributes(attributes, context, node_id);
 			if (child_metadata.namespace !== 'foreign') {
 				add_select_to_spread_update(spread_id, node, context, node_id);
@@ -1840,6 +1831,11 @@ export const template_visitors = {
 			is_attributes_reactive = spread_id !== null;
 		} else {
 			for (const attribute of /** @type {import('#compiler').Attribute[]} */ (attributes)) {
+				if (is_event_attribute(attribute)) {
+					serialize_event_attribute(attribute, context);
+					continue;
+				}
+
 				if (needs_special_value_handling && attribute.name === 'value') {
 					serialize_element_special_value_attribute(node.name, node_id, attribute, context);
 					continue;
@@ -1954,7 +1950,6 @@ export const template_visitors = {
 				init: [],
 				update: [],
 				update_effects: [],
-				before_update: [],
 				after_update: []
 			}
 		};
@@ -2002,7 +1997,6 @@ export const template_visitors = {
 
 		/** @type {import('estree').Statement[]} */
 		const inner = inner_context.state.init;
-		inner.push(...inner_context.state.before_update);
 		if (inner_context.state.update.length > 0 || inner_context.state.update_effects.length > 0) {
 			if (inner_context.state.update_effects.length > 0) {
 				for (const render of inner_context.state.update_effects) {

packages/svelte/src/compiler/phases/constants.js

@@ -66,32 +66,6 @@ export const VoidElements = [
 	'wbr'
 ];
 
-// list of Element events that will be delegated
-export const DelegatedEvents = [
-	'beforeinput',
-	'click',
-	'dblclick',
-	'contextmenu',
-	'focusin',
-	'focusout',
-	// 'input', This conflicts with bind:input
-	'keydown',
-	'keyup',
-	'mousedown',
-	'mousemove',
-	'mouseout',
-	'mouseover',
-	'mouseup',
-	'pointerdown',
-	'pointermove',
-	'pointerout',
-	'pointerover',
-	'pointerup',
-	'touchend',
-	'touchmove',
-	'touchstart'
-];
-
 export const PassiveEvents = ['wheel', 'touchstart', 'touchmove', 'touchend', 'touchcancel'];
 
 // TODO this is currently unused

packages/svelte/src/compiler/types/template.d.ts

@@ -283,6 +283,14 @@ export interface RegularElement extends BaseElement {
 	metadata: {
 		/** `true` if this is an svg element */
 		svg: boolean;
+		/** `true` if contains a SpreadAttribute */
+		has_spread: boolean;
+		/**
+		 * `true` if events on this element can theoretically be delegated. This doesn't necessarily mean that
+		 * a specific event will be delegated, as there are other factors which affect the final outcome.
+		 * `null` only until it was determined whether this element can be delegated or not.
+		 */
+		can_delegate_events: boolean | null;
 	};
 }
 

packages/svelte/src/constants.js

@@ -3,3 +3,32 @@ export const EACH_INDEX_REACTIVE = 1 << 1;
 export const EACH_KEYED = 1 << 2;
 export const EACH_IS_CONTROLLED = 1 << 3;
 export const EACH_IS_ANIMATED = 1 << 4;
+
+/** List of Element events that will be delegated */
+export const DelegatedEvents = [
+	'beforeinput',
+	'click',
+	'dblclick',
+	'contextmenu',
+	'focusin',
+	'focusout',
+	// 'input', This conflicts with bind:input
+	'keydown',
+	'keyup',
+	'mousedown',
+	'mousemove',
+	'mouseout',
+	'mouseover',
+	'mouseup',
+	'pointerdown',
+	'pointermove',
+	'pointerout',
+	'pointerover',
+	'pointerup',
+	'touchend',
+	'touchmove',
+	'touchstart'
+];
+
+/** List of Element events that will be delegated and are passive */
+export const PassiveDelegatedEvents = ['touchstart', 'touchmove', 'touchend'];

packages/svelte/src/internal/client/render.js

@@ -25,7 +25,9 @@ import {
 	EACH_IS_CONTROLLED,
 	EACH_INDEX_REACTIVE,
 	EACH_ITEM_REACTIVE,
-	EACH_IS_ANIMATED
+	EACH_IS_ANIMATED,
+	PassiveDelegatedEvents,
+	DelegatedEvents
 } from '../../constants.js';
 import {
 	create_fragment_from_html,
@@ -77,7 +79,6 @@ const all_registerd_events = new Set();
 
 /** @type {Set<(events: Array<string>) => void>} */
 const root_event_handles = new Set();
-const passive_delegated_events = new Set(['touchstart', 'touchmove', 'touchend']);
 
 /** @returns {Text} */
 function empty() {
@@ -2864,10 +2865,10 @@ export function spread_attributes(dom, prev, attrs, css_hash) {
 		if (prefix === '$$') continue;
 
 		if (prefix === 'on') {
-			// TODO delegate?
 			/** @type {{ capture?: true }} */
 			const opts = {};
 			let event_name = key.slice(2).toLowerCase();
+			const delegated = DelegatedEvents.includes(event_name);
 
 			if (
 				event_name.endsWith('capture') &&
@@ -2877,17 +2878,17 @@ export function spread_attributes(dom, prev, attrs, css_hash) {
 				event_name = event_name.slice(0, -7);
 				opts.capture = true;
 			}
-			if (prev?.[key]) {
+			if (!delegated && prev?.[key]) {
 				dom.removeEventListener(event_name, /** @type {any} */ (prev[key]), opts);
 			}
 			if (value != null) {
-				const delegated_property = '__' + event_name;
-				// @ts-expect-error internal event property
-				if (dom[delegated_property]) {
-					// @ts-expect-error internal event property
-					dom[delegated_property] = undefined;
-				}
+				if (!delegated) {
 					dom.addEventListener(event_name, value, opts);
+				} else {
+					// @ts-ignore
+					dom[`__${event_name}`] = value;
+					delegate([event_name]);
+				}
 			}
 		} else if (value == null) {
 			dom.removeAttribute(key);
@@ -3237,7 +3238,7 @@ export function mount(component, options) {
 				container.addEventListener(
 					event_name,
 					bound_event_listener,
-					passive_delegated_events.has(event_name)
+					PassiveDelegatedEvents.includes(event_name)
 						? {
 								passive: true
 						  }

packages/svelte/tests/runtime-runes/samples/event-attribute-spread-collision-2/Button.svelte

@@ -1,7 +0,0 @@
-<script>
-	const {log, ...restProps} = $props();
-</script>
-
-<button onclick={() => log.push('internal!')} {...restProps}>
-	<slot />
-</button>

packages/svelte/tests/runtime-runes/samples/event-attribute-spread-collision-2/_config.js

@@ -1,16 +0,0 @@
-import { flushSync } from 'svelte';
-import { test } from '../../test';
-
-export default test({
-	html: `<button>Test</button>`,
-
-	async test({ assert, target, component }) {
-		const [b1] = target.querySelectorAll('button');
-
-		flushSync(() => {
-			b1?.click();
-		});
-
-		assert.deepEqual(component.log, ['external!']);
-	}
-});

packages/svelte/tests/runtime-runes/samples/event-attribute-spread-collision-2/main.svelte

@@ -1,7 +0,0 @@
-<script>
-	import Button from "./Button.svelte";
-
-	const {log = []} = $props();
-</script>
-
-<Button log={log} onclick={() => log.push('external!')}>Test</Button>

packages/svelte/tests/runtime-runes/samples/event-attribute-spread-collision/Button.svelte

@@ -1,7 +0,0 @@
-<script>
-	const {log, children, ...restProps} = $props();
-</script>
-
-<button onclick={() => log.push('internal!')} {...restProps}>
-	{@render children()}
-</button>

packages/svelte/tests/runtime-runes/samples/event-attribute-spread-collision/_config.js

@@ -2,15 +2,66 @@ import { flushSync } from 'svelte';
 import { test } from '../../test';
 
 export default test({
-	html: `<button>Test</button>`,
+	html: `
+	<button>click me</button>
+	<button>click me</button>
+	<button>click me</button>
+	<button>click me</button>
+	`,
 
-	async test({ assert, target, component }) {
-		const [b1] = target.querySelectorAll('button');
+	async test({ assert, target }) {
+		const [b1, b2, b3, b4] = target.querySelectorAll('button');
 
 		flushSync(() => {
 			b1?.click();
 		});
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<button>click spread</button>
+			<button>click spread</button>
+			<button>click spread</button>
+			<button>click spread</button>
+			`
+		);
 
-		assert.deepEqual(component.log, ['external!']);
+		flushSync(() => {
+			b2?.click();
+		});
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<button>click onclick</button>
+			<button>click onclick</button>
+			<button>click onclick</button>
+			<button>click onclick</button>
+			`
+		);
+
+		flushSync(() => {
+			b3?.click();
+		});
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<button>click spread</button>
+			<button>click spread</button>
+			<button>click spread!</button>
+			<button>click spread!</button>
+			`
+		);
+
+		flushSync(() => {
+			b4?.click();
+		});
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<button>click onclick</button>
+			<button>click onclick</button>
+			<button>click onclick?</button>
+			<button>click onclick?</button>
+			`
+		);
 	}
 });

packages/svelte/tests/runtime-runes/samples/event-attribute-spread-collision/main.svelte

@@ -1,7 +1,21 @@
 <script>
-	import Button from "./Button.svelte";
-
-	const {log = []} = $props();
+	let text = $state('click me');
+	let text2 = $state('');
+	let spread = { onclick: () => text = 'click spread' };
 </script>
 
-<Button log={log} onclick={() => log.push('external!')}>Test</Button>
+<button onclick={() => text = 'click onclick'} {...spread}>
+	{text}
+</button>
+
+<button {...spread} onclick={() => text = 'click onclick'}>
+	{text}
+</button>
+
+<button onclick={() => text = 'click onclick'} {...spread} on:click={() => text2 = '!'}>
+	{text}{text2}
+</button>
+
+<button on:click={() => text2 = '?'} {...spread} onclick={() => text = 'click onclick'}>
+	{text}{text2}
+</button>