diff --git a/.changeset/seven-deers-jam.md b/.changeset/seven-deers-jam.md new file mode 100644 index 0000000000..cdb0c8d16b --- /dev/null +++ b/.changeset/seven-deers-jam.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: better attribute casing logic diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index d18c93e54d..68328e5453 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js @@ -12,13 +12,7 @@ import { escape_html, infer_namespace } from '../../utils.js'; -import { - AttributeAliases, - DOMBooleanAttributes, - DOMProperties, - PassiveEvents, - VoidElements -} from '../../../constants.js'; +import { DOMProperties, PassiveEvents, VoidElements } from '../../../constants.js'; import { is_custom_element_node, is_element_node } from '../../../nodes.js'; import * as b from '../../../../utils/builders.js'; import { error } from '../../../../errors.js'; @@ -29,6 +23,8 @@ import { serialize_set_binding } from '../utils.js'; import { + AttributeAliases, + DOMBooleanAttributes, EACH_INDEX_REACTIVE, EACH_IS_CONTROLLED, EACH_ITEM_REACTIVE, @@ -37,6 +33,26 @@ import { import { regex_is_valid_identifier } from '../../../patterns.js'; import { javascript_visitors_runes } from './javascript-runes.js'; +/** + * @param {import('#compiler').RegularElement | import('#compiler').SvelteElement} element + * @param {import('#compiler').Attribute} attribute + * @param {{ state: { metadata: { namespace: import('#compiler').Namespace }}}} context + */ +function get_attribute_name(element, attribute, context) { + let name = attribute.name; + if ( + element.type === 'RegularElement' && + !element.metadata.svg && + context.state.metadata.namespace !== 'foreign' + ) { + name = name.toLowerCase(); + if (name in AttributeAliases) { + name = AttributeAliases[name]; + } + } + return name; +} + /** * Serializes each style directive into something like `$.style(element, style_property, value)` * and adds it either to init or update, depending on whether or not the value or the attributes are dynamic. @@ -259,10 +275,11 @@ function setup_select_synchronization(value_binding, context) { * Returns the id of the spread_attribute variable if spread is deemed reactive, `null` otherwise. * @param {Array} attributes * @param {import('../types.js').ComponentContext} context + * @param {import('#compiler').RegularElement} element * @param {import('estree').Identifier} element_id * @returns {string | null} */ -function serialize_element_spread_attributes(attributes, context, element_id) { +function serialize_element_spread_attributes(attributes, context, element, element_id) { let is_reactive = false; /** @type {import('estree').Expression[]} */ @@ -270,7 +287,7 @@ function serialize_element_spread_attributes(attributes, context, element_id) { for (const attribute of attributes) { if (attribute.type === 'Attribute') { - const name = get_attribute_name(attribute, context.state); + const name = get_attribute_name(element, attribute, context); // TODO: handle contains_call_expression const [, value] = serialize_attribute_value(attribute.value, context); values.push(b.object([b.init(name, value)])); @@ -281,6 +298,9 @@ function serialize_element_spread_attributes(attributes, context, element_id) { is_reactive ||= attribute.metadata.dynamic; } + const lowercase_attributes = + element.metadata.svg || is_custom_element_node(element) ? b.false : b.true; + if (is_reactive) { const id = context.state.scope.generate('spread_attributes'); context.state.init.push(b.let(id, undefined)); @@ -294,6 +314,7 @@ function serialize_element_spread_attributes(attributes, context, element_id) { element_id, b.id(id), b.array(values), + lowercase_attributes, b.literal(context.state.analysis.stylesheet.id) ) ) @@ -308,6 +329,7 @@ function serialize_element_spread_attributes(attributes, context, element_id) { element_id, b.literal(null), b.array(values), + lowercase_attributes, b.literal(context.state.analysis.stylesheet.id) ) ) @@ -398,14 +420,15 @@ function serialize_dynamic_element_spread_attributes(attributes, context, elemen * }); * ``` * Returns true if attribute is deemed reactive, false otherwise. + * @param {import('#compiler').RegularElement} element * @param {import('estree').Identifier} node_id * @param {import('#compiler').Attribute} attribute * @param {import('../types.js').ComponentContext} context * @returns {boolean} */ -function serialize_element_attribute_update_assignment(node_id, attribute, context) { +function serialize_element_attribute_update_assignment(element, node_id, attribute, context) { const state = context.state; - const name = get_attribute_name(attribute, state); + const name = get_attribute_name(element, attribute, context); let [contains_call_expression, value] = serialize_attribute_value(attribute.value, context); // The foreign namespace doesn't have any special handling, everything goes through the attr function @@ -672,21 +695,6 @@ function collect_parent_each_blocks(context) { ); } -/** - * @param {import('#compiler').Attribute} attribute - * @param {import('../types.js').ComponentClientTransformState} state - */ -function get_attribute_name(attribute, state) { - let name = attribute.name; - if (state.metadata.namespace !== 'foreign') { - name = name.toLowerCase(); - if (name !== 'class' && name in AttributeAliases) { - name = AttributeAliases[name]; - } - } - return name; -} - /** * @param {import('#compiler').Component | import('#compiler').SvelteComponent | import('#compiler').SvelteSelf} node * @param {string} component_name @@ -1899,7 +1907,7 @@ export const template_visitors = { // Then do attributes let is_attributes_reactive = false; if (node.metadata.has_spread) { - const spread_id = serialize_element_spread_attributes(attributes, context, node_id); + const spread_id = serialize_element_spread_attributes(attributes, context, node, node_id); if (child_metadata.namespace !== 'foreign') { add_select_to_spread_update(spread_id, node, context, node_id); } @@ -1920,7 +1928,7 @@ export const template_visitors = { attribute.name !== 'autofocus' && (attribute.value === true || is_text_attribute(attribute)) ) { - const name = get_attribute_name(attribute, context.state); + const name = get_attribute_name(node, attribute, context); const literal_value = /** @type {import('estree').Literal} */ ( serialize_attribute_value(attribute.value, context)[1] ).value; @@ -1941,7 +1949,7 @@ export const template_visitors = { const is = is_custom_element && child_metadata.namespace !== 'foreign' ? serialize_custom_element_attribute_update_assignment(node_id, attribute, context) - : serialize_element_attribute_update_assignment(node_id, attribute, context); + : serialize_element_attribute_update_assignment(node, node_id, attribute, context); if (is) is_attributes_reactive = true; } } diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js index 78356d8f5f..505ddeaed1 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js @@ -5,7 +5,6 @@ import * as b from '../../../utils/builders.js'; import is_reference from 'is-reference'; import { ContentEditableBindings, - DOMBooleanAttributes, VoidElements, WhitespaceInsensitiveAttributes } from '../../constants.js'; @@ -15,11 +14,12 @@ import { escape_html, infer_namespace } from '../utils.js'; -import { create_attribute, is_element_node } from '../../nodes.js'; +import { create_attribute, is_custom_element_node, is_element_node } from '../../nodes.js'; import { error } from '../../../errors.js'; import { binding_properties } from '../../bindings.js'; import { regex_starts_with_newline, regex_whitespaces_strict } from '../../patterns.js'; import { remove_types } from '../typescript.js'; +import { DOMBooleanAttributes } from '../../../../constants.js'; /** * @param {string} value @@ -471,6 +471,25 @@ function serialize_set_binding(node, context, fallback) { return fallback(); } +/** + * @param {import('#compiler').RegularElement | import('#compiler').SvelteElement} element + * @param {import('#compiler').Attribute} attribute + * @param {{ state: { metadata: { namespace: import('#compiler').Namespace }}}} context + */ +function get_attribute_name(element, attribute, context) { + let name = attribute.name; + if ( + element.type === 'RegularElement' && + !element.metadata.svg && + context.state.metadata.namespace !== 'foreign' + ) { + name = name.toLowerCase(); + // don't lookup boolean aliases here, the server runtime function does only + // check for the lowercase variants of boolean attributes + } + return name; +} + /** @type {import('./types').Visitors} */ const global_visitors = { Identifier(node, { path, state }) { @@ -690,12 +709,14 @@ function serialize_attribute_value( /** * + * @param {import('#compiler').RegularElement | import('#compiler').SvelteElement} element * @param {Array} attributes * @param {import('#compiler').StyleDirective[]} style_directives * @param {import('#compiler').ClassDirective[]} class_directives * @param {import('./types').ComponentContext} context */ function serialize_element_spread_attributes( + element, attributes, style_directives, class_directives, @@ -706,7 +727,7 @@ function serialize_element_spread_attributes( for (const attribute of attributes) { if (attribute.type === 'Attribute') { - const name = attribute.name.toLowerCase(); + const name = get_attribute_name(element, attribute, context); const value = serialize_attribute_value( attribute.value, context, @@ -718,8 +739,18 @@ function serialize_element_spread_attributes( } } + const lowercase_attributes = + element.type !== 'RegularElement' || element.metadata.svg || is_custom_element_node(element) + ? b.false + : b.true; + const is_svg = element.type === 'RegularElement' && element.metadata.svg ? b.true : b.false; /** @type {import('estree').Expression[]} */ - const args = [b.array(values), b.literal(context.state.analysis.stylesheet.id)]; + const args = [ + b.array(values), + lowercase_attributes, + is_svg, + b.literal(context.state.analysis.stylesheet.id) + ]; if (style_directives.length > 0 || class_directives.length > 0) { const styles = style_directives.map((directive) => @@ -1760,11 +1791,17 @@ function serialize_element_attributes(node, context) { context.state.init.push(...lets); if (has_spread) { - serialize_element_spread_attributes(attributes, style_directives, class_directives, context); + serialize_element_spread_attributes( + node, + attributes, + style_directives, + class_directives, + context + ); } else { for (const attribute of /** @type {import('#compiler').Attribute[]} */ (attributes)) { if (attribute.value === true || is_text_attribute(attribute)) { - const name = attribute.name.toLowerCase(); + const name = get_attribute_name(node, attribute, context); const literal_value = /** @type {import('estree').Literal} */ ( serialize_attribute_value( attribute.value, @@ -1786,7 +1823,7 @@ function serialize_element_attributes(node, context) { continue; } - const name = attribute.name.toLowerCase(); + const name = get_attribute_name(node, attribute, context); const is_boolean = DOMBooleanAttributes.includes(name); const value = serialize_attribute_value( attribute.value, diff --git a/packages/svelte/src/compiler/phases/constants.js b/packages/svelte/src/compiler/phases/constants.js index 394168ea53..0b06c5eda0 100644 --- a/packages/svelte/src/compiler/phases/constants.js +++ b/packages/svelte/src/compiler/phases/constants.js @@ -1,52 +1,12 @@ -export const DOMBooleanAttributes = [ - 'allowfullscreen', - 'async', - 'autofocus', - 'autoplay', - 'checked', - 'controls', - 'default', - 'disabled', - 'formnovalidate', - 'hidden', - 'indeterminate', - 'ismap', - 'loop', - 'multiple', - 'muted', - 'nomodule', - 'novalidate', - 'open', - 'playsinline', - 'readonly', - 'required', - 'reversed', - 'seamless', - 'selected' -]; +import { AttributeAliases, DOMBooleanAttributes } from '../../constants.js'; export const DOMProperties = [ - 'className', + ...Object.values(AttributeAliases), 'value', - 'readOnly', - 'formNoValidate', - 'isMap', - 'noModule', - 'playsInline', 'inert', ...DOMBooleanAttributes ]; -/** @type {Record} */ -export const AttributeAliases = { - class: 'className', - formnovalidate: 'formNoValidate', - ismap: 'isMap', - nomodule: 'noModule', - playsinline: 'playsInline', - readonly: 'readOnly' -}; - export const VoidElements = [ 'area', 'base', diff --git a/packages/svelte/src/constants.js b/packages/svelte/src/constants.js index 17b91d1f11..a8abda2237 100644 --- a/packages/svelte/src/constants.js +++ b/packages/svelte/src/constants.js @@ -32,3 +32,48 @@ export const DelegatedEvents = [ /** List of Element events that will be delegated and are passive */ export const PassiveDelegatedEvents = ['touchstart', 'touchmove', 'touchend']; + +/** + * @type {Record} + * List of attribute names that should be aliased to their property names + * because they behave differently between setting them as an attribute and + * setting them as a property. + */ +export const AttributeAliases = { + // no `class: 'className'` because we handle that separately + formnovalidate: 'formNoValidate', + ismap: 'isMap', + nomodule: 'noModule', + playsinline: 'playsInline', + readonly: 'readOnly' +}; + +/** + * Attributes that are boolean, i.e. they are present or not present. + */ +export const DOMBooleanAttributes = [ + 'allowfullscreen', + 'async', + 'autofocus', + 'autoplay', + 'checked', + 'controls', + 'default', + 'disabled', + 'formnovalidate', + 'hidden', + 'indeterminate', + 'ismap', + 'loop', + 'multiple', + 'muted', + 'nomodule', + 'novalidate', + 'open', + 'playsinline', + 'readonly', + 'required', + 'reversed', + 'seamless', + 'selected' +]; diff --git a/packages/svelte/src/internal/client/render.js b/packages/svelte/src/internal/client/render.js index a20d6126a3..a86c05a616 100644 --- a/packages/svelte/src/internal/client/render.js +++ b/packages/svelte/src/internal/client/render.js @@ -27,7 +27,8 @@ import { EACH_INDEX_REACTIVE, EACH_ITEM_REACTIVE, PassiveDelegatedEvents, - DelegatedEvents + DelegatedEvents, + AttributeAliases } from '../../constants.js'; import { create_fragment_from_html, @@ -2702,10 +2703,11 @@ function get_setters(element) { * @param {Element & ElementCSSInlineStyle} dom * @param {Record | null} prev * @param {Record[]} attrs + * @param {boolean} lowercase_attributes * @param {string} css_hash * @returns {Record} */ -export function spread_attributes(dom, prev, attrs, css_hash) { +export function spread_attributes(dom, prev, attrs, lowercase_attributes, css_hash) { const next = Object.assign({}, ...attrs); const has_hash = css_hash.length !== 0; for (const key in prev) { @@ -2724,13 +2726,13 @@ export function spread_attributes(dom, prev, attrs, css_hash) { let value = next[key]; if (value === prev?.[key]) continue; - const prefix = key.slice(0, 2); + const prefix = key[0] + key[1]; // this is faster than key.slice(0, 2) if (prefix === '$$') continue; if (prefix === 'on') { /** @type {{ capture?: true }} */ const opts = {}; - let event_name = key.slice(2).toLowerCase(); + let event_name = key.slice(2); const delegated = DelegatedEvents.includes(event_name); if ( @@ -2762,25 +2764,33 @@ export function spread_attributes(dom, prev, attrs, css_hash) { } else if (key === '__value' || key === 'value') { // @ts-ignore dom.value = dom[key] = dom.__value = value; - } else if (setters.includes(key)) { - if (DEV) { - check_src_in_dev_hydration(dom, key, value); - } - if ( - current_hydration_fragment === null || - // @ts-ignore see attr method for an explanation of src/srcset - (dom[key] !== value && key !== 'src' && key !== 'srcset') - ) { - // @ts-ignore - dom[key] = value; - } - } else if (typeof value !== 'function') { - if (has_hash && key === 'class') { - if (value) value += ' '; - value += css_hash; + } else { + let name = key; + if (lowercase_attributes) { + name = name.toLowerCase(); + name = AttributeAliases[name] || name; } - attr(dom, key, value); + if (setters.includes(name)) { + if (DEV) { + check_src_in_dev_hydration(dom, name, value); + } + if ( + current_hydration_fragment === null || + // @ts-ignore see attr method for an explanation of src/srcset + (dom[name] !== value && name !== 'src' && name !== 'srcset') + ) { + // @ts-ignore + dom[name] = value; + } + } else if (typeof value !== 'function') { + if (has_hash && name === 'class') { + if (value) value += ' '; + value += css_hash; + } + + attr(dom, name, value); + } } } return next; @@ -2811,6 +2821,7 @@ export function spread_dynamic_element_attributes(node, prev, attrs, css_hash) { /** @type {Element & ElementCSSInlineStyle} */ (node), prev, attrs, + node.namespaceURI !== 'http://www.w3.org/2000/svg', css_hash ); } diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index 7d57737529..4fe85992f4 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -2,6 +2,7 @@ import * as $ from '../client/runtime.js'; import { set_is_ssr } from '../client/runtime.js'; import { is_promise } from '../common.js'; import { subscribe_to_store } from '../../store/utils.js'; +import { DOMBooleanAttributes } from '../../constants.js'; export * from '../client/validate.js'; @@ -31,34 +32,6 @@ const CONTENT_REGEX = /[&<]/g; const INVALID_ATTR_NAME_CHAR_REGEX = /[\s'">/=\u{FDD0}-\u{FDEF}\u{FFFE}\u{FFFF}\u{1FFFE}\u{1FFFF}\u{2FFFE}\u{2FFFF}\u{3FFFE}\u{3FFFF}\u{4FFFE}\u{4FFFF}\u{5FFFE}\u{5FFFF}\u{6FFFE}\u{6FFFF}\u{7FFFE}\u{7FFFF}\u{8FFFE}\u{8FFFF}\u{9FFFE}\u{9FFFF}\u{AFFFE}\u{AFFFF}\u{BFFFE}\u{BFFFF}\u{CFFFE}\u{CFFFF}\u{DFFFE}\u{DFFFF}\u{EFFFE}\u{EFFFF}\u{FFFFE}\u{FFFFF}\u{10FFFE}\u{10FFFF}]/u; -// This is duplicated from the compiler, but we need it at runtime too. -export const DOMBooleanAttributes = [ - 'allowfullscreen', - 'async', - 'autofocus', - 'autoplay', - 'checked', - 'controls', - 'default', - 'disabled', - 'formnovalidate', - 'hidden', - 'indeterminate', - 'ismap', - 'loop', - 'multiple', - 'muted', - 'nomodule', - 'novalidate', - 'open', - 'playsinline', - 'readonly', - 'required', - 'reversed', - 'seamless', - 'selected' -]; - export const VoidElements = new Set([ 'area', 'base', @@ -225,11 +198,13 @@ export function css_props(payload, is_html, props, component) { /** * @param {Record[]} attrs + * @param {boolean} lowercase_attributes + * @param {boolean} is_svg * @param {string} class_hash * @param {{ styles: Record | null; classes: string }} [additional] * @returns {string} */ -export function spread_attributes(attrs, class_hash, additional) { +export function spread_attributes(attrs, lowercase_attributes, is_svg, class_hash, additional) { /** @type {Record} */ const merged_attrs = {}; let key; @@ -276,7 +251,10 @@ export function spread_attributes(attrs, class_hash, additional) { for (name in merged_attrs) { if (INVALID_ATTR_NAME_CHAR_REGEX.test(name)) continue; - const is_boolean = DOMBooleanAttributes.includes(name); + if (lowercase_attributes) { + name = name.toLowerCase(); + } + const is_boolean = !is_svg && DOMBooleanAttributes.includes(name); attr_str += attr(name, merged_attrs[name], is_boolean); } diff --git a/packages/svelte/tests/runtime-runes/samples/attribute-spread-casing/_config.js b/packages/svelte/tests/runtime-runes/samples/attribute-spread-casing/_config.js new file mode 100644 index 0000000000..eac7b72914 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/attribute-spread-casing/_config.js @@ -0,0 +1,29 @@ +import { test } from '../../test'; + +export default test({ + // There's a slight difference in the output between modes, because the server doesn't know + // whether or not the custom element has the readonly boolean, so it plays it save and + // assumes it does. + html: ` + + + + + + + + + + `, + ssrHtml: ` + + + + + + + + + +` +}); diff --git a/packages/svelte/tests/runtime-runes/samples/attribute-spread-casing/main.svelte b/packages/svelte/tests/runtime-runes/samples/attribute-spread-casing/main.svelte new file mode 100644 index 0000000000..88cb1cfee5 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/attribute-spread-casing/main.svelte @@ -0,0 +1,18 @@ + + + + + + + + + + + + + + diff --git a/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js new file mode 100644 index 0000000000..6f53521673 --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js @@ -0,0 +1,48 @@ +// main.svelte (Svelte VERSION) +// Note: compiler output will change before 5.0 is released! +import "svelte/internal/disclose-version"; +import * as $ from "svelte/internal"; + +var frag = $.template(`
`, true); + +export default function Main($$anchor, $$props) { + $.push($$props, true); + + // needs to be a snapshot test because jsdom does auto-correct the attribute casing + let x = $.source('test'); + let y = $.source(() => 'test'); + /* Init */ + var fragment = $.open_frag($$anchor, false, frag); + var node = $.child_frag(fragment); + var svg = $.sibling($.sibling(node)); + var custom_element = $.sibling($.sibling(svg)); + var div = $.sibling($.sibling(custom_element)); + var svg_1 = $.sibling($.sibling(div)); + var custom_element_1 = $.sibling($.sibling(svg_1)); + + /* Update */ + $.attr_effect(div, "foobar", () => $.get(y)()); + $.attr_effect(svg_1, "viewBox", () => $.get(y)()); + $.set_custom_element_data_effect(custom_element_1, "fooBar", () => $.get(y)()); + + var node_foobar; + var svg_viewBox; + var custom_element_fooBar; + + $.render_effect(() => { + if (node_foobar !== (node_foobar = $.get(x))) { + $.attr(node, "foobar", node_foobar); + } + + if (svg_viewBox !== (svg_viewBox = $.get(x))) { + $.attr(svg, "viewBox", svg_viewBox); + } + + if (custom_element_fooBar !== (custom_element_fooBar = $.get(x))) { + $.set_custom_element_data(custom_element, "fooBar", custom_element_fooBar); + } + }); + + $.close_frag($$anchor, fragment); + $.pop(); +} \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/server/main.svelte.js b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/server/main.svelte.js new file mode 100644 index 0000000000..0352ce77aa --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/server/main.svelte.js @@ -0,0 +1,14 @@ +// main.svelte (Svelte VERSION) +// Note: compiler output will change before 5.0 is released! +import * as $ from "svelte/internal/server"; + +export default function Main($$payload, $$props) { + $.push(true); + + // needs to be a snapshot test because jsdom does auto-correct the attribute casing + let x = 'test'; + let y = () => 'test'; + + $$payload.out += ` `; + $.pop(); +} \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/main.svelte b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/main.svelte new file mode 100644 index 0000000000..a38659a486 --- /dev/null +++ b/packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/main.svelte @@ -0,0 +1,14 @@ + + +
+ + + + +
+ +