diff --git a/.changeset/sharp-kids-happen.md b/.changeset/sharp-kids-happen.md new file mode 100644 index 0000000000..88b5badb15 --- /dev/null +++ b/.changeset/sharp-kids-happen.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: always treat spread attributes as reactive and separate them if needed 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 30461ca169..3de77c8303 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 @@ -289,7 +289,7 @@ function setup_select_synchronization(value_binding, context) { * value = $.spread_attributes(element, value, [...]) * }); * ``` - * Returns the id of the spread_attribute variable if spread is deemed reactive, `null` otherwise. + * Returns the id of the spread_attribute variable if spread isn't isolated, `null` otherwise. * @param {Array} attributes * @param {import('../types.js').ComponentContext} context * @param {import('#compiler').RegularElement} element @@ -297,7 +297,7 @@ function setup_select_synchronization(value_binding, context) { * @returns {string | null} */ function serialize_element_spread_attributes(attributes, context, element, element_id) { - let is_reactive = false; + let needs_isolation = false; /** @type {import('estree').Expression[]} */ const values = []; @@ -312,18 +312,32 @@ function serialize_element_spread_attributes(attributes, context, element, eleme values.push(/** @type {import('estree').Expression} */ (context.visit(attribute))); } - is_reactive ||= - attribute.metadata.dynamic || - (attribute.type === 'SpreadAttribute' && attribute.metadata.contains_call_expression); + needs_isolation ||= + attribute.type === 'SpreadAttribute' && attribute.metadata.contains_call_expression; } const lowercase_attributes = element.metadata.svg || is_custom_element_node(element) ? b.false : b.true; - if (is_reactive) { + const isolated = b.stmt( + b.call( + '$.spread_attributes_effect', + element_id, + b.thunk(b.array(values)), + lowercase_attributes, + b.literal(context.state.analysis.stylesheet.id) + ) + ); + + // objects could contain reactive getters -> play it safe and always assume spread attributes are reactive + if (needs_isolation) { + context.state.update_effects.push(isolated); + return null; + } else { const id = context.state.scope.generate('spread_attributes'); - context.state.init.push(b.let(id, undefined)); + context.state.init.push(b.let(id)); context.state.update.push({ + singular: isolated, grouped: b.stmt( b.assignment( '=', @@ -340,20 +354,6 @@ function serialize_element_spread_attributes(attributes, context, element, eleme ) }); return id; - } else { - context.state.init.push( - b.stmt( - b.call( - '$.spread_attributes', - element_id, - b.literal(null), - b.array(values), - lowercase_attributes, - b.literal(context.state.analysis.stylesheet.id) - ) - ) - ); - return null; } } @@ -365,7 +365,7 @@ function serialize_element_spread_attributes(attributes, context, element, eleme * @param {import('estree').Identifier} element_id * @returns {boolean} */ -function serialize_dynamic_element_spread_attributes(attributes, context, element_id) { +function serialize_dynamic_element_attributes(attributes, context, element_id) { if (attributes.length === 0) { if (context.state.analysis.stylesheet.id) { context.state.init.push( @@ -375,6 +375,7 @@ function serialize_dynamic_element_spread_attributes(attributes, context, elemen return false; } + let needs_isolation = false; let is_reactive = false; /** @type {import('estree').Expression[]} */ @@ -388,13 +389,31 @@ function serialize_dynamic_element_spread_attributes(attributes, context, elemen values.push(/** @type {import('estree').Expression} */ (context.visit(attribute))); } - is_reactive ||= attribute.metadata.dynamic; + is_reactive ||= + attribute.metadata.dynamic || + // objects could contain reactive getters -> play it safe and always assume spread attributes are reactive + attribute.type === 'SpreadAttribute'; + needs_isolation ||= + attribute.type === 'SpreadAttribute' && attribute.metadata.contains_call_expression; } - if (is_reactive) { + const isolated = b.stmt( + b.call( + '$.spread_dynamic_element_attributes_effect', + element_id, + b.thunk(b.array(values)), + b.literal(context.state.analysis.stylesheet.id) + ) + ); + + if (needs_isolation) { + context.state.update_effects.push(isolated); + return false; + } else if (is_reactive) { const id = context.state.scope.generate('spread_attributes'); context.state.init.push(b.let(id)); context.state.update.push({ + singular: isolated, grouped: b.stmt( b.assignment( '=', @@ -2101,7 +2120,7 @@ export const template_visitors = { // Always use spread because we don't know whether the element is a custom element or not, // therefore we need to do the "how to set an attribute" logic at runtime. const is_attributes_reactive = - serialize_dynamic_element_spread_attributes(attributes, inner_context, element_id) !== null; + serialize_dynamic_element_attributes(attributes, inner_context, element_id) !== null; // class/style directives must be applied last since they could override class/style attributes serialize_class_directives(class_directives, element_id, inner_context, is_attributes_reactive); diff --git a/packages/svelte/src/internal/client/render.js b/packages/svelte/src/internal/client/render.js index 06bf9d406c..301a7480c9 100644 --- a/packages/svelte/src/internal/client/render.js +++ b/packages/svelte/src/internal/client/render.js @@ -2428,10 +2428,26 @@ function get_setters(element) { return setters; } +/** + * Like `spread_attributes` but self-contained + * @param {Element & ElementCSSInlineStyle} dom + * @param {() => Record[]} attrs + * @param {boolean} lowercase_attributes + * @param {string} css_hash + */ +export function spread_attributes_effect(dom, attrs, lowercase_attributes, css_hash) { + /** @type {Record | undefined} */ + let current = undefined; + + render_effect(() => { + current = spread_attributes(dom, current, attrs(), lowercase_attributes, css_hash); + }); +} + /** * Spreads attributes onto a DOM element, taking into account the currently set attributes * @param {Element & ElementCSSInlineStyle} dom - * @param {Record | null} prev + * @param {Record | undefined} prev * @param {Record[]} attrs * @param {boolean} lowercase_attributes * @param {string} css_hash @@ -2528,18 +2544,30 @@ export function spread_attributes(dom, prev, attrs, lowercase_attributes, css_ha /** * @param {Element} node - * @param {Record | null} prev + * @param {() => Record[]} attrs + * @param {string} css_hash + */ +export function spread_dynamic_element_attributes_effect(node, attrs, css_hash) { + /** @type {Record | undefined} */ + let current = undefined; + + render_effect(() => { + current = spread_dynamic_element_attributes(node, current, attrs(), css_hash); + }); +} + +/** + * @param {Element} node + * @param {Record | undefined} prev * @param {Record[]} attrs * @param {string} css_hash */ export function spread_dynamic_element_attributes(node, prev, attrs, css_hash) { if (node.tagName.includes('-')) { const next = object_assign({}, ...attrs); - if (prev !== null) { - for (const key in prev) { - if (!(key in next)) { - next[key] = null; - } + for (const key in prev) { + if (!(key in next)) { + next[key] = null; } } for (const key in next) { diff --git a/packages/svelte/tests/runtime-runes/samples/attribute-spread-call-expression/_config.js b/packages/svelte/tests/runtime-runes/samples/attribute-spread-call-expression/_config.js new file mode 100644 index 0000000000..8b25b7ad43 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/attribute-spread-call-expression/_config.js @@ -0,0 +1,62 @@ +import { test } from '../../test'; + +export default test({ + html: ` + + + + + `, + + async test({ assert, target }) { + const [b1, b2, b3, b4] = target.querySelectorAll('button'); + + b1?.click(); + await Promise.resolve(); + assert.htmlEqual( + target.innerHTML, + ` + + + + + ` + ); + + b2?.click(); + await Promise.resolve(); + assert.htmlEqual( + target.innerHTML, + ` + + + + + ` + ); + + b3?.click(); + await Promise.resolve(); + assert.htmlEqual( + target.innerHTML, + ` + + + + + ` + ); + + b4?.click(); + await Promise.resolve(); + assert.htmlEqual( + target.innerHTML, + ` + + + + + ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/attribute-spread-call-expression/main.svelte b/packages/svelte/tests/runtime-runes/samples/attribute-spread-call-expression/main.svelte new file mode 100644 index 0000000000..4a239a7e09 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/attribute-spread-call-expression/main.svelte @@ -0,0 +1,24 @@ + + + + + +{values.c} +{values.d} diff --git a/packages/svelte/tests/runtime-runes/samples/attribute-spread-reactivitiy/_config.js b/packages/svelte/tests/runtime-runes/samples/attribute-spread-reactivitiy/_config.js new file mode 100644 index 0000000000..9baec30b4f --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/attribute-spread-reactivitiy/_config.js @@ -0,0 +1,24 @@ +import { test } from '../../test'; + +export default test({ + html: ` +
+
+
+
+ let value = $state('red'); + let tag = $state('div'); const getValue = () => { return value; @@ -10,9 +11,19 @@ const getSpread = () => { return { class: value }; } + const props = { + get class() { + return value; + } + }
- +
+ + + + + diff --git a/packages/svelte/tests/runtime-runes/samples/event-attribute-call-expressions/_config.js b/packages/svelte/tests/runtime-runes/samples/event-attribute-call-expressions/_config.js deleted file mode 100644 index e9dae16277..0000000000 --- a/packages/svelte/tests/runtime-runes/samples/event-attribute-call-expressions/_config.js +++ /dev/null @@ -1,16 +0,0 @@ -import { test } from '../../test'; - -export default test({ - html: `
' - ); - } -});