Remove null and undefined attributes

[cvlab]

set_attributes() should delete attributes from node when new value is null or undefined.

I meet the issue for this case

<script>
	let size
</script>
<input type="text" { size } />
vs
<input type="text" { ...{ size } } />

Thank you! Svelte is amazing!

[Rich-Harris]

This is an interesting case, I wasn't aware of size causing errors like that. I'm not totally convinced this is a valid fix though — treating something as an attribute instead of a property purely based on whether the value is null or undefined is likely to have unwanted effects.

For example, while this does work...

input = document.createElement('input');
input.size = 10;

input.setAttribute('size', 5);
input.size; // 5

// this errors — `input.size = null`
input.setAttribute('size', null);
input.size; // 20

...this doesn't:

button = document.createElement('button');
button.disabled = true;

// this works — `button.disabled = null;`
button.setAttribute('disabled', null);
button.disabled; // true (should be false)

I'm tempted to suggest that this should be fixed in the component instead, with something like

<input type="text" size={size || 20}>

[cvlab]

Thank you for replay. You are right, I'm not sure when also internally set_attributes is used. Maybe is not good for all the cases.

I mean just in cases when set_attributes is generated for HTML node spread attributes ({ ...attributes }).

<htmlTag { ...{ attr1, attr2 } }>

The problem is that for

<script>
    let disabled
</script>
<button { disabled }>Button</button>

is generated attr

svelte/src/runtime/internal/dom.ts

Line 94 in 0be4e28

export function attr(node: Element, attribute: string, value?: string) {

and for

<script>
    let disabled
</script>
<button { ...{ disabled } }>Button</button>

is generated set_attributes

svelte/src/runtime/internal/dom.ts

Line 99 in 0be4e28

export function set_attributes(node: Element & ElementCSSInlineStyle, attributes: { [x: string]: string }) {

that work a little bit differently. And this differently is big for size case.

For my example, there is no problem with

<input type="text" size={size}>

this will call attr and node.removeAttribute(attribute);
there is problem with { ...{} }

<input type="text" { ...{size} }>

because is not generated attr, but set_attributes.

Sorry for my English. Thank you! Have a nice day!

[cvlab]

I thought about idea from your comment, maybe for { ...{} } must not be used node[key] = attributes[key]; at all

Now:

export function set_attributes(node: Element & ElementCSSInlineStyle, attributes: { [x: string]: string }) {
	for (const key in attributes) {
		if (key === 'style') {
			node.style.cssText = attributes[key];
		} else if (key in node) {
			node[key] = attributes[key];
		} else {
			attr(node, key, attributes[key]);
		}
	}
}

but

export function set_attributes(node: Element & ElementCSSInlineStyle, attributes: { [x: string]: string }) {
	for (const key in attributes) {
		if (key === 'style') {
			node.style.cssText = attributes[key];
		} else {
			attr(node, key, attributes[key]);
		}
	}
}

To be more consistent code.

[Rich-Harris]

That idea could work (only ever setting attributes), but there might be some situations where it's necessary to use properties. I forget the specifics; needs research. #3013 is relevant