Width of img becomes zero when using spread props

[ambarvm]

Describe the bug

When using width attribute along with spread props on img, width is always zero.

<img {src} width="100%" alt="Alt text" {...someObject} >

The rendered HTML has width="0"

Reproduction

REPL REPL uses $$restProps but bug occurs with spreading any object.

Logs

None

System Info

System:
    OS: Windows 10 10.0.19043
    CPU: (8) x64 Intel(R) Core(TM) i5-8300H CPU @ 2.30GHz
    Memory: 1.25 GB / 7.85 GB
  Binaries:
    Node: 16.6.0 - C:\Program Files\nodejs\node.EXE
    npm: 7.21.1 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.19041.1023.0), Chromium (93.0.961.52)
  npmPackages:
    svelte: ^3.42.6 => 3.42.6

Severity

annoyance

[Conduitry]

This is presumably the result of the elm.setAttribute('width', whatever) effectively becoming elm.width = whatever; when there's an attribute spread present, and the browser is presumably able to handle the normalization of certain types of values in the attribute that it doesn't do when setting the property. (In particular, it looks like the width property is always a number indicating a number of pixels. I don't know that there's a prop equivalent to the width attribute.)

The set_attributes helper function being used is here: https://github.com/sveltejs/svelte/blob/fc4797c6f84cf299117243e7193bf77d96914e85/src/runtime/internal/dom.ts#L294 It has special handling for a couple of attribute names, but other than that, it looks for a settable property with the same name, uses that if present, and if not falls back to setting the attribute. The logic here has evolved several times over the years, but it's still an imperfect approximation of the ideal.

I don't know what the solution to this is, and I don't know where the balance is between optimal behavior and having to ship a whole bunch of attribute/property lookup information to the browser with the app whenever someone uses spread attributes.

[ambarvm]

Unlike #6751 this happens on firefox as well

[apfelbox]

As far as I understand it (and how MDN and the WHATWG write it), width="100%" is just invalid.

width The intrinsic width of the image in pixels. Must be an integer without a unit.

It seems like style="width: 100%" should be used here.

[snowwolfjay]

width need to be a number

[baseballyama]

<img> tag's height attributes should set a number. Therefore at this line, maybe a browser (e.g. Chrome) ignored that is not number value. https://github.com/sveltejs/svelte/blob/7b7c312801327e7a05983af97d863d9319043d58/src/runtime/internal/dom.ts#L305

By the way, @Conduitry mentioned this before.

I recall it being suggested somewhere I can't find that one possibility would be to fall back to setting it as an attribute if node[key] = attributes[key]; fails. This might be the best we can reasonably do. This definitely is an area that needs work.

https://github.com/sveltejs/svelte/issues/3681#issuecomment-540841473

I think this is like this.

export function set_attributes(node: Element & ElementCSSInlineStyle, attributes: { [x: string]: string }) {
	// @ts-ignore
	const descriptors = Object.getOwnPropertyDescriptors(node.__proto__);
	for (const key in attributes) {
		if (attributes[key] == null) {
			node.removeAttribute(key);
		} else if (key === 'style') {
			node.style.cssText = attributes[key];
		} else if (key === '__value') {
			(node as any).value = node[key] = attributes[key];
		} else if (descriptors[key] && descriptors[key].set) {
+			const is_different = node[key] !== attributes[key];
			node[key] = attributes[key];
+			if (is_different && node[key] !== attributes[key]) attr(node, key, attributes[key]);
		} else {
			attr(node, key, attributes[key]);
		}
	}
}

In my opinion, now we have 2 ways.

• Fallback if node[key] = attributes[key] failed. (This is above idea) I don't think existence of property is equal to nonexistence of attribute. So I think this fallback way is fair.
• Call set_attributes even it's not nessesary. If we always call set_attributes, always width property will be remove. And we can say "This width is invalid. So we removed." as specification. But I think this is simply inconvenience because plain html can use like <img width:"100%">.

So I prefer option 1. But I want your opinion before improve it.

[mhkeller]

Is there any update on this? Spread props are super useful but seems like they're best to be avoided until this gets resolved...

[Conduitry]

This has been fixed in 3.59.0 - https://svelte.dev/repl/0e6f7df1207e4b9faaaa0fb2597badd9?version=3.59.0