Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Style property used together with a spread property assigns to a read-only property #1664

Closed
kaisermann opened this issue Aug 17, 2018 · 5 comments
Labels

Comments

@kaisermann
Copy link
Member

REPL

<div {style} {...attrs}></div>

Generates a update function as:

		p: function update(changed, ctx) {
			setAttributes(div, getSpreadUpdate(div_levels, [
				(changed.style) && { style: ctx.style },
				(changed.attrs) && ctx.attrs,
				{ class: "svelte-ajb4ym" }
			]));
		},

The setAttribute finds the style property in the node and tries to overwrite it:

image

If you remove the {...attrs}, it generates as:

		p: function update(changed, ctx) {
			if (changed.style) {
				div.style.cssText = ctx.style;
			}
		},
@Rich-Harris
Copy link
Member

Just revisiting this now... I might be losing my marbles but what is wrong here? It seems like it's behaving correctly!

@kaisermann
Copy link
Member Author

kaisermann commented Aug 24, 2018

@Rich-Harris My bad, I wasn't clear enough here! 😬

The style property should not be directly assigned to since it's a read-only property:

image

So, for a single <div {style}>, we get a nice div.style.cssText = ..., but with a spread operator <div {style} {...foo}>, we get a behavior equivalent to div['style'] = ...

In addition, it behaves correctly on modern browsers because they can handle assigning a string directly to the style, but some older browsers can't.

@Rich-Harris
Copy link
Member

Ah, ok, it's a cross-browser thing. I brought it up because I was unable to create a failing test using getComputedStyle! I think it would be easily fixed by changing this:

export function setAttributes(node, attributes) {
	for (var key in attributes) {
-		if (key in node) {
+		if (key === 'style') {
+			node.style.cssText = attributes[key];
+		} else if (key in node) {
			node[key] = attributes[key];
		} else {
			if (attributes[key] === undefined) removeAttribute(node, key);
			else setAttribute(node, key, attributes[key]);
		}
	}
}

@fdArcher
Copy link

for svg el just wrap in a other html tag will it works
<span style="color:#aaa"><svg class="icon" aria-hidden="true"><use xlink:href="#ico-shezhi"></use></svg></span>
if you set slots.default as this and remove
chrome will say: Cannot assign to read only property 'className' of object '# .etc

@Rich-Harris
Copy link
Member

This looks like it's fixed in v3 — closing https://svelte.dev/repl/c1ef7df3e65546aba07137ff2bd7d91d?version=3.6.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants