msb-public
/
svelte
mirror of https://github.com/sveltejs/svelte
1
0
Code Issues Projects Releases Wiki Activity

fix: better attribute casing logic (#9626)

Browse Source 
- don't lowercase attributes on svg and custom element elements, fixes #9605
- better lowercasing + property alias checking for spreads, fixes #9305
pull/9607/head
Simon H 1 year ago committed by GitHub
parent ef68b66dee
commit 72d3a2a8ce
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 296 additions and 129 deletions
Show Stats Download Patch File Download Diff File

5
.changeset/seven-deers-jam.md
Unescape View File

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: better attribute casing logic

66
packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js
Unescape View File

@ -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<import('#compiler').Attribute | import('#compiler').SpreadAttribute>} 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;
}
}

51
packages/svelte/src/compiler/phases/3-transform/server/transform-server.js
Unescape View File

@ -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<import('#compiler').Attribute | import('#compiler').SpreadAttribute>} 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,

44
packages/svelte/src/compiler/phases/constants.js
Unescape View File

@ -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<string, string>} */
export const AttributeAliases = {
class: 'className',
formnovalidate: 'formNoValidate',
ismap: 'isMap',
nomodule: 'noModule',
playsinline: 'playsInline',
readonly: 'readOnly'
};
export const VoidElements = [
'area',
'base',

45
packages/svelte/src/constants.js
Unescape View File

@ -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<string, string>}
* 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'
];

53
packages/svelte/src/internal/client/render.js
Unescape View File

@ -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<string, unknown> | null} prev
* @param {Record<string, unknown>[]} attrs
* @param {boolean} lowercase_attributes
* @param {string} css_hash
* @returns {Record<string, unknown>}
*/
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
);
}

38
packages/svelte/src/internal/server/index.js
Unescape View File

@ -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<string, unknown>[]} attrs
* @param {boolean} lowercase_attributes
* @param {boolean} is_svg
* @param {string} class_hash
* @param {{ styles: Record<string, string> | 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<string, unknown>} */
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);
}

29
packages/svelte/tests/runtime-runes/samples/attribute-spread-casing/_config.js
Unescape View File

@ -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: `
<button>click me</button>
<input>
<input>
<custom-element readonly="false"></custom-element>
<custom-element readonly="false"></custom-element>
<svg readonly="false"></svg>
<svg readonly="false"></svg>
`,
ssrHtml: `
<button>click me</button>
<input>
<input>
<custom-element></custom-element>
<custom-element readonly="false"></custom-element>
<svg readonly="false"></svg>
<svg readonly="false"></svg>
`
});

18
packages/svelte/tests/runtime-runes/samples/attribute-spread-casing/main.svelte
Unescape View File

@ -0,0 +1,18 @@
<script>
const disabled = { dIsAbLeD: false };
const readonly = { readonly: false }
const readOnly = { readOnly: false }
</script>
<!-- lowercase, then compare -->
<button {...disabled}>click me</button>
<input {...readonly}>
<input {...readOnly}>
<!-- keep casing -->
<custom-element {...readonly}></custom-element>
<custom-element {...readOnly}></custom-element>
<!-- keep casing -->
<svg {...readonly}></svg>
<svg {...readOnly}></svg>

48
packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/client/main.svelte.js
Unescape View File

@ -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(`<div></div> <svg></svg> <custom-element></custom-element> <div></div> <svg></svg> <custom-element></custom-element>`, 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();
}

14
packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/_expected/server/main.svelte.js
Unescape View File

@ -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 += `<div${$.attr("foobar", x, false)}></div> <svg${$.attr("viewBox", x, false)}></svg> <custom-element${$.attr("foobar", x, false)}></custom-element> <div${$.attr("foobar", y(), false)}></div> <svg${$.attr("viewBox", y(), false)}></svg> <custom-element${$.attr("foobar", y(), false)}></custom-element>`;
$.pop();
}

14
packages/svelte/tests/snapshot/samples/dynamic-attributes-casing/main.svelte
Unescape View File

@ -0,0 +1,14 @@
<script>
// needs to be a snapshot test because jsdom does auto-correct the attribute casing
let x = $state('test');
let y = $state(() => 'test');
</script>
<div fooBar={x}></div>
<svg viewBox={x}></svg>
<custom-element fooBar={x}></custom-element>
<!-- force them into singular render effects by using function invocations -->
<div fooBar={y()}></div>
<svg viewBox={y()} ></svg>
<custom-element fooBar={y()}></custom-element>
Write Preview
Loading…
Cancel
Save