Some remarks about set_attributes()

[adiguba]

Describe the bug

Not really a bug, but some stuff I noticed while reading the code and testing with REPL with set_attributes().

This function accept 3 optionnals booleans :

/**
 * ...
 * @param {boolean} [preserve_attribute_case]
 * @param {boolean} [is_custom_element]
 * @param {boolean} [skip_warning]
 */

But I think that the generated code is inconsistent between nodes and <svelte:element> :

tag	preserve_attribute_case	is_custom_element	skip_warning
<div>
<custom-elem>	true	true
<svg>	true
<math>	true
<svelte:element>	$$element.namespaceURI === $.NAMESPACE_SVG	$$element.nodeName.includes('-')

So :

• preserve_attribute_case is inconsistent between regular and <svelte:element> nodes.
  On regular node it's true for custom-elements, SVG and MathML nodes.
  On <svelte:element> it's only true for SVG nodes.
• skip_warning is never passed, and it's even never used inside the function.
  I was also unable to generate any code that used it on set_attribute() (without 's'), even in DEV mode.
  Either it is useless and can be deleted, or this needs to be corrected.
• Also I think that preserve_attribute_case/is_custom_element (and perhaps skip_warning if it's needed) can be replaced by a single flag inside set_attributes().
  Something like that :

	// flag is a number :
	//      * 0 : svelte-element
	//      * 1 : HTML nodes
	//      * 2 : Custom Element
	//      * 3 : SVG
	//      * 4 : MathML
	//
	let is_custom_element = flag === 2 || (flag === 0 && element.nodeName.includes('-'));
	let preserve_attribute_case = flag > 1 || is_custom_element  || (flag === 0 && element.namespaceURI !== NAMESPACE_HTML)

Reproduction

Exemple in REPL (see generated code)

Logs

-

System Info

REPL

Severity

annoyance

[Ocean-OS]

About the flags comment, I could see this using bitmasks, like how the $.state and other reactive components work. For example:

const SVELTE_ELEMENT = 1 << 0;
const HTML = 1 << 1;
const SVG = 1 << 2;
const MATHML = 1 << 3;
function set_attributes(insert_args_here, flags=0) {
    if (flags & SVELTE_ELEMENT) //svelte:element
} 

This would also simplify the parameters for SVG svelte:elements.

[adiguba]

I think I have a better idea : is_custom_element and preserve_attribute_case can be determined when initializing <svelte:element>.

Currently, this line :

<svelte:element this={tag} title={title} {...rest}></svelte:element>

is compiled to something like that :

	$.element(node, tag, false, ($$element, $$anchor) => {
		let attributes;

		$.template_effect(() => attributes = $.set_attributes($$element, attributes,
				{ title: title(), ...rest }, undefined,
				$$element.namespaceURI === $.NAMESPACE_SVG,
				$$element.nodeName.includes('-')
		));
	});

And $$element.namespaceURI === $.NAMESPACE_SVG and $$element.nodeName.includes('-') are re-evaluated on all updates...

But, by moving the checks inside $.element() when the node is created, we could generate something like this :

	$.element(node, tag, false, ($$element, $$anchor, $$preserve_attribute_case, $$is_custom_element) => {
		let attributes;

		$.template_effect(() => attributes = $.set_attributes($$element, attributes,
				{ title: title(), ...rest }, undefined,
				$$preserve_attribute_case,
				$$is_custom_element
		));
	});

Will try to test that...

[adiguba]

Closed by #15443