diff --git a/.changeset/soft-jobs-sip.md b/.changeset/soft-jobs-sip.md new file mode 100644 index 000000000000..fcee398d45db --- /dev/null +++ b/.changeset/soft-jobs-sip.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: avoid rerunning attachments when unrelated spread attributes change diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index c1806b5128df..e82379299315 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -17,7 +17,7 @@ import { build_getter } from '../utils.js'; import { get_attribute_name, build_attribute_value, - build_set_attributes, + build_attribute_effect, build_set_class, build_set_style } from './shared/element.js'; @@ -201,37 +201,7 @@ export function RegularElement(node, context) { const node_id = context.state.node; if (has_spread) { - const attributes_id = b.id(context.state.scope.generate('attributes')); - - build_set_attributes( - attributes, - class_directives, - style_directives, - context, - node, - node_id, - attributes_id - ); - - // If value binding exists, that one takes care of calling $.init_select - if (node.name === 'select' && !bindings.has('value')) { - context.state.init.push( - b.stmt(b.call('$.init_select', node_id, b.thunk(b.member(attributes_id, 'value')))) - ); - - context.state.update.push( - b.if( - b.binary('in', b.literal('value'), attributes_id), - b.block([ - // This ensures a one-way street to the DOM in case it's . We need it in addition to $.init_select - // because the select value is not reflected as an attribute, so the - // mutation observer wouldn't notice. - b.stmt(b.call('$.select_option', node_id, b.member(attributes_id, 'value'))) - ]) - ) - ); - } + build_attribute_effect(attributes, class_directives, style_directives, context, node, node_id); } else { /** If true, needs `__value` for inputs */ const needs_special_value_handling = @@ -290,7 +260,8 @@ export function RegularElement(node, context) { const { value, has_state } = build_attribute_value( attribute.value, context, - (value, metadata) => (metadata.has_call ? get_expression_id(context.state, value) : value) + (value, metadata) => + metadata.has_call ? get_expression_id(context.state.expressions, value) : value ); const update = build_element_attribute_update(node, node_id, name, value, attributes); @@ -482,10 +453,11 @@ function setup_select_synchronization(value_binding, context) { /** * @param {AST.ClassDirective[]} class_directives + * @param {Expression[]} expressions * @param {ComponentContext} context * @return {ObjectExpression | Identifier} */ -export function build_class_directives_object(class_directives, context) { +export function build_class_directives_object(class_directives, expressions, context) { let properties = []; let has_call_or_state = false; @@ -497,15 +469,16 @@ export function build_class_directives_object(class_directives, context) { const directives = b.object(properties); - return has_call_or_state ? get_expression_id(context.state, directives) : directives; + return has_call_or_state ? get_expression_id(expressions, directives) : directives; } /** * @param {AST.StyleDirective[]} style_directives + * @param {Expression[]} expressions * @param {ComponentContext} context * @return {ObjectExpression | ArrayExpression}} */ -export function build_style_directives_object(style_directives, context) { +export function build_style_directives_object(style_directives, expressions, context) { let normal_properties = []; let important_properties = []; @@ -514,7 +487,7 @@ export function build_style_directives_object(style_directives, context) { directive.value === true ? build_getter({ name: directive.name, type: 'Identifier' }, context.state) : build_attribute_value(directive.value, context, (value, metadata) => - metadata.has_call ? get_expression_id(context.state, value) : value + metadata.has_call ? get_expression_id(expressions, value) : value ).value; const property = b.init(directive.name, expression); @@ -653,7 +626,7 @@ function build_element_special_value_attribute(element, node_id, attribute, cont ? // if is a select with value we will also invoke `init_select` which need a reference before the template effect so we memoize separately is_select_with_value ? memoize_expression(state, value) - : get_expression_id(state, value) + : get_expression_id(state.expressions, value) : value ); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js index 7e97035e0017..7381553dbe02 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js @@ -5,7 +5,11 @@ import { dev, locator } from '../../../../state.js'; import { is_text_attribute } from '../../../../utils/ast.js'; import * as b from '#compiler/builders'; import { determine_namespace_for_children } from '../../utils.js'; -import { build_attribute_value, build_set_attributes, build_set_class } from './shared/element.js'; +import { + build_attribute_value, + build_attribute_effect, + build_set_class +} from './shared/element.js'; import { build_render_statement } from './shared/utils.js'; /** @@ -80,18 +84,15 @@ export function SvelteElement(node, context) { ) { build_set_class(node, element_id, attributes[0], class_directives, inner_context, false); } else if (attributes.length) { - const attributes_id = b.id(context.state.scope.generate('attributes')); - // 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. - build_set_attributes( + build_attribute_effect( attributes, class_directives, style_directives, inner_context, node, - element_id, - attributes_id + element_id ); } diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js index a093a0bf4a96..67de25b77041 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js @@ -16,28 +16,30 @@ import { build_template_chunk, get_expression_id } from './utils.js'; * @param {ComponentContext} context * @param {AST.RegularElement | AST.SvelteElement} element * @param {Identifier} element_id - * @param {Identifier} attributes_id */ -export function build_set_attributes( +export function build_attribute_effect( attributes, class_directives, style_directives, context, element, - element_id, - attributes_id + element_id ) { - let is_dynamic = false; - /** @type {ObjectExpression['properties']} */ const values = []; + /** @type {Expression[]} */ + const expressions = []; + + /** @param {Expression} value */ + function memoize(value) { + return b.id(`$${expressions.push(value) - 1}`); + } + for (const attribute of attributes) { if (attribute.type === 'Attribute') { - const { value, has_state } = build_attribute_value( - attribute.value, - context, - (value, metadata) => (metadata.has_call ? get_expression_id(context.state, value) : value) + const { value } = build_attribute_value(attribute.value, context, (value, metadata) => + metadata.has_call ? memoize(value) : value ); if ( @@ -51,16 +53,11 @@ export function build_set_attributes( } else { values.push(b.init(attribute.name, value)); } - - is_dynamic ||= has_state; } else { - // objects could contain reactive getters -> play it safe and always assume spread attributes are reactive - is_dynamic = true; - let value = /** @type {Expression} */ (context.visit(attribute)); if (attribute.metadata.expression.has_call) { - value = get_expression_id(context.state, value); + value = memoize(value); } values.push(b.spread(value)); @@ -72,12 +69,9 @@ export function build_set_attributes( b.prop( 'init', b.array([b.id('$.CLASS')]), - build_class_directives_object(class_directives, context) + build_class_directives_object(class_directives, expressions, context) ) ); - - is_dynamic ||= - class_directives.find((directive) => directive.metadata.expression.has_state) !== null; } if (style_directives.length) { @@ -85,31 +79,28 @@ export function build_set_attributes( b.prop( 'init', b.array([b.id('$.STYLE')]), - build_style_directives_object(style_directives, context) + build_style_directives_object(style_directives, expressions, context) ) ); - - is_dynamic ||= style_directives.some((directive) => directive.metadata.expression.has_state); } - const call = b.call( - '$.set_attributes', - element_id, - is_dynamic ? attributes_id : b.null, - b.object(values), - element.metadata.scoped && - context.state.analysis.css.hash !== '' && - b.literal(context.state.analysis.css.hash), - is_ignored(element, 'hydration_attribute_changed') && b.true + context.state.init.push( + b.stmt( + b.call( + '$.attribute_effect', + element_id, + b.arrow( + expressions.map((_, i) => b.id(`$${i}`)), + b.object(values) + ), + expressions.length > 0 && b.array(expressions.map((expression) => b.thunk(expression))), + element.metadata.scoped && + context.state.analysis.css.hash !== '' && + b.literal(context.state.analysis.css.hash), + is_ignored(element, 'hydration_attribute_changed') && b.true + ) + ) ); - - if (is_dynamic) { - context.state.init.push(b.let(attributes_id)); - const update = b.stmt(b.assignment('=', attributes_id, call)); - context.state.update.push(update); - } else { - context.state.init.push(b.stmt(call)); - } } /** @@ -167,7 +158,7 @@ export function build_set_class(element, node_id, attribute, class_directives, c value = b.call('$.clsx', value); } - return metadata.has_call ? get_expression_id(context.state, value) : value; + return metadata.has_call ? get_expression_id(context.state.expressions, value) : value; }); /** @type {Identifier | undefined} */ @@ -180,7 +171,7 @@ export function build_set_class(element, node_id, attribute, class_directives, c let next; if (class_directives.length) { - next = build_class_directives_object(class_directives, context); + next = build_class_directives_object(class_directives, context.state.expressions, context); has_state ||= class_directives.some((d) => d.metadata.expression.has_state); if (has_state) { @@ -235,7 +226,7 @@ export function build_set_class(element, node_id, attribute, class_directives, c */ export function build_set_style(node_id, attribute, style_directives, context) { let { value, has_state } = build_attribute_value(attribute.value, context, (value, metadata) => - metadata.has_call ? get_expression_id(context.state, value) : value + metadata.has_call ? get_expression_id(context.state.expressions, value) : value ); /** @type {Identifier | undefined} */ @@ -248,7 +239,7 @@ export function build_set_style(node_id, attribute, style_directives, context) { let next; if (style_directives.length) { - next = build_style_directives_object(style_directives, context); + next = build_style_directives_object(style_directives, context.state.expressions, context); has_state ||= style_directives.some((d) => d.metadata.expression.has_state); if (has_state) { diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js index 80b472ac378e..ebf88e878f8d 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/utils.js @@ -21,12 +21,12 @@ export function memoize_expression(state, value) { } /** - * - * @param {ComponentClientTransformState} state + * Pushes `value` into `expressions` and returns a new id + * @param {Expression[]} expressions * @param {Expression} value */ -export function get_expression_id(state, value) { - return b.id(`$${state.expressions.push(value) - 1}`); +export function get_expression_id(expressions, value) { + return b.id(`$${expressions.push(value) - 1}`); } /** @@ -40,7 +40,8 @@ export function build_template_chunk( values, visit, state, - memoize = (value, metadata) => (metadata.has_call ? get_expression_id(state, value) : value) + memoize = (value, metadata) => + metadata.has_call ? get_expression_id(state.expressions, value) : value ) { /** @type {Expression[]} */ const expressions = []; diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index 3d1acbd31ce1..6cfc123cc251 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -1,3 +1,4 @@ +/** @import { Effect } from '#client' */ import { DEV } from 'esm-env'; import { hydrating, set_hydrating } from '../hydration.js'; import { get_descriptors, get_prototype_of } from '../../../shared/utils.js'; @@ -10,6 +11,7 @@ import { is_capture_event, is_delegated, normalize_attribute } from '../../../.. import { active_effect, active_reaction, + get, set_active_effect, set_active_reaction } from '../../runtime.js'; @@ -18,6 +20,9 @@ import { clsx } from '../../../shared/attributes.js'; import { set_class } from './class.js'; import { set_style } from './style.js'; import { ATTACHMENT_KEY, NAMESPACE_HTML } from '../../../../constants.js'; +import { block, branch, destroy_effect } from '../../reactivity/effects.js'; +import { derived } from '../../reactivity/deriveds.js'; +import { init_select, select_option } from './bindings/select.js'; export const CLASS = Symbol('class'); export const STYLE = Symbol('style'); @@ -447,13 +452,68 @@ export function set_attributes(element, prev, next, css_hash, skip_warning = fal set_hydrating(true); } - for (let symbol of Object.getOwnPropertySymbols(next)) { - if (symbol.description === ATTACHMENT_KEY) { - attach(element, () => next[symbol]); + return current; +} + +/** + * @param {Element & ElementCSSInlineStyle} element + * @param {(...expressions: any) => Record} fn + * @param {Array<() => any>} thunks + * @param {string} [css_hash] + * @param {boolean} [skip_warning] + */ +export function attribute_effect( + element, + fn, + thunks = [], + css_hash, + skip_warning = false, + d = derived +) { + const deriveds = thunks.map(d); + + /** @type {Record | undefined} */ + var prev = undefined; + + /** @type {Record} */ + var effects = {}; + + var is_select = element.nodeName === 'SELECT'; + var inited = false; + + block(() => { + var next = fn(...deriveds.map(get)); + + set_attributes(element, prev, next, css_hash, skip_warning); + + if (inited && is_select) { + select_option(/** @type {HTMLSelectElement} */ (element), next.value, false); + } + + for (let symbol of Object.getOwnPropertySymbols(effects)) { + if (!next[symbol]) destroy_effect(effects[symbol]); } + + for (let symbol of Object.getOwnPropertySymbols(next)) { + var n = next[symbol]; + + if (symbol.description === ATTACHMENT_KEY && (!prev || n !== prev[symbol])) { + if (effects[symbol]) destroy_effect(effects[symbol]); + effects[symbol] = branch(() => attach(element, () => n)); + } + } + + prev = next; + }); + + if (is_select) { + init_select( + /** @type {HTMLSelectElement} */ (element), + () => /** @type {Record} */ (prev).value + ); } - return current; + inited = true; } /** diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/select.js b/packages/svelte/src/internal/client/dom/elements/bindings/select.js index 20f95af5ec0c..e3263c65afae 100644 --- a/packages/svelte/src/internal/client/dom/elements/bindings/select.js +++ b/packages/svelte/src/internal/client/dom/elements/bindings/select.js @@ -25,10 +25,14 @@ export function select_option(select, value, mounting) { } // Otherwise, update the selection - return select_options(select, value); + for (var option of select.options) { + option.selected = value.includes(get_option_value(option)); + } + + return; } - for (var option of select.options) { + for (option of select.options) { var option_value = get_option_value(option); if (is(option_value, value)) { option.selected = true; @@ -136,16 +140,6 @@ export function bind_select_value(select, get, set = get) { init_select(select); } -/** - * @param {HTMLSelectElement} select - * @param {unknown[]} value - */ -function select_options(select, value) { - for (var option of select.options) { - option.selected = value.includes(get_option_value(option)); - } -} - /** @param {HTMLOptionElement} option */ function get_option_value(option) { // __value only exists if the