From c05abb5e54f578c7bf260d3721d8dd2c3ffb5d51 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 6 Jan 2025 15:53:05 +0100 Subject: [PATCH] fix: don't delegate events on custom elements, solve edge case stopPropagation issue - don't delegate events on custom elements - still invoke listener for cancelled event on the element where it was cancelled: when you do `stopPropagation`, `event.cancelBubble` becomes `true`. We can't use this as an indicator to not invoke a listener directly, because the listner could be on the element where propagation was cancelled, i.e. it should still run for that listener. Instead, adjust the event propagation algorithm to detect when a delegated event listener caused the event to be cancelled fixes #14704 --- .changeset/kind-wombats-jam.md | 5 +++++ .changeset/six-steaks-provide.md | 5 +++++ .../phases/2-analyze/visitors/Attribute.js | 11 +++++++++-- .../client/dom/elements/attributes.js | 3 ++- .../internal/client/dom/elements/events.js | 12 +++++++++++- .../_config.js | 18 ++++++++++++++++++ .../main.svelte | 19 +++++++++++++++++++ 7 files changed, 69 insertions(+), 4 deletions(-) create mode 100644 .changeset/kind-wombats-jam.md create mode 100644 .changeset/six-steaks-provide.md create mode 100644 packages/svelte/tests/runtime-runes/samples/event-attribute-delegation-custom-elements/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/event-attribute-delegation-custom-elements/main.svelte diff --git a/.changeset/kind-wombats-jam.md b/.changeset/kind-wombats-jam.md new file mode 100644 index 000000000000..ff872aa64529 --- /dev/null +++ b/.changeset/kind-wombats-jam.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: don't delegate events on custom elements diff --git a/.changeset/six-steaks-provide.md b/.changeset/six-steaks-provide.md new file mode 100644 index 000000000000..3459c832d0d7 --- /dev/null +++ b/.changeset/six-steaks-provide.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: still invoke listener for cancelled event on the element where it was cancelled 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 9d801e095e8d..515871032fa3 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/Attribute.js @@ -7,6 +7,7 @@ import { get_attribute_expression, is_event_attribute } from '../../../utils/ast.js'; +import { is_custom_element_node } from '../../nodes.js'; import { mark_subtree_dynamic } from './shared/fragment.js'; /** @@ -67,7 +68,6 @@ export function Attribute(node, context) { } if (is_event_attribute(node)) { - const parent = context.path.at(-1); if (parent?.type === 'RegularElement' || parent?.type === 'SvelteElement') { context.state.analysis.uses_event_attributes = true; } @@ -75,7 +75,14 @@ export function Attribute(node, context) { const expression = get_attribute_expression(node); const delegated_event = get_delegated_event(node.name.slice(2), expression, context); - if (delegated_event !== null) { + if ( + delegated_event !== null && + // We can't assume that the events from within the shadow root bubble beyond it. + // If someone dispatches them without the composed option, they won't. Also + // people could repurpose the event names to do something else, or call stopPropagation + // on the shadow root so it doesn't bubble beyond it. + !(parent?.type === 'RegularElement' && is_custom_element_node(parent)) + ) { if (delegated_event.hoisted) { delegated_event.function.metadata.hoisted = true; } diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index 6656532986d7..c91a4e018f36 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -318,7 +318,8 @@ export function set_attributes( const opts = {}; const event_handle_key = '$$' + key; let event_name = key.slice(2); - var delegated = is_delegated(event_name); + // Events on custom elements can be anything, we can't assume they bubble + var delegated = !is_custom_element && is_delegated(event_name); if (is_capture_event(event_name)) { event_name = event_name.slice(0, -7); diff --git a/packages/svelte/src/internal/client/dom/elements/events.js b/packages/svelte/src/internal/client/dom/elements/events.js index f2038f96ada3..9af6040bbb39 100644 --- a/packages/svelte/src/internal/client/dom/elements/events.js +++ b/packages/svelte/src/internal/client/dom/elements/events.js @@ -61,7 +61,10 @@ export function create_event(event_name, dom, handler, options) { // Only call in the bubble phase, else delegated events would be called before the capturing events handle_event_propagation.call(dom, event); } - if (!event.cancelBubble) { + + // @ts-expect-error Use this instead of cancelBubble, because cancelBubble is also true if + // we're the last element on which the event will be handled. + if (!event.__cancelled || event.__cancelled === dom) { return without_reactive_context(() => { return handler.call(this, event); }); @@ -171,6 +174,8 @@ export function handle_event_propagation(event) { // chain in case someone manually dispatches the same event object again. // @ts-expect-error event.__root = handler_element; + // @ts-expect-error + event.__cancelled = null; return; } @@ -216,6 +221,7 @@ export function handle_event_propagation(event) { set_active_effect(null); try { + var cancelled = event.cancelBubble; /** * @type {unknown} */ @@ -253,6 +259,10 @@ export function handle_event_propagation(event) { } } if (event.cancelBubble || parent_element === handler_element || parent_element === null) { + if (!cancelled && event.cancelBubble) { + // @ts-expect-error + event.__cancelled = current_target; + } break; } current_target = parent_element; diff --git a/packages/svelte/tests/runtime-runes/samples/event-attribute-delegation-custom-elements/_config.js b/packages/svelte/tests/runtime-runes/samples/event-attribute-delegation-custom-elements/_config.js new file mode 100644 index 000000000000..15d3395b7af7 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/event-attribute-delegation-custom-elements/_config.js @@ -0,0 +1,18 @@ +import { test } from '../../test'; + +export default test({ + mode: ['client'], + async test({ assert, target, logs }) { + const [btn1, btn2] = [...target.querySelectorAll('custom-element')].map((c) => + c.shadowRoot?.querySelector('button') + ); + + btn1?.click(); + await Promise.resolve(); + assert.deepEqual(logs, ['reached shadow root1']); + + btn2?.click(); + await Promise.resolve(); + assert.deepEqual(logs, ['reached shadow root1', 'reached shadow root2']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/event-attribute-delegation-custom-elements/main.svelte b/packages/svelte/tests/runtime-runes/samples/event-attribute-delegation-custom-elements/main.svelte new file mode 100644 index 000000000000..2f5e6afe6ca1 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/event-attribute-delegation-custom-elements/main.svelte @@ -0,0 +1,19 @@ + + +
console.log('bubbled beyond shadow root')}> + console.log('reached shadow root1')}> + console.log('reached shadow root2')}}> +