From 55761143caa8b9394478d0f0cb240e7547bb3558 Mon Sep 17 00:00:00 2001 From: adiguba Date: Fri, 21 Feb 2025 11:38:57 +0100 Subject: [PATCH 01/24] set_class with class: directives --- .../src/compiler/phases/2-analyze/index.js | 85 ++++------ .../client/visitors/RegularElement.js | 94 +++++++---- .../client/visitors/SvelteElement.js | 33 ++-- .../client/visitors/shared/element.js | 132 ++++++++++++---- .../server/visitors/shared/element.js | 117 +++++++------- .../client/dom/elements/attributes.js | 21 ++- .../src/internal/client/dom/elements/class.js | 147 +++++------------- .../src/internal/client/dom/operations.js | 2 +- packages/svelte/src/internal/client/index.js | 6 +- packages/svelte/src/internal/server/index.js | 22 +-- .../svelte/src/internal/shared/attributes.js | 43 +++++ 11 files changed, 383 insertions(+), 319 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 93c438c8529f..8dff8f4b8f31 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -767,66 +767,39 @@ export function analyze_component(root, source, options) { if (!should_ignore_unused) { warn_unused(analysis.css.ast); } + } - outer: for (const node of analysis.elements) { - if (node.metadata.scoped) { - // Dynamic elements in dom mode always use spread for attributes and therefore shouldn't have a class attribute added to them - // TODO this happens during the analysis phase, which shouldn't know anything about client vs server - if (node.type === 'SvelteElement' && options.generate === 'client') continue; - - /** @type {AST.Attribute | undefined} */ - let class_attribute = undefined; - - for (const attribute of node.attributes) { - if (attribute.type === 'SpreadAttribute') { - // The spread method appends the hash to the end of the class attribute on its own - continue outer; - } - - if (attribute.type !== 'Attribute') continue; - if (attribute.name.toLowerCase() !== 'class') continue; - // The dynamic class method appends the hash to the end of the class attribute on its own - if (attribute.metadata.needs_clsx) continue outer; + for (const node of analysis.elements) { + if (node.metadata.scoped && is_custom_element_node(node)) { + mark_subtree_dynamic(node.metadata.path); + } - class_attribute = attribute; - } + let has_class = false; + let has_spread = false; + let has_class_directive = false; + for (const attribute of node.attributes) { + // The spread method appends the hash to the end of the class attribute on its own + if (attribute.type === 'SpreadAttribute') { + has_spread = true; + break; + } + has_class_directive ||= attribute.type === 'ClassDirective'; + has_class ||= attribute.type === 'Attribute' && attribute.name.toLowerCase() === 'class'; + } - if (class_attribute && class_attribute.value !== true) { - if (is_text_attribute(class_attribute)) { - class_attribute.value[0].data += ` ${analysis.css.hash}`; - } else { - /** @type {AST.Text} */ - const css_text = { - type: 'Text', - data: ` ${analysis.css.hash}`, - raw: ` ${analysis.css.hash}`, - start: -1, - end: -1 - }; - - if (Array.isArray(class_attribute.value)) { - class_attribute.value.push(css_text); - } else { - class_attribute.value = [class_attribute.value, css_text]; - } + // We need an empty class to generate the set_class() or class="" correctly + if (!has_spread && !has_class && (node.metadata.scoped || has_class_directive)) { + node.attributes.push( + create_attribute('class', -1, -1, [ + { + type: 'Text', + data: '', + raw: '', + start: -1, + end: -1 } - } else { - node.attributes.push( - create_attribute('class', -1, -1, [ - { - type: 'Text', - data: analysis.css.hash, - raw: analysis.css.hash, - start: -1, - end: -1 - } - ]) - ); - if (is_custom_element_node(node) && node.attributes.length === 1) { - mark_subtree_dynamic(node.metadata.path); - } - } - } + ]) + ); } } 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 018bdacc5ecd..b149efbaf1a7 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 @@ -1,4 +1,4 @@ -/** @import { Expression, ExpressionStatement, Identifier, MemberExpression, Statement } from 'estree' */ +/** @import { Expression, ExpressionStatement, Identifier, MemberExpression, ObjectExpression, Statement } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { SourceLocation } from '#shared' */ /** @import { ComponentClientTransformState, ComponentContext } from '../types' */ @@ -14,15 +14,15 @@ import { escape_html } from '../../../../../escaping.js'; import { dev, is_ignored, locator } from '../../../../state.js'; import { is_event_attribute, is_text_attribute } from '../../../../utils/ast.js'; import * as b from '../../../../utils/builders.js'; -import { is_custom_element_node } from '../../../nodes.js'; +import { create_expression_metadata, is_custom_element_node } from '../../../nodes.js'; import { clean_nodes, determine_namespace_for_children } from '../../utils.js'; import { build_getter } from '../utils.js'; import { get_attribute_name, build_attribute_value, - build_class_directives, build_style_directives, - build_set_attributes + build_set_attributes, + build_set_class } from './shared/element.js'; import { process_children } from './shared/fragment.js'; import { @@ -223,6 +223,8 @@ export function RegularElement(node, context) { build_set_attributes( attributes, + class_directives, + style_directives, context, node, node_id, @@ -270,13 +272,22 @@ export function RegularElement(node, context) { continue; } + const name = get_attribute_name(node, attribute); if ( !is_custom_element && !cannot_be_set_statically(attribute.name) && - (attribute.value === true || is_text_attribute(attribute)) + (attribute.value === true || is_text_attribute(attribute)) && + (name !== 'class' || class_directives.length === 0) ) { - const name = get_attribute_name(node, attribute); - const value = is_text_attribute(attribute) ? attribute.value[0].data : true; + let value = is_text_attribute(attribute) ? attribute.value[0].data : true; + + if (name === 'class' && node.metadata.scoped && context.state.analysis.css.hash) { + if (value === true || value === '') { + value = context.state.analysis.css.hash; + } else { + value += ' ' + context.state.analysis.css.hash; + } + } if (name !== 'class' || value) { context.state.template.push( @@ -290,15 +301,23 @@ export function RegularElement(node, context) { continue; } - const is = is_custom_element - ? build_custom_element_attribute_update_assignment(node_id, attribute, context) - : build_element_attribute_update_assignment(node, node_id, attribute, attributes, context); + const is = + is_custom_element && name !== 'class' + ? build_custom_element_attribute_update_assignment(node_id, attribute, context) + : build_element_attribute_update_assignment( + node, + node_id, + attribute, + attributes, + class_directives, + style_directives, + context + ); if (is) is_attributes_reactive = true; } } - // class/style directives must be applied last since they could override class/style attributes - build_class_directives(class_directives, node_id, context, is_attributes_reactive); + // style directives must be applied last since they could override class/style attributes build_style_directives(style_directives, node_id, context, is_attributes_reactive); if ( @@ -492,6 +511,23 @@ function setup_select_synchronization(value_binding, context) { ); } +/** + * @param {AST.ClassDirective[]} class_directives + * @param {ComponentContext} context + * @return {ObjectExpression} + */ +export function build_class_directives_object(class_directives, context) { + let properties = []; + for (const d of class_directives) { + let expression = /** @type Expression */ (context.visit(d.expression)); + if (d.metadata.expression.has_call) { + expression = get_expression_id(context.state, expression); + } + properties.push(b.init(d.name, expression)); + } + return b.object(properties); +} + /** * Serializes an assignment to an element property by adding relevant statements to either only * the init or the the init and update arrays, depending on whether or not the value is dynamic. @@ -518,6 +554,8 @@ function setup_select_synchronization(value_binding, context) { * @param {Identifier} node_id * @param {AST.Attribute} attribute * @param {Array} attributes + * @param {AST.ClassDirective[]} class_directives + * @param {AST.StyleDirective[]} style_directives * @param {ComponentContext} context * @returns {boolean} */ @@ -526,6 +564,8 @@ function build_element_attribute_update_assignment( node_id, attribute, attributes, + class_directives, + style_directives, context ) { const state = context.state; @@ -564,19 +604,15 @@ function build_element_attribute_update_assignment( let update; if (name === 'class') { - if (attribute.metadata.needs_clsx) { - value = b.call('$.clsx', value); - } - - update = b.stmt( - b.call( - is_svg ? '$.set_svg_class' : is_mathml ? '$.set_mathml_class' : '$.set_class', - node_id, - value, - attribute.metadata.needs_clsx && context.state.analysis.css.hash - ? b.literal(context.state.analysis.css.hash) - : undefined - ) + return build_set_class( + element, + node_id, + attribute, + value, + has_state, + class_directives, + context, + !is_svg && !is_mathml ); } else if (name === 'value') { update = b.stmt(b.call('$.set_value', node_id, value)); @@ -640,14 +676,6 @@ function build_custom_element_attribute_update_assignment(node_id, attribute, co const name = attribute.name; // don't lowercase, as we set the element's property, which might be case sensitive let { value, has_state } = build_attribute_value(attribute.value, context); - // We assume that noone's going to redefine the semantics of the class attribute on custom elements, i.e. it's still used for CSS classes - if (name === 'class' && attribute.metadata.needs_clsx) { - if (context.state.analysis.css.hash) { - value = b.array([value, b.literal(context.state.analysis.css.hash)]); - } - value = b.call('$.clsx', value); - } - const update = b.stmt(b.call('$.set_custom_element_data', node_id, b.literal(name), value)); if (has_state) { 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 e27528365518..7e2ef4c5dc3f 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 @@ -7,11 +7,11 @@ import * as b from '../../../../utils/builders.js'; import { determine_namespace_for_children } from '../../utils.js'; import { build_attribute_value, - build_class_directives, build_set_attributes, + build_set_class, build_style_directives } from './shared/element.js'; -import { build_render_statement } from './shared/utils.js'; +import { build_render_statement, get_expression_id } from './shared/utils.js'; /** * @param {AST.SvelteElement} node @@ -80,19 +80,31 @@ export function SvelteElement(node, context) { // Then do attributes let is_attributes_reactive = false; - if (attributes.length === 0) { - if (context.state.analysis.css.hash) { - inner_context.state.init.push( - b.stmt(b.call('$.set_class', element_id, b.literal(context.state.analysis.css.hash))) - ); - } - } else { + if ( + attributes.length === 1 && + attributes[0].type === 'Attribute' && + attributes[0].name.toLowerCase() === 'class' + ) { + // special case when there only a class attribute + build_set_class( + node, + element_id, + attributes[0], + b.null, + false, + 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. is_attributes_reactive = build_set_attributes( attributes, + class_directives, + style_directives, inner_context, node, element_id, @@ -103,8 +115,7 @@ export function SvelteElement(node, context) { ); } - // 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); + // style directives must be applied last since they could override class/style attributes build_style_directives(style_directives, element_id, inner_context, is_attributes_reactive); const get_tag = b.thunk(/** @type {Expression} */ (context.visit(node.tag))); 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 abffad0ff7a4..37c1647fd491 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 @@ -6,10 +6,13 @@ import { is_ignored } from '../../../../../state.js'; import { is_event_attribute } from '../../../../../utils/ast.js'; import * as b from '../../../../../utils/builders.js'; import { build_getter } from '../../utils.js'; +import { build_class_directives_object } from '../RegularElement.js'; import { build_template_chunk, get_expression_id } from './utils.js'; /** * @param {Array} attributes + * @param {AST.ClassDirective[]} class_directives + * @param {AST.StyleDirective[]} style_directives * @param {ComponentContext} context * @param {AST.RegularElement | AST.SvelteElement} element * @param {Identifier} element_id @@ -20,6 +23,8 @@ import { build_template_chunk, get_expression_id } from './utils.js'; */ export function build_set_attributes( attributes, + class_directives, + style_directives, context, element, element_id, @@ -68,6 +73,18 @@ export function build_set_attributes( } } + if (class_directives.length) { + values.push( + b.prop( + 'init', + b.array([b.id('$.CLASS')]), + build_class_directives_object(class_directives, context) + ) + ); + is_dynamic ||= + class_directives.find((directive) => directive.metadata.expression.has_state) !== null; + } + const call = b.call( '$.set_attributes', element_id, @@ -134,39 +151,6 @@ export function build_style_directives( } } -/** - * Serializes each class directive into something like `$.class_toogle(element, class_name, value)` - * and adds it either to init or update, depending on whether or not the value or the attributes are dynamic. - * @param {AST.ClassDirective[]} class_directives - * @param {Identifier} element_id - * @param {ComponentContext} context - * @param {boolean} is_attributes_reactive - */ -export function build_class_directives( - class_directives, - element_id, - context, - is_attributes_reactive -) { - const state = context.state; - for (const directive of class_directives) { - const { has_state, has_call } = directive.metadata.expression; - let value = /** @type {Expression} */ (context.visit(directive.expression)); - - if (has_call) { - value = get_expression_id(state, value); - } - - const update = b.stmt(b.call('$.toggle_class', element_id, b.literal(directive.name), value)); - - if (is_attributes_reactive || has_state) { - state.update.push(update); - } else { - state.init.push(update); - } - } -} - /** * @param {AST.Attribute['value']} value * @param {ComponentContext} context @@ -207,3 +191,85 @@ export function get_attribute_name(element, attribute) { return attribute.name; } + +/** + * @param {AST.RegularElement | AST.SvelteElement} element + * @param {Identifier} node_id + * @param {AST.Attribute | null} attribute + * @param {Expression} value + * @param {boolean} has_state + * @param {AST.ClassDirective[]} class_directives + * @param {ComponentContext} context + * @param {boolean} is_html + * @returns {boolean} + */ +export function build_set_class( + element, + node_id, + attribute, + value, + has_state, + class_directives, + context, + is_html +) { + if (attribute && attribute.metadata.needs_clsx) { + value = b.call('$.clsx', value); + } + + /** @type {Identifier | undefined} */ + let previous_id; + /** @type {ObjectExpression | Identifier | undefined} */ + let prev; + /** @type {ObjectExpression | undefined} */ + let next; + if (class_directives.length) { + next = build_class_directives_object(class_directives, context); + + has_state = has_state || !!class_directives.find((d) => d.metadata.expression.has_state); + if (has_state) { + previous_id = b.id(context.state.scope.generate('classes')); + context.state.init.push(b.declaration('let', [b.declarator(previous_id)])); + prev = previous_id; + } else { + prev = b.object([]); + } + } + + /** @type {Expression | undefined} */ + let css_hash; + if (element.metadata.scoped && context.state.analysis.css.hash) { + if (value.type === 'Literal' && (value.value === '' || value.value === null)) { + value = b.literal(context.state.analysis.css.hash); + } else { + css_hash = b.literal(context.state.analysis.css.hash); + } + } + if (!css_hash && next) { + css_hash = b.null; + } + + /** @type {Expression} */ + let set_class = b.call( + '$.set_class', + node_id, + is_html ? b.literal(1) : b.literal(0), + value, + css_hash, + prev, + next + ); + + if (previous_id) { + set_class = b.assignment('=', previous_id, set_class); + } + + const update = b.stmt(set_class); + if (has_state) { + context.state.update.push(update); + return true; + } else { + context.state.init.push(update); + return false; + } +} diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js index d0d800d3cbc5..690d7d92ee7c 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js @@ -1,4 +1,4 @@ -/** @import { Expression, Literal } from 'estree' */ +/** @import { Expression, Literal, ObjectExpression } from 'estree' */ /** @import { AST, Namespace } from '#compiler' */ /** @import { ComponentContext, ComponentServerTransformState } from '../../types.js' */ import { @@ -24,6 +24,7 @@ import { is_content_editable_binding, is_load_error_element } from '../../../../../../utils.js'; +import { escape_html } from '../../../../../../escaping.js'; const WHITESPACE_INSENSITIVE_ATTRIBUTES = ['class', 'style']; @@ -86,23 +87,15 @@ export function build_element_attributes(node, context) { } else if (attribute.name !== 'defaultValue' && attribute.name !== 'defaultChecked') { if (attribute.name === 'class') { class_index = attributes.length; - if (attribute.metadata.needs_clsx) { - const clsx_value = b.call( - '$.clsx', - /** @type {AST.ExpressionTag} */ (attribute.value).expression - ); attributes.push({ ...attribute, value: { .../** @type {AST.ExpressionTag} */ (attribute.value), - expression: context.state.analysis.css.hash - ? b.binary( - '+', - b.binary('+', clsx_value, b.literal(' ')), - b.literal(context.state.analysis.css.hash) - ) - : clsx_value + expression: b.call( + '$.clsx', + /** @type {AST.ExpressionTag} */ (attribute.value).expression + ) } }); } else { @@ -219,8 +212,9 @@ export function build_element_attributes(node, context) { } } - if (class_directives.length > 0 && !has_spread) { - const class_attribute = build_class_directives( + if ((node.metadata.scoped || class_directives.length) && !has_spread) { + const class_attribute = build_to_class( + node.metadata.scoped ? context.state.analysis.css.hash : null, class_directives, /** @type {AST.Attribute | null} */ (attributes[class_index] ?? null) ); @@ -274,9 +268,14 @@ export function build_element_attributes(node, context) { WHITESPACE_INSENSITIVE_ATTRIBUTES.includes(name) ); - context.state.template.push( - b.call('$.attr', b.literal(name), value, is_boolean_attribute(name) && b.true) - ); + // pre-escape and inline literal attributes : + if (value.type === 'Literal' && typeof value.value === 'string') { + context.state.template.push(b.literal(` ${name}="${escape_html(value.value, true)}"`)); + } else { + context.state.template.push( + b.call('$.attr', b.literal(name), value, is_boolean_attribute(name) && b.true) + ); + } } } @@ -322,7 +321,7 @@ function build_element_spread_attributes( let styles; let flags = 0; - if (class_directives.length > 0 || context.state.analysis.css.hash) { + if (class_directives.length) { const properties = class_directives.map((directive) => b.init( directive.name, @@ -331,11 +330,6 @@ function build_element_spread_attributes( : /** @type {Expression} */ (context.visit(directive.expression)) ) ); - - if (context.state.analysis.css.hash) { - properties.unshift(b.init(context.state.analysis.css.hash, b.literal(true))); - } - classes = b.object(properties); } @@ -374,55 +368,74 @@ function build_element_spread_attributes( }) ); - const args = [object, classes, styles, flags ? b.literal(flags) : undefined]; + const css_hash = context.state.analysis.css.hash + ? b.literal(context.state.analysis.css.hash) + : b.null; + + const args = [object, css_hash, classes, styles, flags ? b.literal(flags) : undefined]; context.state.template.push(b.call('$.spread_attributes', ...args)); } /** * + * @param {string|null} hash * @param {AST.ClassDirective[]} class_directives * @param {AST.Attribute | null} class_attribute * @returns */ -function build_class_directives(class_directives, class_attribute) { - const expressions = class_directives.map((directive) => - b.conditional(directive.expression, b.literal(directive.name), b.literal('')) - ); - +function build_to_class(hash, class_directives, class_attribute) { if (class_attribute === null) { class_attribute = create_attribute('class', -1, -1, []); } - const chunks = get_attribute_chunks(class_attribute.value); - const last = chunks.at(-1); - - if (last?.type === 'Text') { - last.data += ' '; - last.raw += ' '; - } else if (last) { - chunks.push({ - type: 'Text', - start: -1, - end: -1, - data: ' ', - raw: ' ' - }); + /** @type {ObjectExpression | undefined} */ + let classes; + if (class_directives.length) { + classes = b.object( + class_directives.map((directive) => + b.prop('init', b.literal(directive.name), directive.expression) + ) + ); + } + + /** @type {Expression} */ + let class_name; + if (class_attribute.value === true) { + class_name = b.literal(''); + } else if (Array.isArray(class_attribute.value)) { + if (class_attribute.value.length === 0) { + class_name = b.null; + } else { + class_name = class_attribute.value + .map((val) => (val.type === 'Text' ? b.literal(val.data) : val.expression)) + .reduce((left, right) => b.binary('+', left, right)); + } + } else { + class_name = class_attribute.value.expression; } - chunks.push({ + /** @type {Expression} */ + let expression; + + if ( + hash && + !classes && + class_name.type === 'Literal' && + (class_name.value === null || class_name.value === '') + ) { + expression = b.literal(hash); + } else { + expression = b.call('$.to_class', class_name, b.literal(hash), classes); + } + class_attribute.value = { type: 'ExpressionTag', start: -1, end: -1, - expression: b.call( - b.member(b.call(b.member(b.array(expressions), 'filter'), b.id('Boolean')), b.id('join')), - b.literal(' ') - ), + expression: expression, metadata: { expression: create_expression_metadata() } - }); - - class_attribute.value = chunks; + }; return class_attribute; } diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index 2dba2d797a4a..3476f8a955ff 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -14,6 +14,10 @@ import { set_active_reaction } from '../../runtime.js'; import { clsx } from '../../../shared/attributes.js'; +import { set_class } from './class.js'; + +export const CLASS = Symbol('class'); +export const STYLE = Symbol('style'); /** * The value/checked attribute in the template actually corresponds to the defaultValue property, so we need @@ -254,8 +258,8 @@ 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} next New attributes - this function mutates this object + * @param {Record | undefined} prev + * @param {Record} next New attributes - this function mutates this object * @param {string} [css_hash] * @param {boolean} [preserve_attribute_case] * @param {boolean} [is_custom_element] @@ -289,10 +293,8 @@ export function set_attributes( if (next.class) { next.class = clsx(next.class); - } - - if (css_hash !== undefined) { - next.class = next.class ? next.class + ' ' + css_hash : css_hash; + } else if (css_hash || next[CLASS]) { + next.class = null; /* force call to set_class() */ } var setters = get_setters(element); @@ -325,7 +327,9 @@ export function set_attributes( } var prev_value = current[key]; - if (value === prev_value) continue; + if (value === prev_value && key !== 'class') { + continue; + } current[key] = value; @@ -375,6 +379,9 @@ export function set_attributes( // @ts-ignore element[`__${event_name}`] = undefined; } + } else if (key === 'class') { + var is_html = element.namespaceURI === 'http://www.w3.org/1999/xhtml'; + set_class(element, is_html, value, css_hash, prev?.[CLASS], next[CLASS]); } else if (key === 'style' && value != null) { element.style.cssText = value + ''; } else if (key === 'autofocus') { diff --git a/packages/svelte/src/internal/client/dom/elements/class.js b/packages/svelte/src/internal/client/dom/elements/class.js index 62ffb6d14b5c..a11f615cdd45 100644 --- a/packages/svelte/src/internal/client/dom/elements/class.js +++ b/packages/svelte/src/internal/client/dom/elements/class.js @@ -1,120 +1,49 @@ +import { to_class } from '../../../shared/attributes.js'; import { hydrating } from '../hydration.js'; /** - * @param {SVGElement} dom - * @param {string} value - * @param {string} [hash] - * @returns {void} - */ -export function set_svg_class(dom, value, hash) { - // @ts-expect-error need to add __className to patched prototype - var prev_class_name = dom.__className; - var next_class_name = to_class(value, hash); - - if (hydrating && dom.getAttribute('class') === next_class_name) { - // In case of hydration don't reset the class as it's already correct. - // @ts-expect-error need to add __className to patched prototype - dom.__className = next_class_name; - } else if ( - prev_class_name !== next_class_name || - (hydrating && dom.getAttribute('class') !== next_class_name) - ) { - if (next_class_name === '') { - dom.removeAttribute('class'); - } else { - dom.setAttribute('class', next_class_name); - } - - // @ts-expect-error need to add __className to patched prototype - dom.__className = next_class_name; - } -} - -/** - * @param {MathMLElement} dom - * @param {string} value + * @param {Element} dom + * @param {boolean|number} is_html + * @param {string|null} value * @param {string} [hash] - * @returns {void} + * @param {Record} [prev_classes] + * @param {Record} [next_classes] + * @returns {Record|undefined} */ -export function set_mathml_class(dom, value, hash) { +export function set_class(dom, is_html, value, hash, prev_classes, next_classes) { // @ts-expect-error need to add __className to patched prototype - var prev_class_name = dom.__className; - var next_class_name = to_class(value, hash); - - if (hydrating && dom.getAttribute('class') === next_class_name) { - // In case of hydration don't reset the class as it's already correct. - // @ts-expect-error need to add __className to patched prototype - dom.__className = next_class_name; - } else if ( - prev_class_name !== next_class_name || - (hydrating && dom.getAttribute('class') !== next_class_name) - ) { - if (next_class_name === '') { - dom.removeAttribute('class'); - } else { - dom.setAttribute('class', next_class_name); + var prev = dom.__className; + + if (hydrating || prev !== value) { + var next_class_name = to_class(value, hash, next_classes); + if (!hydrating || next_class_name !== dom.getAttribute('class')) { + // Removing the attribute when the value is only an empty string causes + // performance issues vs simply making the className an empty string. So + // we should only remove the class if the the value is nullish + // and there no hash/directives : + if (next_class_name == null) { + dom.removeAttribute('class'); + } else if (is_html) { + dom.className = next_class_name; + } else { + dom.setAttribute('class', next_class_name); + } } - - // @ts-expect-error need to add __className to patched prototype - dom.__className = next_class_name; - } -} - -/** - * @param {HTMLElement} dom - * @param {string} value - * @param {string} [hash] - * @returns {void} - */ -export function set_class(dom, value, hash) { - // @ts-expect-error need to add __className to patched prototype - var prev_class_name = dom.__className; - var next_class_name = to_class(value, hash); - - if (hydrating && dom.className === next_class_name) { - // In case of hydration don't reset the class as it's already correct. // @ts-expect-error need to add __className to patched prototype - dom.__className = next_class_name; - } else if ( - prev_class_name !== next_class_name || - (hydrating && dom.className !== next_class_name) - ) { - // Removing the attribute when the value is only an empty string causes - // peformance issues vs simply making the className an empty string. So - // we should only remove the class if the the value is nullish. - if (value == null && !hash) { - dom.removeAttribute('class'); - } else { - dom.className = next_class_name; + dom.__className = value; + } else if (next_classes) { + prev_classes = prev_classes ?? {}; + for (const key in next_classes) { + const is_present = !!next_classes[key]; + if (is_present !== !!prev_classes[key]) { + // TODO : use dom.classList.toggle instead ? + if (is_present) { + dom.classList.add(key); + } else { + dom.classList.remove(key); + } + } } - - // @ts-expect-error need to add __className to patched prototype - dom.__className = next_class_name; - } -} - -/** - * @template V - * @param {V} value - * @param {string} [hash] - * @returns {string | V} - */ -function to_class(value, hash) { - return (value == null ? '' : value) + (hash ? ' ' + hash : ''); -} - -/** - * @param {Element} dom - * @param {string} class_name - * @param {boolean} value - * @returns {void} - */ -export function toggle_class(dom, class_name, value) { - if (value) { - if (dom.classList.contains(class_name)) return; - dom.classList.add(class_name); - } else { - if (!dom.classList.contains(class_name)) return; - dom.classList.remove(class_name); } + return next_classes; } diff --git a/packages/svelte/src/internal/client/dom/operations.js b/packages/svelte/src/internal/client/dom/operations.js index 83565d17ae68..f6ac92456e78 100644 --- a/packages/svelte/src/internal/client/dom/operations.js +++ b/packages/svelte/src/internal/client/dom/operations.js @@ -44,7 +44,7 @@ export function init_operations() { // @ts-expect-error element_prototype.__click = undefined; // @ts-expect-error - element_prototype.__className = ''; + element_prototype.__className = undefined; // @ts-expect-error element_prototype.__attributes = null; // @ts-expect-error diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index d78f6d452e84..431ac8cf2492 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -39,9 +39,11 @@ export { set_checked, set_selected, set_default_checked, - set_default_value + set_default_value, + CLASS, + STYLE } from './dom/elements/attributes.js'; -export { set_class, set_svg_class, set_mathml_class, toggle_class } from './dom/elements/class.js'; +export { set_class } from './dom/elements/class.js'; export { apply, event, delegate, replay_events } from './dom/elements/events.js'; export { autofocus, remove_textarea_child } from './dom/elements/misc.js'; export { set_style } from './dom/elements/style.js'; diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index e8ffeed2fef5..1995985b56a7 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -2,7 +2,7 @@ /** @import { Component, Payload, RenderOutput } from '#server' */ /** @import { Store } from '#shared' */ export { FILENAME, HMR } from '../../constants.js'; -import { attr, clsx } from '../shared/attributes.js'; +import { attr, clsx, to_class } from '../shared/attributes.js'; import { is_promise, noop } from '../shared/utils.js'; import { subscribe_to_store } from '../../store/utils.js'; import { @@ -198,12 +198,13 @@ export function css_props(payload, is_html, props, component, dynamic = false) { /** * @param {Record} attrs - * @param {Record} [classes] + * @param {string|null} css_hash + * @param {Record} [classes] * @param {Record} [styles] * @param {number} [flags] * @returns {string} */ -export function spread_attributes(attrs, classes, styles, flags = 0) { +export function spread_attributes(attrs, css_hash, classes, styles, flags = 0) { if (styles) { attrs.style = attrs.style ? style_object_to_string(merge_styles(/** @type {string} */ (attrs.style), styles)) @@ -213,17 +214,8 @@ export function spread_attributes(attrs, classes, styles, flags = 0) { if (attrs.class) { attrs.class = clsx(attrs.class); } - - if (classes) { - const classlist = attrs.class ? [attrs.class] : []; - - for (const key in classes) { - if (classes[key]) { - classlist.push(key); - } - } - - attrs.class = classlist.join(' '); + if (css_hash || classes) { + attrs.class = to_class(attrs.class, css_hash, classes); } let attr_str = ''; @@ -552,7 +544,7 @@ export function props_id(payload) { return uid; } -export { attr, clsx }; +export { attr, clsx, to_class }; export { html } from './blocks/html.js'; diff --git a/packages/svelte/src/internal/shared/attributes.js b/packages/svelte/src/internal/shared/attributes.js index a561501bf4f6..e08cfeb431e5 100644 --- a/packages/svelte/src/internal/shared/attributes.js +++ b/packages/svelte/src/internal/shared/attributes.js @@ -40,3 +40,46 @@ export function clsx(value) { return value ?? ''; } } + +/** + * @param {any} clazz + * @param {string|null} [hash] + * @param {Record} [classes] + * @returns {string|null} + */ +export function to_class(clazz, hash, classes) { + let class_name = clazz == null ? '' : '' + clazz; + if (hash) { + class_name = class_name ? class_name + ' ' + hash : hash; + } + if (classes) { + const white_spaces = ' \t\n\r\f\u00a0\u000b\ufeff'; + for (const key in classes) { + if (classes[key]) { + class_name = class_name ? class_name + ' ' + key : key; + } else if (class_name.length) { + const len = key.length; + let start = 0; + while ((start = class_name.indexOf(key, start)) >= 0) { + let stop = start + len; + if ( + white_spaces.indexOf(class_name[start - 1] ?? ' ') >= 0 && + white_spaces.indexOf(class_name[stop] ?? ' ') >= 0 + ) { + class_name = ( + class_name.substring(0, start).trim() + + ' ' + + class_name.substring(stop).trim() + ).trim(); + } else { + start = stop; + } + } + } + } + } + if (class_name === '') { + return null; + } + return class_name; +} From 42860b6b3882ba9aad63199d7bc01959a319a880 Mon Sep 17 00:00:00 2001 From: adiguba Date: Fri, 21 Feb 2025 11:40:03 +0100 Subject: [PATCH 02/24] update expected result (remove leading space) --- .../css/samples/undefined-with-scope/expected.html | 2 +- .../attribute-null-classname-with-style/_config.js | 10 +++++----- .../_config.js | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/svelte/tests/css/samples/undefined-with-scope/expected.html b/packages/svelte/tests/css/samples/undefined-with-scope/expected.html index ddb9429bc891..5eecaa9bb256 100644 --- a/packages/svelte/tests/css/samples/undefined-with-scope/expected.html +++ b/packages/svelte/tests/css/samples/undefined-with-scope/expected.html @@ -1 +1 @@ -

Foo

\ No newline at end of file +

Foo

\ No newline at end of file diff --git a/packages/svelte/tests/runtime-legacy/samples/attribute-null-classname-with-style/_config.js b/packages/svelte/tests/runtime-legacy/samples/attribute-null-classname-with-style/_config.js index c8710f9038b9..cbd0456e1335 100644 --- a/packages/svelte/tests/runtime-legacy/samples/attribute-null-classname-with-style/_config.js +++ b/packages/svelte/tests/runtime-legacy/samples/attribute-null-classname-with-style/_config.js @@ -1,17 +1,17 @@ import { ok, test } from '../../test'; export default test({ - html: '
', + html: '
', test({ assert, component, target }) { const div = target.querySelector('div'); ok(div); component.testName = null; - assert.equal(div.className, ' svelte-x1o6ra'); + assert.equal(div.className, 'svelte-x1o6ra'); component.testName = undefined; - assert.equal(div.className, ' svelte-x1o6ra'); + assert.equal(div.className, 'svelte-x1o6ra'); component.testName = undefined + ''; assert.equal(div.className, 'undefined svelte-x1o6ra'); @@ -32,10 +32,10 @@ export default test({ assert.equal(div.className, 'true svelte-x1o6ra'); component.testName = {}; - assert.equal(div.className, ' svelte-x1o6ra'); + assert.equal(div.className, 'svelte-x1o6ra'); component.testName = ''; - assert.equal(div.className, ' svelte-x1o6ra'); + assert.equal(div.className, 'svelte-x1o6ra'); component.testName = 'testClassName'; assert.equal(div.className, 'testClassName svelte-x1o6ra'); diff --git a/packages/svelte/tests/runtime-legacy/samples/attribute-null-func-classname-with-style/_config.js b/packages/svelte/tests/runtime-legacy/samples/attribute-null-func-classname-with-style/_config.js index 8d0f411b8fd2..081fceecf279 100644 --- a/packages/svelte/tests/runtime-legacy/samples/attribute-null-func-classname-with-style/_config.js +++ b/packages/svelte/tests/runtime-legacy/samples/attribute-null-func-classname-with-style/_config.js @@ -16,10 +16,10 @@ export default test({ assert.equal(div.className, 'testClassName svelte-x1o6ra'); component.testName = null; - assert.equal(div.className, ' svelte-x1o6ra'); + assert.equal(div.className, 'svelte-x1o6ra'); component.testName = undefined; - assert.equal(div.className, ' svelte-x1o6ra'); + assert.equal(div.className, 'svelte-x1o6ra'); component.testName = undefined + ''; assert.equal(div.className, 'undefined svelte-x1o6ra'); @@ -40,9 +40,9 @@ export default test({ assert.equal(div.className, 'true svelte-x1o6ra'); component.testName = {}; - assert.equal(div.className, ' svelte-x1o6ra'); + assert.equal(div.className, 'svelte-x1o6ra'); component.testName = ''; - assert.equal(div.className, ' svelte-x1o6ra'); + assert.equal(div.className, 'svelte-x1o6ra'); } }); From 0073e0d51e8c66606015ab427400df66363a1efc Mon Sep 17 00:00:00 2001 From: adiguba Date: Fri, 21 Feb 2025 12:22:45 +0100 Subject: [PATCH 03/24] fix --- .../3-transform/client/visitors/SvelteElement.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) 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 7e2ef4c5dc3f..6b6260e6b3ff 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 @@ -86,12 +86,17 @@ export function SvelteElement(node, context) { attributes[0].name.toLowerCase() === 'class' ) { // special case when there only a class attribute - build_set_class( + let { value, has_state } = build_attribute_value( + attributes[0].value, + context, + (value, metadata) => (metadata.has_call ? get_expression_id(context.state, value) : value) + ); + is_attributes_reactive = build_set_class( node, element_id, attributes[0], - b.null, - false, + value, + has_state, class_directives, inner_context, false From 2b3fb8e04de706159374784beb3515506d982b28 Mon Sep 17 00:00:00 2001 From: adiguba Date: Fri, 21 Feb 2025 12:26:17 +0100 Subject: [PATCH 04/24] optimize literals --- .../phases/3-transform/client/visitors/shared/element.js | 3 +++ .../phases/3-transform/server/visitors/shared/element.js | 8 ++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) 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 37c1647fd491..b8efbca17903 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 @@ -1,6 +1,7 @@ /** @import { Expression, Identifier, ObjectExpression } from 'estree' */ /** @import { AST, ExpressionMetadata } from '#compiler' */ /** @import { ComponentClientTransformState, ComponentContext } from '../../types' */ +import { escape_html } from '../../../../../../escaping.js'; import { normalize_attribute } from '../../../../../../utils.js'; import { is_ignored } from '../../../../../state.js'; import { is_event_attribute } from '../../../../../utils/ast.js'; @@ -241,6 +242,8 @@ export function build_set_class( if (element.metadata.scoped && context.state.analysis.css.hash) { if (value.type === 'Literal' && (value.value === '' || value.value === null)) { value = b.literal(context.state.analysis.css.hash); + } else if (value.type === 'Literal' && typeof value.value === 'string') { + value = b.literal(escape_html(value.value, true) + ' ' + context.state.analysis.css.hash); } else { css_hash = b.literal(context.state.analysis.css.hash); } diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js index 690d7d92ee7c..e656506bbaf4 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js @@ -421,9 +421,13 @@ function build_to_class(hash, class_directives, class_attribute) { hash && !classes && class_name.type === 'Literal' && - (class_name.value === null || class_name.value === '') + (class_name.value === null || class_name.value === '' || typeof class_name.value === 'string') ) { - expression = b.literal(hash); + if (class_name.value === null || class_name.value === '') { + expression = b.literal(hash); + } else { + expression = b.literal(escape_html(class_name.value, true) + ' ' + hash); + } } else { expression = b.call('$.to_class', class_name, b.literal(hash), classes); } From 1cbb44bf63876974512a1fa3fb866bcfea6250c0 Mon Sep 17 00:00:00 2001 From: adiguba Date: Fri, 21 Feb 2025 12:26:32 +0100 Subject: [PATCH 05/24] add test --- .../samples/class-directive/_config.js | 145 ++++++++++++++++++ .../samples/class-directive/main.svelte | 42 +++++ 2 files changed, 187 insertions(+) create mode 100644 packages/svelte/tests/runtime-runes/samples/class-directive/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/class-directive/main.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/class-directive/_config.js b/packages/svelte/tests/runtime-runes/samples/class-directive/_config.js new file mode 100644 index 000000000000..2756b40493e2 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-directive/_config.js @@ -0,0 +1,145 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + html: ` +
+ +
+ +
+ +
+ + +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ + `, + test({ assert, target, component }) { + component.foo = true; + flushSync(); + + assert.htmlEqual( + target.innerHTML, + ` +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ ` + ); + + component.bar = false; + flushSync(); + + assert.htmlEqual( + target.innerHTML, + ` +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ ` + ); + + component.foo = false; + flushSync(); + + assert.htmlEqual( + target.innerHTML, + ` +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/class-directive/main.svelte b/packages/svelte/tests/runtime-runes/samples/class-directive/main.svelte new file mode 100644 index 000000000000..e56ca871baf4 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-directive/main.svelte @@ -0,0 +1,42 @@ + + + +
+ +
+ +
+ +
+ + +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ + \ No newline at end of file From 2ae5869f52e122b744043614c93b0102ad25b275 Mon Sep 17 00:00:00 2001 From: adiguba Date: Fri, 21 Feb 2025 14:31:07 +0100 Subject: [PATCH 06/24] add test for mutations on hydration --- .../class-directive-mutations/_config.js | 44 +++++++++++++++++ .../class-directive-mutations/main.svelte | 49 +++++++++++++++++++ 2 files changed, 93 insertions(+) create mode 100644 packages/svelte/tests/runtime-runes/samples/class-directive-mutations/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/class-directive-mutations/main.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/_config.js b/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/_config.js new file mode 100644 index 000000000000..6e1188d4bc41 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/_config.js @@ -0,0 +1,44 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +// This test count mutations on hydration +// set_class() shoult not mutate class on hydration, except if mismatch +export default test({ + server_props: { + browser: false + }, + + props: { + browser: true + }, + + html: ` +
+
+ + + +
+ `, + + ssrHtml: ` +
+
+ + + +
+ `, + + async test({ assert, component, instance, variant }) { + // only on hydration + if (variant === 'hydrate') { + flushSync(); + assert.deepEqual(instance.get_and_clear_mutations(), ['MAIN']); + + component.foo = false; + flushSync(); + assert.deepEqual(instance.get_and_clear_mutations(), ['DIV', 'SPAN', 'B', 'I']); + } + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/main.svelte b/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/main.svelte new file mode 100644 index 000000000000..6df1e1c39cd8 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/main.svelte @@ -0,0 +1,49 @@ + + +
+
+ + + +
+ + From be008aeb857b91c070888da4b020b25c5971f5f8 Mon Sep 17 00:00:00 2001 From: adiguba Date: Fri, 21 Feb 2025 18:29:45 +0100 Subject: [PATCH 07/24] clean observer --- .../samples/class-directive-mutations/main.svelte | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/main.svelte b/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/main.svelte index 6df1e1c39cd8..b3a77e4507ea 100644 --- a/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/main.svelte @@ -1,4 +1,6 @@
From 559c1681572e9320cd63c2a32c5f3c34115f6a47 Mon Sep 17 00:00:00 2001 From: adiGuba Date: Sat, 22 Feb 2025 07:16:05 +0100 Subject: [PATCH 08/24] Update packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js unused Co-authored-by: Rich Harris --- .../phases/3-transform/client/visitors/RegularElement.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 b149efbaf1a7..4aa4361b0919 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 @@ -14,7 +14,7 @@ import { escape_html } from '../../../../../escaping.js'; import { dev, is_ignored, locator } from '../../../../state.js'; import { is_event_attribute, is_text_attribute } from '../../../../utils/ast.js'; import * as b from '../../../../utils/builders.js'; -import { create_expression_metadata, is_custom_element_node } from '../../../nodes.js'; +import { is_custom_element_node } from '../../../nodes.js'; import { clean_nodes, determine_namespace_for_children } from '../../utils.js'; import { build_getter } from '../utils.js'; import { From 203ba37d02f63f09719746cd9f03aefa374b372e Mon Sep 17 00:00:00 2001 From: adiGuba Date: Sat, 22 Feb 2025 07:23:38 +0100 Subject: [PATCH 09/24] Update packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js unused for now Co-authored-by: Rich Harris --- .../phases/3-transform/client/visitors/RegularElement.js | 1 - 1 file changed, 1 deletion(-) 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 4aa4361b0919..055b3cc66860 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 @@ -565,7 +565,6 @@ function build_element_attribute_update_assignment( attribute, attributes, class_directives, - style_directives, context ) { const state = context.state; From 412fe18fc5417db8bf5d37de20dad9534cec0c24 Mon Sep 17 00:00:00 2001 From: adiGuba Date: Sat, 22 Feb 2025 07:24:02 +0100 Subject: [PATCH 10/24] Update packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js unused for now Co-authored-by: Rich Harris --- .../phases/3-transform/client/visitors/shared/element.js | 1 - 1 file changed, 1 deletion(-) 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 b8efbca17903..2775f8fd907c 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 @@ -25,7 +25,6 @@ import { build_template_chunk, get_expression_id } from './utils.js'; export function build_set_attributes( attributes, class_directives, - style_directives, context, element, element_id, From a4b7280d3c630c42503ac2cb0c5f83c2c5f1b185 Mon Sep 17 00:00:00 2001 From: adiGuba Date: Sat, 22 Feb 2025 07:24:23 +0100 Subject: [PATCH 11/24] Update packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js nit Co-authored-by: Rich Harris --- .../phases/3-transform/server/visitors/shared/element.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js index e656506bbaf4..efdeec249955 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js @@ -378,7 +378,7 @@ function build_element_spread_attributes( /** * - * @param {string|null} hash + * @param {string | null} hash * @param {AST.ClassDirective[]} class_directives * @param {AST.Attribute | null} class_attribute * @returns From 86fb42ff2279c33b888c39ec5de355b2afdfe274 Mon Sep 17 00:00:00 2001 From: adiGuba Date: Sat, 22 Feb 2025 07:24:34 +0100 Subject: [PATCH 12/24] Update packages/svelte/src/internal/client/dom/elements/attributes.js nit Co-authored-by: Rich Harris --- .../svelte/src/internal/client/dom/elements/attributes.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index 3476f8a955ff..6acc231d2f8e 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -258,8 +258,8 @@ 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} next New attributes - this function mutates this object + * @param {Record | undefined} prev + * @param {Record} next New attributes - this function mutates this object * @param {string} [css_hash] * @param {boolean} [preserve_attribute_case] * @param {boolean} [is_custom_element] From 8f3c108f75d3d73dcfbf95a2af51bb20edb6bb7f Mon Sep 17 00:00:00 2001 From: adiGuba Date: Sat, 22 Feb 2025 07:25:12 +0100 Subject: [PATCH 13/24] Update packages/svelte/src/internal/shared/attributes.js rename clazz to value :D Co-authored-by: Rich Harris --- packages/svelte/src/internal/shared/attributes.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/shared/attributes.js b/packages/svelte/src/internal/shared/attributes.js index e08cfeb431e5..e8d9b998dc5e 100644 --- a/packages/svelte/src/internal/shared/attributes.js +++ b/packages/svelte/src/internal/shared/attributes.js @@ -47,8 +47,8 @@ export function clsx(value) { * @param {Record} [classes] * @returns {string|null} */ -export function to_class(clazz, hash, classes) { - let class_name = clazz == null ? '' : '' + clazz; +export function to_class(value, hash, classes) { + let class_name = value == null ? '' : '' + value; if (hash) { class_name = class_name ? class_name + ' ' + hash : hash; } From bf9c3052330549b69769279e5d323bbd108e7f36 Mon Sep 17 00:00:00 2001 From: adiguba Date: Sat, 22 Feb 2025 07:33:50 +0100 Subject: [PATCH 14/24] remove unused + fix JSDoc --- .../phases/3-transform/client/visitors/RegularElement.js | 3 --- .../phases/3-transform/client/visitors/SvelteElement.js | 1 - .../phases/3-transform/client/visitors/shared/element.js | 1 - packages/svelte/src/internal/shared/attributes.js | 2 +- 4 files changed, 1 insertion(+), 6 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 055b3cc66860..bacb1ada8c38 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 @@ -224,7 +224,6 @@ export function RegularElement(node, context) { build_set_attributes( attributes, class_directives, - style_directives, context, node, node_id, @@ -310,7 +309,6 @@ export function RegularElement(node, context) { attribute, attributes, class_directives, - style_directives, context ); if (is) is_attributes_reactive = true; @@ -555,7 +553,6 @@ export function build_class_directives_object(class_directives, context) { * @param {AST.Attribute} attribute * @param {Array} attributes * @param {AST.ClassDirective[]} class_directives - * @param {AST.StyleDirective[]} style_directives * @param {ComponentContext} context * @returns {boolean} */ 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 6b6260e6b3ff..679b7835c38b 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 @@ -109,7 +109,6 @@ export function SvelteElement(node, context) { is_attributes_reactive = build_set_attributes( attributes, class_directives, - style_directives, inner_context, node, 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 2775f8fd907c..eca99667de72 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 @@ -13,7 +13,6 @@ import { build_template_chunk, get_expression_id } from './utils.js'; /** * @param {Array} attributes * @param {AST.ClassDirective[]} class_directives - * @param {AST.StyleDirective[]} style_directives * @param {ComponentContext} context * @param {AST.RegularElement | AST.SvelteElement} element * @param {Identifier} element_id diff --git a/packages/svelte/src/internal/shared/attributes.js b/packages/svelte/src/internal/shared/attributes.js index e8d9b998dc5e..ecff63c36a4d 100644 --- a/packages/svelte/src/internal/shared/attributes.js +++ b/packages/svelte/src/internal/shared/attributes.js @@ -42,7 +42,7 @@ export function clsx(value) { } /** - * @param {any} clazz + * @param {any} value * @param {string|null} [hash] * @param {Record} [classes] * @returns {string|null} From ec4709f1d1258bc8e594bd39ca49aac59b78df24 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 22 Feb 2025 14:33:51 -0500 Subject: [PATCH 15/24] drive-by fix --- .../phases/3-transform/client/visitors/RegularElement.js | 3 +-- .../phases/3-transform/client/visitors/SvelteElement.js | 3 +-- .../phases/3-transform/client/visitors/shared/element.js | 4 +--- 3 files changed, 3 insertions(+), 7 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 bacb1ada8c38..b835e5f235b3 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 @@ -229,8 +229,7 @@ export function RegularElement(node, context) { node_id, attributes_id, (node.metadata.svg || node.metadata.mathml || is_custom_element_node(node)) && b.true, - is_custom_element_node(node) && b.true, - context.state + is_custom_element_node(node) && b.true ); // If value binding exists, that one takes care of calling $.init_select 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 679b7835c38b..6bfb231e83d0 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 @@ -114,8 +114,7 @@ export function SvelteElement(node, context) { element_id, attributes_id, b.binary('===', b.member(element_id, 'namespaceURI'), b.id('$.NAMESPACE_SVG')), - b.call(b.member(b.member(element_id, 'nodeName'), 'includes'), b.literal('-')), - context.state + b.call(b.member(b.member(element_id, 'nodeName'), 'includes'), b.literal('-')) ); } 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 eca99667de72..6f3ebd83e6a4 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 @@ -19,7 +19,6 @@ import { build_template_chunk, get_expression_id } from './utils.js'; * @param {Identifier} attributes_id * @param {false | Expression} preserve_attribute_case * @param {false | Expression} is_custom_element - * @param {ComponentClientTransformState} state */ export function build_set_attributes( attributes, @@ -29,8 +28,7 @@ export function build_set_attributes( element_id, attributes_id, preserve_attribute_case, - is_custom_element, - state + is_custom_element ) { let is_dynamic = false; From 2704f6135dfc1ae682d474f16fe1ee4f5848383d Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 22 Feb 2025 14:52:55 -0500 Subject: [PATCH 16/24] minor style tweaks --- .../src/compiler/phases/2-analyze/index.js | 1 + .../client/visitors/RegularElement.js | 4 ++ .../client/visitors/SvelteElement.js | 1 + .../client/visitors/shared/element.js | 15 ++++-- .../server/visitors/shared/element.js | 4 ++ .../client/dom/elements/attributes.js | 4 +- .../src/internal/client/dom/elements/class.js | 15 ++++-- packages/svelte/src/internal/server/index.js | 4 +- .../svelte/src/internal/shared/attributes.js | 11 +++-- .../class-directive-mutations/_config.js | 47 +++++++++---------- .../class-directive-mutations/main.svelte | 47 +++++++++---------- .../samples/class-directive/main.svelte | 4 +- 12 files changed, 87 insertions(+), 70 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 8dff8f4b8f31..ca9297279ee2 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -777,6 +777,7 @@ export function analyze_component(root, source, options) { let has_class = false; let has_spread = false; let has_class_directive = false; + for (const attribute of node.attributes) { // The spread method appends the hash to the end of the class attribute on its own if (attribute.type === 'SpreadAttribute') { 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 b835e5f235b3..9cc6a1e28851 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 @@ -515,13 +515,17 @@ function setup_select_synchronization(value_binding, context) { */ export function build_class_directives_object(class_directives, context) { let properties = []; + for (const d of class_directives) { let expression = /** @type Expression */ (context.visit(d.expression)); + if (d.metadata.expression.has_call) { expression = get_expression_id(context.state, expression); } + properties.push(b.init(d.name, expression)); } + return b.object(properties); } 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 6bfb231e83d0..7149f8d0e4af 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 @@ -91,6 +91,7 @@ export function SvelteElement(node, context) { context, (value, metadata) => (metadata.has_call ? get_expression_id(context.state, value) : value) ); + is_attributes_reactive = build_set_class( node, 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 6f3ebd83e6a4..fc5fd2cc4c53 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 @@ -78,6 +78,7 @@ export function build_set_attributes( build_class_directives_object(class_directives, context) ) ); + is_dynamic ||= class_directives.find((directive) => directive.metadata.expression.has_state) !== null; } @@ -216,14 +217,17 @@ export function build_set_class( /** @type {Identifier | undefined} */ let previous_id; + /** @type {ObjectExpression | Identifier | undefined} */ let prev; + /** @type {ObjectExpression | undefined} */ let next; + if (class_directives.length) { next = build_class_directives_object(class_directives, context); + has_state ||= class_directives.some((d) => d.metadata.expression.has_state); - has_state = has_state || !!class_directives.find((d) => d.metadata.expression.has_state); if (has_state) { previous_id = b.id(context.state.scope.generate('classes')); context.state.init.push(b.declaration('let', [b.declarator(previous_id)])); @@ -235,6 +239,7 @@ export function build_set_class( /** @type {Expression | undefined} */ let css_hash; + if (element.metadata.scoped && context.state.analysis.css.hash) { if (value.type === 'Literal' && (value.value === '' || value.value === null)) { value = b.literal(context.state.analysis.css.hash); @@ -244,6 +249,7 @@ export function build_set_class( css_hash = b.literal(context.state.analysis.css.hash); } } + if (!css_hash && next) { css_hash = b.null; } @@ -264,11 +270,12 @@ export function build_set_class( } const update = b.stmt(set_class); + if (has_state) { context.state.update.push(update); return true; - } else { - context.state.init.push(update); - return false; } + + context.state.init.push(update); + return false; } diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js index efdeec249955..57101af4b823 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js @@ -390,6 +390,7 @@ function build_to_class(hash, class_directives, class_attribute) { /** @type {ObjectExpression | undefined} */ let classes; + if (class_directives.length) { classes = b.object( class_directives.map((directive) => @@ -400,6 +401,7 @@ function build_to_class(hash, class_directives, class_attribute) { /** @type {Expression} */ let class_name; + if (class_attribute.value === true) { class_name = b.literal(''); } else if (Array.isArray(class_attribute.value)) { @@ -431,6 +433,7 @@ function build_to_class(hash, class_directives, class_attribute) { } else { expression = b.call('$.to_class', class_name, b.literal(hash), classes); } + class_attribute.value = { type: 'ExpressionTag', start: -1, @@ -440,6 +443,7 @@ function build_to_class(hash, class_directives, class_attribute) { expression: create_expression_metadata() } }; + return class_attribute; } diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index 6acc231d2f8e..151024e85c2e 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -327,9 +327,7 @@ export function set_attributes( } var prev_value = current[key]; - if (value === prev_value && key !== 'class') { - continue; - } + if (value === prev_value && key !== 'class') continue; current[key] = value; diff --git a/packages/svelte/src/internal/client/dom/elements/class.js b/packages/svelte/src/internal/client/dom/elements/class.js index a11f615cdd45..5cd1c590f218 100644 --- a/packages/svelte/src/internal/client/dom/elements/class.js +++ b/packages/svelte/src/internal/client/dom/elements/class.js @@ -3,12 +3,12 @@ import { hydrating } from '../hydration.js'; /** * @param {Element} dom - * @param {boolean|number} is_html - * @param {string|null} value + * @param {boolean | number} is_html + * @param {string | null} value * @param {string} [hash] - * @param {Record} [prev_classes] - * @param {Record} [next_classes] - * @returns {Record|undefined} + * @param {Record} [prev_classes] + * @param {Record} [next_classes] + * @returns {Record | undefined} */ export function set_class(dom, is_html, value, hash, prev_classes, next_classes) { // @ts-expect-error need to add __className to patched prototype @@ -16,6 +16,7 @@ export function set_class(dom, is_html, value, hash, prev_classes, next_classes) if (hydrating || prev !== value) { var next_class_name = to_class(value, hash, next_classes); + if (!hydrating || next_class_name !== dom.getAttribute('class')) { // Removing the attribute when the value is only an empty string causes // performance issues vs simply making the className an empty string. So @@ -29,12 +30,15 @@ export function set_class(dom, is_html, value, hash, prev_classes, next_classes) dom.setAttribute('class', next_class_name); } } + // @ts-expect-error need to add __className to patched prototype dom.__className = value; } else if (next_classes) { prev_classes = prev_classes ?? {}; + for (const key in next_classes) { const is_present = !!next_classes[key]; + if (is_present !== !!prev_classes[key]) { // TODO : use dom.classList.toggle instead ? if (is_present) { @@ -45,5 +49,6 @@ export function set_class(dom, is_html, value, hash, prev_classes, next_classes) } } } + return next_classes; } diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index 1995985b56a7..160a1faa653e 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -10,7 +10,6 @@ import { ELEMENT_PRESERVE_ATTRIBUTE_CASE, ELEMENT_IS_NAMESPACED } from '../../constants.js'; - import { escape_html } from '../../escaping.js'; import { DEV } from 'esm-env'; import { current_component, pop, push } from './context.js'; @@ -198,7 +197,7 @@ export function css_props(payload, is_html, props, component, dynamic = false) { /** * @param {Record} attrs - * @param {string|null} css_hash + * @param {string | null} css_hash * @param {Record} [classes] * @param {Record} [styles] * @param {number} [flags] @@ -214,6 +213,7 @@ export function spread_attributes(attrs, css_hash, classes, styles, flags = 0) { if (attrs.class) { attrs.class = clsx(attrs.class); } + if (css_hash || classes) { attrs.class = to_class(attrs.class, css_hash, classes); } diff --git a/packages/svelte/src/internal/shared/attributes.js b/packages/svelte/src/internal/shared/attributes.js index ecff63c36a4d..7d8b49d2ecbe 100644 --- a/packages/svelte/src/internal/shared/attributes.js +++ b/packages/svelte/src/internal/shared/attributes.js @@ -49,19 +49,24 @@ export function clsx(value) { */ export function to_class(value, hash, classes) { let class_name = value == null ? '' : '' + value; + if (hash) { class_name = class_name ? class_name + ' ' + hash : hash; } + if (classes) { const white_spaces = ' \t\n\r\f\u00a0\u000b\ufeff'; + for (const key in classes) { if (classes[key]) { class_name = class_name ? class_name + ' ' + key : key; } else if (class_name.length) { const len = key.length; let start = 0; + while ((start = class_name.indexOf(key, start)) >= 0) { let stop = start + len; + if ( white_spaces.indexOf(class_name[start - 1] ?? ' ') >= 0 && white_spaces.indexOf(class_name[stop] ?? ' ') >= 0 @@ -78,8 +83,6 @@ export function to_class(value, hash, classes) { } } } - if (class_name === '') { - return null; - } - return class_name; + + return class_name === '' ? null : class_name; } diff --git a/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/_config.js b/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/_config.js index 6e1188d4bc41..77e46b7582c2 100644 --- a/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/_config.js @@ -1,9 +1,11 @@ import { flushSync } from 'svelte'; import { test } from '../../test'; -// This test count mutations on hydration +// This test counts mutations on hydration // set_class() shoult not mutate class on hydration, except if mismatch export default test({ + mode: ['server', 'hydrate'], + server_props: { browser: false }, @@ -13,32 +15,29 @@ export default test({ }, html: ` -
-
- - - -
- `, +
+
+ + + +
+ `, ssrHtml: ` -
-
- - - -
- `, +
+
+ + + +
+ `, - async test({ assert, component, instance, variant }) { - // only on hydration - if (variant === 'hydrate') { - flushSync(); - assert.deepEqual(instance.get_and_clear_mutations(), ['MAIN']); + async test({ assert, component, instance }) { + flushSync(); + assert.deepEqual(instance.get_and_clear_mutations(), ['MAIN']); - component.foo = false; - flushSync(); - assert.deepEqual(instance.get_and_clear_mutations(), ['DIV', 'SPAN', 'B', 'I']); - } + component.foo = false; + flushSync(); + assert.deepEqual(instance.get_and_clear_mutations(), ['DIV', 'SPAN', 'B', 'I']); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/main.svelte b/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/main.svelte index b3a77e4507ea..802904379d0c 100644 --- a/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/main.svelte @@ -2,40 +2,37 @@ import { onDestroy } from "svelte"; let { - clazz = 'custom', - foo = true, - bar = true, - browser - } = $props(); + clazz = 'custom', + foo = true, + bar = true, + browser + } = $props(); - let mutations = []; - let observer; + let mutations = []; + let observer; - if (browser) { + if (browser) { observer = new MutationObserver(updateMutationRecords); - const main = document.querySelector('main#main'); - if (main) { - observer.observe(main, { attributes: true, subtree: true }); - } - + const main = document.querySelector('main#main'); + if (main) { + observer.observe(main, { attributes: true, subtree: true }); + } } - - function updateMutationRecords(results) { - for (const r of results) { + function updateMutationRecords(results) { + for (const r of results) { mutations.push(r.target.nodeName); } - } - - export function get_and_clear_mutations() { - updateMutationRecords(observer.takeRecords()); - const result = mutations; - mutations = []; - return result; - } + } - onDestroy(() => { if (observer) observer.disconnect(); }); + export function get_and_clear_mutations() { + updateMutationRecords(observer.takeRecords()); + const result = mutations; + mutations = []; + return result; + } + onDestroy(() => { if (observer) observer.disconnect(); });
diff --git a/packages/svelte/tests/runtime-runes/samples/class-directive/main.svelte b/packages/svelte/tests/runtime-runes/samples/class-directive/main.svelte index e56ca871baf4..966c07a78e8d 100644 --- a/packages/svelte/tests/runtime-runes/samples/class-directive/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/class-directive/main.svelte @@ -2,7 +2,6 @@ let { foo = false, bar = true } = $props(); -
@@ -11,7 +10,6 @@
-
@@ -39,4 +37,4 @@ div > span { font-weight: bold; } - \ No newline at end of file + From f96124b6e0567b7cbf717fb37b9e1274c67ba34e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 22 Feb 2025 15:01:35 -0500 Subject: [PATCH 17/24] tweak test --- .../class-directive-mutations/main.svelte | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/main.svelte b/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/main.svelte index 802904379d0c..825362dcaf82 100644 --- a/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/main.svelte @@ -1,8 +1,6 @@
-
+
- +
From a5ff0bb02a8be5e2450e5ea3884254e9e7982b8e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 22 Feb 2025 15:38:10 -0500 Subject: [PATCH 18/24] this is faster --- packages/svelte/src/internal/shared/attributes.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/svelte/src/internal/shared/attributes.js b/packages/svelte/src/internal/shared/attributes.js index 7d8b49d2ecbe..18f640627927 100644 --- a/packages/svelte/src/internal/shared/attributes.js +++ b/packages/svelte/src/internal/shared/attributes.js @@ -41,6 +41,8 @@ export function clsx(value) { } } +const white_spaces = [...' \t\n\r\f\u00a0\u000b\ufeff']; + /** * @param {any} value * @param {string|null} [hash] @@ -55,8 +57,6 @@ export function to_class(value, hash, classes) { } if (classes) { - const white_spaces = ' \t\n\r\f\u00a0\u000b\ufeff'; - for (const key in classes) { if (classes[key]) { class_name = class_name ? class_name + ' ' + key : key; @@ -68,8 +68,8 @@ export function to_class(value, hash, classes) { let stop = start + len; if ( - white_spaces.indexOf(class_name[start - 1] ?? ' ') >= 0 && - white_spaces.indexOf(class_name[stop] ?? ' ') >= 0 + white_spaces.includes(class_name[start - 1] ?? ' ') && + white_spaces.includes(class_name[stop] ?? ' ') ) { class_name = ( class_name.substring(0, start).trim() + From 395debd8b2c6fc78986253d8ee90cd1ee1451e44 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Sat, 22 Feb 2025 16:20:35 -0500 Subject: [PATCH 19/24] tweak --- packages/svelte/src/internal/shared/attributes.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/svelte/src/internal/shared/attributes.js b/packages/svelte/src/internal/shared/attributes.js index 18f640627927..e667623837a5 100644 --- a/packages/svelte/src/internal/shared/attributes.js +++ b/packages/svelte/src/internal/shared/attributes.js @@ -45,20 +45,20 @@ const white_spaces = [...' \t\n\r\f\u00a0\u000b\ufeff']; /** * @param {any} value - * @param {string|null} [hash] - * @param {Record} [classes] - * @returns {string|null} + * @param {string | null} [hash] + * @param {Record} [directives] + * @returns {string | null} */ -export function to_class(value, hash, classes) { +export function to_class(value, hash, directives) { let class_name = value == null ? '' : '' + value; if (hash) { class_name = class_name ? class_name + ' ' + hash : hash; } - if (classes) { - for (const key in classes) { - if (classes[key]) { + if (directives) { + for (const key in directives) { + if (directives[key]) { class_name = class_name ? class_name + ' ' + key : key; } else if (class_name.length) { const len = key.length; From 501f726efb023147b4330082202104433f4354cd Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 24 Feb 2025 07:04:20 -0500 Subject: [PATCH 20/24] tweak --- .../svelte/src/internal/shared/attributes.js | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/svelte/src/internal/shared/attributes.js b/packages/svelte/src/internal/shared/attributes.js index e667623837a5..cd2b00ec9c60 100644 --- a/packages/svelte/src/internal/shared/attributes.js +++ b/packages/svelte/src/internal/shared/attributes.js @@ -41,7 +41,7 @@ export function clsx(value) { } } -const white_spaces = [...' \t\n\r\f\u00a0\u000b\ufeff']; +const whitespace = [...' \t\n\r\f\u00a0\u000b\ufeff']; /** * @param {any} value @@ -50,39 +50,39 @@ const white_spaces = [...' \t\n\r\f\u00a0\u000b\ufeff']; * @returns {string | null} */ export function to_class(value, hash, directives) { - let class_name = value == null ? '' : '' + value; + var classname = value == null ? '' : '' + value; if (hash) { - class_name = class_name ? class_name + ' ' + hash : hash; + classname = classname ? classname + ' ' + hash : hash; } if (directives) { - for (const key in directives) { + for (var key in directives) { if (directives[key]) { - class_name = class_name ? class_name + ' ' + key : key; - } else if (class_name.length) { - const len = key.length; - let start = 0; + classname = classname ? classname + ' ' + key : key; + } else if (classname.length) { + var len = key.length; + var a = 0; - while ((start = class_name.indexOf(key, start)) >= 0) { - let stop = start + len; + while ((a = classname.indexOf(key, a)) >= 0) { + var b = a + len; if ( - white_spaces.includes(class_name[start - 1] ?? ' ') && - white_spaces.includes(class_name[stop] ?? ' ') + (a === 0 || whitespace.includes(classname[a - 1])) && + (b === classname.length || whitespace.includes(classname[b])) ) { - class_name = ( - class_name.substring(0, start).trim() + + classname = ( + classname.substring(0, a).trim() + ' ' + - class_name.substring(stop).trim() + classname.substring(b).trim() ).trim(); } else { - start = stop; + a = b; } } } } } - return class_name === '' ? null : class_name; + return classname === '' ? null : classname; } From 296193b293cef38dce2baa972e35a4a03019385a Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 24 Feb 2025 07:22:55 -0500 Subject: [PATCH 21/24] this is faster --- packages/svelte/src/internal/shared/attributes.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/svelte/src/internal/shared/attributes.js b/packages/svelte/src/internal/shared/attributes.js index cd2b00ec9c60..89cc17e51b9d 100644 --- a/packages/svelte/src/internal/shared/attributes.js +++ b/packages/svelte/src/internal/shared/attributes.js @@ -71,11 +71,7 @@ export function to_class(value, hash, directives) { (a === 0 || whitespace.includes(classname[a - 1])) && (b === classname.length || whitespace.includes(classname[b])) ) { - classname = ( - classname.substring(0, a).trim() + - ' ' + - classname.substring(b).trim() - ).trim(); + classname = (a === 0 ? '' : classname.substring(0, a)) + classname.substring(b + 1); } else { a = b; } From b9f90bd7511b2ea6fadbe7c37654299055d4b537 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 24 Feb 2025 07:36:01 -0500 Subject: [PATCH 22/24] typo --- .../runtime-runes/samples/class-directive-mutations/_config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/_config.js b/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/_config.js index 77e46b7582c2..dd1bc6ac1a79 100644 --- a/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/_config.js @@ -2,7 +2,7 @@ import { flushSync } from 'svelte'; import { test } from '../../test'; // This test counts mutations on hydration -// set_class() shoult not mutate class on hydration, except if mismatch +// set_class() should not mutate class on hydration, except if mismatch export default test({ mode: ['server', 'hydrate'], From 456be79562e56379cb440aa9546d8eadc9223061 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 24 Feb 2025 07:40:54 -0500 Subject: [PATCH 23/24] tweak --- .../src/internal/client/dom/elements/class.js | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/elements/class.js b/packages/svelte/src/internal/client/dom/elements/class.js index 5cd1c590f218..3308709a247f 100644 --- a/packages/svelte/src/internal/client/dom/elements/class.js +++ b/packages/svelte/src/internal/client/dom/elements/class.js @@ -34,18 +34,13 @@ export function set_class(dom, is_html, value, hash, prev_classes, next_classes) // @ts-expect-error need to add __className to patched prototype dom.__className = value; } else if (next_classes) { - prev_classes = prev_classes ?? {}; + prev_classes ??= {}; - for (const key in next_classes) { - const is_present = !!next_classes[key]; + for (var key in next_classes) { + var is_present = !!next_classes[key]; if (is_present !== !!prev_classes[key]) { - // TODO : use dom.classList.toggle instead ? - if (is_present) { - dom.classList.add(key); - } else { - dom.classList.remove(key); - } + dom.classList.toggle(key, is_present); } } } From b4bcaf430b393afd79fb62a565816f9f4e4730a2 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 24 Feb 2025 07:42:05 -0500 Subject: [PATCH 24/24] changeset --- .changeset/stale-plums-drop.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/stale-plums-drop.md diff --git a/.changeset/stale-plums-drop.md b/.changeset/stale-plums-drop.md new file mode 100644 index 000000000000..d39268eb729b --- /dev/null +++ b/.changeset/stale-plums-drop.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: correctly override class attributes with class directives