Unable to create svg elements with CSS selectors

[tusharmath]

Something like this never works —

h('line.form-field-underline-default', {attrs: {x1: '0', y1: '0', x2: '100', y2: '0'}})

ERROR:

Uncaught TypeError: Cannot assign to read only property 'className' of object '#<SVGLineElement>'
    at createElm (snabbdom.js:75)
    at createElm (snabbdom.js:78)
    at createElm (snabbdom.js:78)
    at createElm (snabbdom.js:78)
    at createElm (snabbdom.js:78)
    at updateChildren (snabbdom.js:172)
    at patchVnode (snabbdom.js:212)
    at updateChildren (snabbdom.js:151)
    at patchVnode (snabbdom.js:212)
    at updateChildren (snabbdom.js:151)

[acstll]

That's because that selector syntax uses className and as the error states: on that type of element SVGLineElement, className is read-only. You cannot set it.

It's not a problem with snabbdom I think.

It should work if you use the class module since it uses the classList API.

h('line', {
  attrs: {x1: '0', y1: '0', x2: '100', y2: '0'},
  class: {'form-field-underline-default': true}
})

[tusharmath]

IMHO a selector should just work everywhere don't u think? This is an implementation detail that as a consumer of the library, I shouldn't be concerned about. I'd be happy to send a PR for it.

[paldepind]

Thank you for submitting the bug.

IMHO a selector should just work everywhere don't u think? This is an implementation detail that as a consumer of the library, I shouldn't be concerned about.

I see your point. It's a bit odd that className is read only for svg elements. As @acstll mentions there really isn't anything wrong with the current implementation. At least if it wasn't for that.

I'd be happy to send a PR for it.

Go ahead 👍 The relevant code is here snabbdom.ts#L94.

[caesarsol]

@paldepind If nothing moves, I'd like to try a PR about this.

I just have a couple of questions:

• I have an intuition that selectors on SVG elements should improve performance due to smarter sameVNode() diffing, could that be right​? I'm rendering large SVGs and feel thia problem.
• is there a reason for not using classList in this node-creation context? That would solve the issue if I understood correctly. The class module uses it.
• is there a reason for not passing through the snabbdom DOMAPI abstraction for setting the node class/id?

Thank you for any clarification!

[caridy]

@caesarsol send it :)

[caesarsol]

Hello, I have the PR ready but I'd like some reassurance about substituting .className= for .setAttribute(class).

I think it's even more consistent being the phase of creation of a DOM element, but I'm not an expert on raw DOM manipulation...

[caridy]

@caesarsol using setAttribute is just fine. They both have the same limitations though, setting it to null, undefined, etc, might have undesired effects (it uses toString() under the hook). But in this case, since it is a controlled environment, you can only set it the attribute if really needed.

[caesarsol]

Ok thanks! Sending the PR in a few hours.