diff --git a/.changeset/seven-garlics-serve.md b/.changeset/seven-garlics-serve.md new file mode 100644 index 000000000..4c56f95b4 --- /dev/null +++ b/.changeset/seven-garlics-serve.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +fix: make sure event attributes run after bindings diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index 79a161c77..fa711c324 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -263,7 +263,7 @@ function serialize_element_spread_attributes( ) { let needs_isolation = false; - /** @type {import('estree').Expression[]} */ + /** @type {import('estree').ObjectExpression['properties']} */ const values = []; for (const attribute of attributes) { @@ -271,9 +271,21 @@ function serialize_element_spread_attributes( const name = get_attribute_name(element, attribute, context); // TODO: handle contains_call_expression const [, value] = serialize_attribute_value(attribute.value, context); - values.push(b.object([b.init(name, value)])); + + if ( + is_event_attribute(attribute) && + (attribute.value[0].expression.type === 'ArrowFunctionExpression' || + attribute.value[0].expression.type === 'FunctionExpression') + ) { + // Give the event handler a stable ID so it isn't removed and readded on every update + const id = context.state.scope.generate('event_handler'); + context.state.init.push(b.var(id, value)); + values.push(b.init(attribute.name, b.id(id))); + } else { + values.push(b.init(name, value)); + } } else { - values.push(/** @type {import('estree').Expression} */ (context.visit(attribute))); + values.push(b.spread(/** @type {import('estree').Expression} */ (context.visit(attribute)))); } needs_isolation ||= @@ -292,7 +304,7 @@ function serialize_element_spread_attributes( '$.set_attributes', element_id, b.id(id), - b.array(values), + b.object(values), lowercase_attributes, b.literal(context.state.analysis.css.hash) ) @@ -350,15 +362,27 @@ function serialize_dynamic_element_attributes(attributes, context, element_id) { let needs_isolation = false; let is_reactive = false; - /** @type {import('estree').Expression[]} */ + /** @type {import('estree').ObjectExpression['properties']} */ const values = []; for (const attribute of attributes) { if (attribute.type === 'Attribute') { const [, value] = serialize_attribute_value(attribute.value, context); - values.push(b.object([b.init(attribute.name, value)])); + + if ( + is_event_attribute(attribute) && + (attribute.value[0].expression.type === 'ArrowFunctionExpression' || + attribute.value[0].expression.type === 'FunctionExpression') + ) { + // Give the event handler a stable ID so it isn't removed and readded on every update + const id = context.state.scope.generate('event_handler'); + context.state.init.push(b.var(id, value)); + values.push(b.init(attribute.name, b.id(id))); + } else { + values.push(b.init(attribute.name, value)); + } } else { - values.push(/** @type {import('estree').Expression} */ (context.visit(attribute))); + values.push(b.spread(/** @type {import('estree').Expression} */ (context.visit(attribute)))); } is_reactive ||= @@ -381,7 +405,7 @@ function serialize_dynamic_element_attributes(attributes, context, element_id) { '$.set_dynamic_element_attributes', element_id, b.id(id), - b.array(values), + b.object(values), b.literal(context.state.analysis.css.hash) ) ) @@ -402,7 +426,7 @@ function serialize_dynamic_element_attributes(attributes, context, element_id) { '$.set_dynamic_element_attributes', element_id, b.literal(null), - b.array(values), + b.object(values), b.literal(context.state.analysis.css.hash) ) ) diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index 94ea12be9..67e336744 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -4,6 +4,8 @@ import { get_descriptors, map_get, map_set, object_assign } from '../../utils.js import { AttributeAliases, DelegatedEvents, namespace_svg } from '../../../../constants.js'; import { delegate } from './events.js'; import { autofocus } from './misc.js'; +import { effect } from '../../reactivity/effects.js'; +import { run } from '../../../shared/utils.js'; /** * The value/checked attribute in the template actually corresponds to the defaultValue property, so we need @@ -81,14 +83,13 @@ export function set_custom_element_data(node, prop, value) { /** * Spreads attributes onto a DOM element, taking into account the currently set attributes * @param {Element & ElementCSSInlineStyle} element - * @param {Record | undefined} prev - * @param {Record[]} attrs + * @param {Record | undefined} prev + * @param {Record} next New attributes - this function mutates this object * @param {boolean} lowercase_attributes * @param {string} css_hash - * @returns {Record} + * @returns {Record} */ -export function set_attributes(element, prev, attrs, lowercase_attributes, css_hash) { - var next = object_assign({}, ...attrs); +export function set_attributes(element, prev, next, lowercase_attributes, css_hash) { var has_hash = css_hash.length !== 0; for (var key in prev) { @@ -106,6 +107,8 @@ export function set_attributes(element, prev, attrs, lowercase_attributes, css_h // @ts-expect-error var attributes = /** @type {Record} **/ (element.__attributes ??= {}); + /** @type {Array<() => void>} */ + var events = []; for (key in next) { var value = next[key]; @@ -135,7 +138,11 @@ export function set_attributes(element, prev, attrs, lowercase_attributes, css_h if (value != null) { if (!delegated) { - element.addEventListener(event_name, value, opts); + if (!prev) { + events.push(() => element.addEventListener(event_name, value, opts)); + } else { + element.addEventListener(event_name, value, opts); + } } else { // @ts-ignore element[`__${event_name}`] = value; @@ -177,19 +184,23 @@ export function set_attributes(element, prev, attrs, lowercase_attributes, css_h } } + // On the first run, ensure that events are added after bindings so + // that their listeners fire after the binding listeners + if (!prev) { + effect(() => events.forEach(run)); + } + return next; } /** * @param {Element} node - * @param {Record | undefined} prev - * @param {Record[]} attrs + * @param {Record | undefined} prev + * @param {Record} next The new attributes - this function mutates this object * @param {string} css_hash */ -export function set_dynamic_element_attributes(node, prev, attrs, css_hash) { +export function set_dynamic_element_attributes(node, prev, next, css_hash) { if (node.tagName.includes('-')) { - var next = object_assign({}, ...attrs); - for (var key in prev) { if (!(key in next)) { next[key] = null; @@ -206,7 +217,7 @@ export function set_dynamic_element_attributes(node, prev, attrs, css_hash) { return set_attributes( /** @type {Element & ElementCSSInlineStyle} */ (node), prev, - attrs, + next, node.namespaceURI !== namespace_svg, css_hash ); diff --git a/packages/svelte/tests/runtime-runes/samples/event-attribute-after-binding/_config.js b/packages/svelte/tests/runtime-runes/samples/event-attribute-after-binding/_config.js new file mode 100644 index 000000000..e5ec3698e --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/event-attribute-after-binding/_config.js @@ -0,0 +1,21 @@ +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + const [i1, i2] = target.querySelectorAll('input'); + + i1?.click(); + await Promise.resolve(); + assert.htmlEqual( + target.innerHTML, + 'true true false false ' + ); + + i2?.click(); + await Promise.resolve(); + assert.htmlEqual( + target.innerHTML, + 'true true true true ' + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/event-attribute-after-binding/main.svelte b/packages/svelte/tests/runtime-runes/samples/event-attribute-after-binding/main.svelte new file mode 100644 index 000000000..5446c2a18 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/event-attribute-after-binding/main.svelte @@ -0,0 +1,15 @@ + + +{checked_simple} {checked_simple_copy} + {checked_simple_copy = checked_simple}} bind:checked={checked_simple} /> + +{checked_rest} {checked_rest_copy} + + {checked_rest_copy = checked_rest}} {...rest()} bind:checked={checked_rest} />