Skip to content

Commit 72d3a2a

Browse files
authored
fix: better attribute casing logic (#9626)
- don't lowercase attributes on svg and custom element elements, fixes #9605 - better lowercasing + property alias checking for spreads, fixes #9305
1 parent ef68b66 commit 72d3a2a

File tree

12 files changed

+296
-129
lines changed

12 files changed

+296
-129
lines changed

.changeset/seven-deers-jam.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: better attribute casing logic

packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js

Lines changed: 37 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,7 @@ import {
1212
escape_html,
1313
infer_namespace
1414
} from '../../utils.js';
15-
import {
16-
AttributeAliases,
17-
DOMBooleanAttributes,
18-
DOMProperties,
19-
PassiveEvents,
20-
VoidElements
21-
} from '../../../constants.js';
15+
import { DOMProperties, PassiveEvents, VoidElements } from '../../../constants.js';
2216
import { is_custom_element_node, is_element_node } from '../../../nodes.js';
2317
import * as b from '../../../../utils/builders.js';
2418
import { error } from '../../../../errors.js';
@@ -29,6 +23,8 @@ import {
2923
serialize_set_binding
3024
} from '../utils.js';
3125
import {
26+
AttributeAliases,
27+
DOMBooleanAttributes,
3228
EACH_INDEX_REACTIVE,
3329
EACH_IS_CONTROLLED,
3430
EACH_ITEM_REACTIVE,
@@ -37,6 +33,26 @@ import {
3733
import { regex_is_valid_identifier } from '../../../patterns.js';
3834
import { javascript_visitors_runes } from './javascript-runes.js';
3935

36+
/**
37+
* @param {import('#compiler').RegularElement | import('#compiler').SvelteElement} element
38+
* @param {import('#compiler').Attribute} attribute
39+
* @param {{ state: { metadata: { namespace: import('#compiler').Namespace }}}} context
40+
*/
41+
function get_attribute_name(element, attribute, context) {
42+
let name = attribute.name;
43+
if (
44+
element.type === 'RegularElement' &&
45+
!element.metadata.svg &&
46+
context.state.metadata.namespace !== 'foreign'
47+
) {
48+
name = name.toLowerCase();
49+
if (name in AttributeAliases) {
50+
name = AttributeAliases[name];
51+
}
52+
}
53+
return name;
54+
}
55+
4056
/**
4157
* Serializes each style directive into something like `$.style(element, style_property, value)`
4258
* and adds it either to init or update, depending on whether or not the value or the attributes are dynamic.
@@ -259,18 +275,19 @@ function setup_select_synchronization(value_binding, context) {
259275
* Returns the id of the spread_attribute variable if spread is deemed reactive, `null` otherwise.
260276
* @param {Array<import('#compiler').Attribute | import('#compiler').SpreadAttribute>} attributes
261277
* @param {import('../types.js').ComponentContext} context
278+
* @param {import('#compiler').RegularElement} element
262279
* @param {import('estree').Identifier} element_id
263280
* @returns {string | null}
264281
*/
265-
function serialize_element_spread_attributes(attributes, context, element_id) {
282+
function serialize_element_spread_attributes(attributes, context, element, element_id) {
266283
let is_reactive = false;
267284

268285
/** @type {import('estree').Expression[]} */
269286
const values = [];
270287

271288
for (const attribute of attributes) {
272289
if (attribute.type === 'Attribute') {
273-
const name = get_attribute_name(attribute, context.state);
290+
const name = get_attribute_name(element, attribute, context);
274291
// TODO: handle contains_call_expression
275292
const [, value] = serialize_attribute_value(attribute.value, context);
276293
values.push(b.object([b.init(name, value)]));
@@ -281,6 +298,9 @@ function serialize_element_spread_attributes(attributes, context, element_id) {
281298
is_reactive ||= attribute.metadata.dynamic;
282299
}
283300

301+
const lowercase_attributes =
302+
element.metadata.svg || is_custom_element_node(element) ? b.false : b.true;
303+
284304
if (is_reactive) {
285305
const id = context.state.scope.generate('spread_attributes');
286306
context.state.init.push(b.let(id, undefined));
@@ -294,6 +314,7 @@ function serialize_element_spread_attributes(attributes, context, element_id) {
294314
element_id,
295315
b.id(id),
296316
b.array(values),
317+
lowercase_attributes,
297318
b.literal(context.state.analysis.stylesheet.id)
298319
)
299320
)
@@ -308,6 +329,7 @@ function serialize_element_spread_attributes(attributes, context, element_id) {
308329
element_id,
309330
b.literal(null),
310331
b.array(values),
332+
lowercase_attributes,
311333
b.literal(context.state.analysis.stylesheet.id)
312334
)
313335
)
@@ -398,14 +420,15 @@ function serialize_dynamic_element_spread_attributes(attributes, context, elemen
398420
* });
399421
* ```
400422
* Returns true if attribute is deemed reactive, false otherwise.
423+
* @param {import('#compiler').RegularElement} element
401424
* @param {import('estree').Identifier} node_id
402425
* @param {import('#compiler').Attribute} attribute
403426
* @param {import('../types.js').ComponentContext} context
404427
* @returns {boolean}
405428
*/
406-
function serialize_element_attribute_update_assignment(node_id, attribute, context) {
429+
function serialize_element_attribute_update_assignment(element, node_id, attribute, context) {
407430
const state = context.state;
408-
const name = get_attribute_name(attribute, state);
431+
const name = get_attribute_name(element, attribute, context);
409432
let [contains_call_expression, value] = serialize_attribute_value(attribute.value, context);
410433

411434
// 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) {
672695
);
673696
}
674697

675-
/**
676-
* @param {import('#compiler').Attribute} attribute
677-
* @param {import('../types.js').ComponentClientTransformState} state
678-
*/
679-
function get_attribute_name(attribute, state) {
680-
let name = attribute.name;
681-
if (state.metadata.namespace !== 'foreign') {
682-
name = name.toLowerCase();
683-
if (name !== 'class' && name in AttributeAliases) {
684-
name = AttributeAliases[name];
685-
}
686-
}
687-
return name;
688-
}
689-
690698
/**
691699
* @param {import('#compiler').Component | import('#compiler').SvelteComponent | import('#compiler').SvelteSelf} node
692700
* @param {string} component_name
@@ -1899,7 +1907,7 @@ export const template_visitors = {
18991907
// Then do attributes
19001908
let is_attributes_reactive = false;
19011909
if (node.metadata.has_spread) {
1902-
const spread_id = serialize_element_spread_attributes(attributes, context, node_id);
1910+
const spread_id = serialize_element_spread_attributes(attributes, context, node, node_id);
19031911
if (child_metadata.namespace !== 'foreign') {
19041912
add_select_to_spread_update(spread_id, node, context, node_id);
19051913
}
@@ -1920,7 +1928,7 @@ export const template_visitors = {
19201928
attribute.name !== 'autofocus' &&
19211929
(attribute.value === true || is_text_attribute(attribute))
19221930
) {
1923-
const name = get_attribute_name(attribute, context.state);
1931+
const name = get_attribute_name(node, attribute, context);
19241932
const literal_value = /** @type {import('estree').Literal} */ (
19251933
serialize_attribute_value(attribute.value, context)[1]
19261934
).value;
@@ -1941,7 +1949,7 @@ export const template_visitors = {
19411949
const is =
19421950
is_custom_element && child_metadata.namespace !== 'foreign'
19431951
? serialize_custom_element_attribute_update_assignment(node_id, attribute, context)
1944-
: serialize_element_attribute_update_assignment(node_id, attribute, context);
1952+
: serialize_element_attribute_update_assignment(node, node_id, attribute, context);
19451953
if (is) is_attributes_reactive = true;
19461954
}
19471955
}

packages/svelte/src/compiler/phases/3-transform/server/transform-server.js

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import * as b from '../../../utils/builders.js';
55
import is_reference from 'is-reference';
66
import {
77
ContentEditableBindings,
8-
DOMBooleanAttributes,
98
VoidElements,
109
WhitespaceInsensitiveAttributes
1110
} from '../../constants.js';
@@ -15,11 +14,12 @@ import {
1514
escape_html,
1615
infer_namespace
1716
} from '../utils.js';
18-
import { create_attribute, is_element_node } from '../../nodes.js';
17+
import { create_attribute, is_custom_element_node, is_element_node } from '../../nodes.js';
1918
import { error } from '../../../errors.js';
2019
import { binding_properties } from '../../bindings.js';
2120
import { regex_starts_with_newline, regex_whitespaces_strict } from '../../patterns.js';
2221
import { remove_types } from '../typescript.js';
22+
import { DOMBooleanAttributes } from '../../../../constants.js';
2323

2424
/**
2525
* @param {string} value
@@ -471,6 +471,25 @@ function serialize_set_binding(node, context, fallback) {
471471
return fallback();
472472
}
473473

474+
/**
475+
* @param {import('#compiler').RegularElement | import('#compiler').SvelteElement} element
476+
* @param {import('#compiler').Attribute} attribute
477+
* @param {{ state: { metadata: { namespace: import('#compiler').Namespace }}}} context
478+
*/
479+
function get_attribute_name(element, attribute, context) {
480+
let name = attribute.name;
481+
if (
482+
element.type === 'RegularElement' &&
483+
!element.metadata.svg &&
484+
context.state.metadata.namespace !== 'foreign'
485+
) {
486+
name = name.toLowerCase();
487+
// don't lookup boolean aliases here, the server runtime function does only
488+
// check for the lowercase variants of boolean attributes
489+
}
490+
return name;
491+
}
492+
474493
/** @type {import('./types').Visitors} */
475494
const global_visitors = {
476495
Identifier(node, { path, state }) {
@@ -690,12 +709,14 @@ function serialize_attribute_value(
690709

691710
/**
692711
*
712+
* @param {import('#compiler').RegularElement | import('#compiler').SvelteElement} element
693713
* @param {Array<import('#compiler').Attribute | import('#compiler').SpreadAttribute>} attributes
694714
* @param {import('#compiler').StyleDirective[]} style_directives
695715
* @param {import('#compiler').ClassDirective[]} class_directives
696716
* @param {import('./types').ComponentContext} context
697717
*/
698718
function serialize_element_spread_attributes(
719+
element,
699720
attributes,
700721
style_directives,
701722
class_directives,
@@ -706,7 +727,7 @@ function serialize_element_spread_attributes(
706727

707728
for (const attribute of attributes) {
708729
if (attribute.type === 'Attribute') {
709-
const name = attribute.name.toLowerCase();
730+
const name = get_attribute_name(element, attribute, context);
710731
const value = serialize_attribute_value(
711732
attribute.value,
712733
context,
@@ -718,8 +739,18 @@ function serialize_element_spread_attributes(
718739
}
719740
}
720741

742+
const lowercase_attributes =
743+
element.type !== 'RegularElement' || element.metadata.svg || is_custom_element_node(element)
744+
? b.false
745+
: b.true;
746+
const is_svg = element.type === 'RegularElement' && element.metadata.svg ? b.true : b.false;
721747
/** @type {import('estree').Expression[]} */
722-
const args = [b.array(values), b.literal(context.state.analysis.stylesheet.id)];
748+
const args = [
749+
b.array(values),
750+
lowercase_attributes,
751+
is_svg,
752+
b.literal(context.state.analysis.stylesheet.id)
753+
];
723754

724755
if (style_directives.length > 0 || class_directives.length > 0) {
725756
const styles = style_directives.map((directive) =>
@@ -1760,11 +1791,17 @@ function serialize_element_attributes(node, context) {
17601791
context.state.init.push(...lets);
17611792

17621793
if (has_spread) {
1763-
serialize_element_spread_attributes(attributes, style_directives, class_directives, context);
1794+
serialize_element_spread_attributes(
1795+
node,
1796+
attributes,
1797+
style_directives,
1798+
class_directives,
1799+
context
1800+
);
17641801
} else {
17651802
for (const attribute of /** @type {import('#compiler').Attribute[]} */ (attributes)) {
17661803
if (attribute.value === true || is_text_attribute(attribute)) {
1767-
const name = attribute.name.toLowerCase();
1804+
const name = get_attribute_name(node, attribute, context);
17681805
const literal_value = /** @type {import('estree').Literal} */ (
17691806
serialize_attribute_value(
17701807
attribute.value,
@@ -1786,7 +1823,7 @@ function serialize_element_attributes(node, context) {
17861823
continue;
17871824
}
17881825

1789-
const name = attribute.name.toLowerCase();
1826+
const name = get_attribute_name(node, attribute, context);
17901827
const is_boolean = DOMBooleanAttributes.includes(name);
17911828
const value = serialize_attribute_value(
17921829
attribute.value,

packages/svelte/src/compiler/phases/constants.js

Lines changed: 2 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,52 +1,12 @@
1-
export const DOMBooleanAttributes = [
2-
'allowfullscreen',
3-
'async',
4-
'autofocus',
5-
'autoplay',
6-
'checked',
7-
'controls',
8-
'default',
9-
'disabled',
10-
'formnovalidate',
11-
'hidden',
12-
'indeterminate',
13-
'ismap',
14-
'loop',
15-
'multiple',
16-
'muted',
17-
'nomodule',
18-
'novalidate',
19-
'open',
20-
'playsinline',
21-
'readonly',
22-
'required',
23-
'reversed',
24-
'seamless',
25-
'selected'
26-
];
1+
import { AttributeAliases, DOMBooleanAttributes } from '../../constants.js';
272

283
export const DOMProperties = [
29-
'className',
4+
...Object.values(AttributeAliases),
305
'value',
31-
'readOnly',
32-
'formNoValidate',
33-
'isMap',
34-
'noModule',
35-
'playsInline',
366
'inert',
377
...DOMBooleanAttributes
388
];
399

40-
/** @type {Record<string, string>} */
41-
export const AttributeAliases = {
42-
class: 'className',
43-
formnovalidate: 'formNoValidate',
44-
ismap: 'isMap',
45-
nomodule: 'noModule',
46-
playsinline: 'playsInline',
47-
readonly: 'readOnly'
48-
};
49-
5010
export const VoidElements = [
5111
'area',
5212
'base',

packages/svelte/src/constants.js

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,48 @@ export const DelegatedEvents = [
3232

3333
/** List of Element events that will be delegated and are passive */
3434
export const PassiveDelegatedEvents = ['touchstart', 'touchmove', 'touchend'];
35+
36+
/**
37+
* @type {Record<string, string>}
38+
* List of attribute names that should be aliased to their property names
39+
* because they behave differently between setting them as an attribute and
40+
* setting them as a property.
41+
*/
42+
export const AttributeAliases = {
43+
// no `class: 'className'` because we handle that separately
44+
formnovalidate: 'formNoValidate',
45+
ismap: 'isMap',
46+
nomodule: 'noModule',
47+
playsinline: 'playsInline',
48+
readonly: 'readOnly'
49+
};
50+
51+
/**
52+
* Attributes that are boolean, i.e. they are present or not present.
53+
*/
54+
export const DOMBooleanAttributes = [
55+
'allowfullscreen',
56+
'async',
57+
'autofocus',
58+
'autoplay',
59+
'checked',
60+
'controls',
61+
'default',
62+
'disabled',
63+
'formnovalidate',
64+
'hidden',
65+
'indeterminate',
66+
'ismap',
67+
'loop',
68+
'multiple',
69+
'muted',
70+
'nomodule',
71+
'novalidate',
72+
'open',
73+
'playsinline',
74+
'readonly',
75+
'required',
76+
'reversed',
77+
'seamless',
78+
'selected'
79+
];

0 commit comments

Comments
 (0)