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 diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 93c438c8529f..ca9297279ee2 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -767,66 +767,40 @@ 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; - } + for (const node of analysis.elements) { + if (node.metadata.scoped && is_custom_element_node(node)) { + mark_subtree_dynamic(node.metadata.path); + } - 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; + let has_class = false; + let has_spread = false; + let has_class_directive = false; - class_attribute = attribute; - } + 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]; - } - } - } 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); + // 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 } - } - } + ]) + ); } } 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..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 @@ -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' */ @@ -20,9 +20,9 @@ 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,13 +223,13 @@ export function RegularElement(node, context) { build_set_attributes( attributes, + class_directives, context, node, 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 @@ -270,13 +270,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 +299,22 @@ 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, + 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 +508,27 @@ 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 +555,7 @@ function setup_select_synchronization(value_binding, context) { * @param {Identifier} node_id * @param {AST.Attribute} attribute * @param {Array} attributes + * @param {AST.ClassDirective[]} class_directives * @param {ComponentContext} context * @returns {boolean} */ @@ -526,6 +564,7 @@ function build_element_attribute_update_assignment( node_id, attribute, attributes, + class_directives, context ) { const state = context.state; @@ -564,19 +603,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 +675,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..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 @@ -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,31 +80,46 @@ 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 + 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], + value, + has_state, + 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, inner_context, node, 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('-')) ); } - // 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..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 @@ -1,32 +1,34 @@ /** @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'; 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 {ComponentContext} context * @param {AST.RegularElement | AST.SvelteElement} element * @param {Identifier} element_id * @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, + class_directives, context, element, element_id, attributes_id, preserve_attribute_case, - is_custom_element, - state + is_custom_element ) { let is_dynamic = false; @@ -68,6 +70,19 @@ 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 +149,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 +189,93 @@ 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 ||= class_directives.some((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 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); + } + } + + 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; + } + + 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..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 @@ -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,82 @@ 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 === '' || typeof class_name.value === 'string') + ) { + 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); + } + + 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..151024e85c2e 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,7 @@ 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 +377,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..3308709a247f 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; - } -} + dom.__className = value; + } else if (next_classes) { + prev_classes ??= {}; -/** - * @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); + for (var key in next_classes) { + var is_present = !!next_classes[key]; - 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; + if (is_present !== !!prev_classes[key]) { + dom.classList.toggle(key, is_present); + } } - - // @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..160a1faa653e 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 { @@ -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,12 +197,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)) @@ -214,16 +214,8 @@ export function spread_attributes(attrs, classes, styles, flags = 0) { 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..89cc17e51b9d 100644 --- a/packages/svelte/src/internal/shared/attributes.js +++ b/packages/svelte/src/internal/shared/attributes.js @@ -40,3 +40,45 @@ export function clsx(value) { return value ?? ''; } } + +const whitespace = [...' \t\n\r\f\u00a0\u000b\ufeff']; + +/** + * @param {any} value + * @param {string | null} [hash] + * @param {Record} [directives] + * @returns {string | null} + */ +export function to_class(value, hash, directives) { + var classname = value == null ? '' : '' + value; + + if (hash) { + classname = classname ? classname + ' ' + hash : hash; + } + + if (directives) { + for (var key in directives) { + if (directives[key]) { + classname = classname ? classname + ' ' + key : key; + } else if (classname.length) { + var len = key.length; + var a = 0; + + while ((a = classname.indexOf(key, a)) >= 0) { + var b = a + len; + + if ( + (a === 0 || whitespace.includes(classname[a - 1])) && + (b === classname.length || whitespace.includes(classname[b])) + ) { + classname = (a === 0 ? '' : classname.substring(0, a)) + classname.substring(b + 1); + } else { + a = b; + } + } + } + } + } + + return classname === '' ? null : classname; +} 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'); } }); 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..dd1bc6ac1a79 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/_config.js @@ -0,0 +1,43 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +// This test counts mutations on hydration +// set_class() should not mutate class on hydration, except if mismatch +export default test({ + mode: ['server', 'hydrate'], + + server_props: { + browser: false + }, + + props: { + browser: true + }, + + html: ` +
+
+ + + +
+ `, + + ssrHtml: ` +
+
+ + + +
+ `, + + 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']); + } +}); 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..825362dcaf82 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-directive-mutations/main.svelte @@ -0,0 +1,47 @@ + + +
+
+ + + +
+ + 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..966c07a78e8d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/class-directive/main.svelte @@ -0,0 +1,40 @@ + + +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +