Skip to content

Conversation

btakita
Copy link
Contributor

@btakita btakita commented May 17, 2019

Fixes #2732

I have not found a way to reliably reflect on the property, but try/catch seems to do the trick.

@Conduitry
Copy link
Member

Conduitry commented May 27, 2019

I don't feel like this is the correct solution - we should only be attempting to use properties when they're writable in the first place, via attribute_lookup. Oh, this is not fixing quite the issue I thought it was. Never mind, I'm not sure what the right thing is here.

@Conduitry
Copy link
Member

It looks like this might also be relevant to #1708.

@Conduitry
Copy link
Member

I'm not sure what should happen here in light of #3013. Do we want to switch to using attr a lot more of the time, and not even try using a property?

@Rich-Harris
Copy link
Member

I'm not sure what should happen here in light of #3013. Do we want to switch to using attr a lot more of the time, and not even try using a property?

maybe? Presumably that would prevent issues like this, but result in incorrect behaviour in cases where the prop isn't controlled by the attribute (<input {...objectThatIncludesAKeyNamedValue}>). The difference is you could easily work around that like so:

<input {...objectThatIncludesAKeyNamedValue} value={objectThatIncludesAKeyNamedValue.value}>

...whereas you can't really work around this issue. Man, SVG is terrible.

@btakita
Copy link
Contributor Author

btakita commented Jun 25, 2019

Agreed, SVG & this props/attributes duality is lame. Here's a workaround that uses a reactive block:

	let dom__svg
	let props
	$: props = unpick($$props, 'class')
	$: {
		props
		dom__svg && each(keys(props),
			prop => dom__svg.setAttribute(prop, props[prop]))
	}

@utherpally
Copy link

utherpally commented Jul 21, 2019

How about per-component node attribute setter?

<svelte:options forceNodeAttributes={{'tagName' :['viewBox', 'width', 'height' ]}} />  <!-- Or any better prop name -->
// runtime/internal/dom.ts
function set_attributes(node, props,  forceAttributes) { // forceNodeAttributes[node.tag]
	for (const key in props) {
		if (key === 'style') {
			node.style.cssText = props[key];
		} else if  (forceAttributes && key in forceAttributes) {
			attr(node, key, props[key]);
		} else if (key in node) { // Own property
			node[key] = props[key];
		} else {
			attr(node, key, props[key]);
		}
	}
}

Or

<svelte:options setter={(node, attributes) => { // Do something }} />

@Conduitry
Copy link
Member

#1708 has an interesting idea, of checking node.hasOwnProperty(key) rather than key in node, which helped in that issue. I don't know whether that would help here.

@Conduitry
Copy link
Member

Closing: See #3531.

@Conduitry Conduitry closed this Sep 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rehydrating SVG: Cannot assign to read only property
4 participants