From f81f4feab851da65cb7830b8e1cc5576611db585 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 20 Sep 2024 07:24:08 -0400 Subject: [PATCH] chore: simplify attributes (#13337) * chore: simplify attributes * fix/optimise * unused --- .../client/visitors/RegularElement.js | 3 +- .../client/visitors/SvelteElement.js | 21 ++++-- .../client/dom/elements/attributes.js | 64 ++++--------------- packages/svelte/src/internal/client/index.js | 3 +- 4 files changed, 28 insertions(+), 63 deletions(-) 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 d004da0411..3b3888cf5b 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 @@ -511,7 +511,8 @@ function build_element_spread_attributes( b.object(values), context.state.analysis.css.hash !== '' && b.literal(context.state.analysis.css.hash), preserve_attribute_case && b.true, - is_ignored(element, 'hydration_attribute_changed') && b.true + is_ignored(element, 'hydration_attribute_changed') && b.true, + element.name.includes('-') && b.true ) ) ); 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 81314d67dd..01bd970035 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 @@ -1,7 +1,7 @@ /** @import { BlockStatement, Expression, ExpressionStatement, Identifier, ObjectExpression, Statement } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { ComponentContext } from '../types' */ -import { dev, locator } from '../../../../state.js'; +import { dev, is_ignored, locator } from '../../../../state.js'; import { get_attribute_expression, is_event_attribute, @@ -84,7 +84,7 @@ export function SvelteElement(node, context) { // 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 = - build_dynamic_element_attributes(attributes, inner_context, element_id) !== null; + build_dynamic_element_attributes(node, attributes, inner_context, element_id) !== null; // class/style directives must be applied last since they could override class/style attributes build_class_directives(class_directives, element_id, inner_context, is_attributes_reactive); @@ -137,12 +137,13 @@ export function SvelteElement(node, context) { /** * Serializes dynamic element attribute assignments. * Returns the `true` if spread is deemed reactive. + * @param {AST.SvelteElement} element * @param {Array} attributes * @param {ComponentContext} context * @param {Identifier} element_id * @returns {boolean} */ -function build_dynamic_element_attributes(attributes, context, element_id) { +function build_dynamic_element_attributes(element, attributes, context, element_id) { if (attributes.length === 0) { if (context.state.analysis.css.hash) { context.state.init.push( @@ -197,11 +198,14 @@ function build_dynamic_element_attributes(attributes, context, element_id) { '=', b.id(id), b.call( - '$.set_dynamic_element_attributes', + '$.set_attributes', element_id, b.id(id), b.object(values), - context.state.analysis.css.hash !== '' && b.literal(context.state.analysis.css.hash) + context.state.analysis.css.hash !== '' && b.literal(context.state.analysis.css.hash), + b.binary('!==', b.member(element_id, 'namespaceURI'), b.id('$.NAMESPACE_SVG')), + is_ignored(element, 'hydration_attribute_changed') && b.true, + b.call(b.member(b.member(element_id, 'nodeName'), 'includes'), b.literal('-')) ) ) ); @@ -218,11 +222,14 @@ function build_dynamic_element_attributes(attributes, context, element_id) { context.state.init.push( b.stmt( b.call( - '$.set_dynamic_element_attributes', + '$.set_attributes', element_id, b.literal(null), b.object(values), - context.state.analysis.css.hash !== '' && b.literal(context.state.analysis.css.hash) + context.state.analysis.css.hash !== '' && b.literal(context.state.analysis.css.hash), + b.binary('!==', b.member(element_id, 'namespaceURI'), b.id('$.NAMESPACE_SVG')), + is_ignored(element, 'hydration_attribute_changed') && b.true, + b.call(b.member(b.member(element_id, 'nodeName'), 'includes'), b.literal('-')) ) ) ); diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index ee6525d8d5..29e872dc24 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -136,18 +136,8 @@ export function set_xlink_attribute(dom, attribute, value) { * @param {any} value */ export function set_custom_element_data(node, prop, value) { - if (prop in node) { - // Reading the prop could cause an error, we don't want this to fail everything else - try { - var curr_val = node[prop]; - } catch { - set_attribute(node, prop, value); - return; - } - var next_val = typeof curr_val === 'boolean' && value === '' ? true : value; - if (typeof curr_val !== 'object' || curr_val !== next_val) { - node[prop] = next_val; - } + if (get_setters(node).includes(prop)) { + node[prop] = value; } else { set_attribute(node, prop, value); } @@ -161,6 +151,7 @@ export function set_custom_element_data(node, prop, value) { * @param {string} [css_hash] * @param {boolean} preserve_attribute_case * @param {boolean} [skip_warning] + * @param {boolean} [is_custom_element] * @returns {Record} */ export function set_attributes( @@ -169,7 +160,8 @@ export function set_attributes( next, css_hash, preserve_attribute_case = false, - skip_warning + skip_warning = false, + is_custom_element = false ) { var current = prev || {}; var is_option_element = element.tagName === 'OPTION'; @@ -271,14 +263,11 @@ export function set_attributes( delegate([event_name]); } } - } else if (value == null) { - attributes[key] = null; - element.removeAttribute(key); - } else if (key === 'style') { + } else if (key === 'style' && value != null) { element.style.cssText = value + ''; } else if (key === 'autofocus') { autofocus(/** @type {HTMLElement} */ (element), Boolean(value)); - } else if (key === '__value' || key === 'value') { + } else if (key === '__value' || (key === 'value' && value != null)) { // @ts-ignore element.value = element[key] = element.__value = value; } else { @@ -287,7 +276,10 @@ export function set_attributes( name = normalize_attribute(name); } - if (setters.includes(name) && typeof value !== 'string') { + if (value == null && !is_custom_element) { + attributes[key] = null; + element.removeAttribute(key); + } else if (setters.includes(name) && (is_custom_element || typeof value !== 'string')) { // @ts-ignore element[name] = value; } else if (typeof value !== 'function') { @@ -316,40 +308,6 @@ export function set_attributes( return current; } -/** - * @param {Element} node - * @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, next, css_hash) { - if (node.tagName.includes('-')) { - for (var key in prev) { - if (!(key in next)) { - next[key] = null; - } - } - - if (css_hash !== undefined) { - next.class = next.class ? next.class + ' ' + css_hash : css_hash; - } - - for (key in next) { - set_custom_element_data(node, key, next[key]); - } - - return next; - } - - return set_attributes( - /** @type {Element & ElementCSSInlineStyle} */ (node), - prev, - next, - css_hash, - node.namespaceURI !== NAMESPACE_SVG - ); -} - /** @type {Map} */ var setters_cache = new Map(); diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index 0debfe9d19..766494e856 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -1,4 +1,4 @@ -export { FILENAME, HMR } from '../../constants.js'; +export { FILENAME, HMR, NAMESPACE_SVG } from '../../constants.js'; export { cleanup_styles } from './dev/css.js'; export { add_locations } from './dev/elements.js'; export { hmr } from './dev/hmr.js'; @@ -30,7 +30,6 @@ export { set_attribute, set_attributes, set_custom_element_data, - set_dynamic_element_attributes, set_xlink_attribute, handle_lazy_img, set_value,