Custom attributes doesn't work

[cjh9]

Custom attributes such as inline string events and data attributes doesn't work.

vnode = h('div', { className:123, 'data-columns':123, onclick:"function(){alert('hello')}"}, ['text'])

https://developer.mozilla.org/en-US/docs/Learn/HTML/Howto/Use_data_attributes

Also using the word class is fine if it's inside an object, for instance https://github.com/picodom/picodom doesn't use className.

[cjh9]

I saw now that petit-dom only uses DOM properties, which is also recommended. I just learned the difference between them and attributes, sorry for my ignorance :)

[yelouafi]

Sorry for the late reply.

yes petit-dom uses props for HTML elements and attributes for SVG elements. But I think there has to be a way to set things that can't be set directly via simple assignement (eg dataset, aria attributes ...). So I think the issue is still valid

[cjh9]

@yelouafi Hmm, I'm not a fan of how snabbdom does it https://github.com/snabbdom/snabbdom#the-attributes-module, a lot of boilerplate code with attributes and also events, nested Json...

What if we did first argument is a string with tag or css selector, 'div.class1#id1'
Second argument is an object with props,
If the third argument is an object and i available it is the attributes,
Third or forth is an array with the children

h('div.class1', {propertyOne:'234'}, {'data-columns':123, onclick:"function(){alert('hello')}"}, ['textchild'])
or
h('div.class2', {propertyOne:'234'},['This element has no attributes'])

[mindplay-dk]

petit-dom uses props for HTML elements and attributes for SVG elements

In my opinion, this is pretty counter-intuitive - and inconsistent.

It's easy enough to support both - just give priority to properties over attributes, like I did here - something along the lines of:

if (name in element) {
  element[name] = value
} else {
  element.setAttribute(name, value)
}

While I'm not generally in favor of conditionally mapping one set (JSX attibutes) against two sets (DOM properties, DOM attributes) the problem is practically non-existent in the case of HTML elements, where practically every property is just a (typed) accessor for an attribute - and since JSX works with typed values (and not just strings) there's likely no case in which setting an attribute with the same name as a property is preferable to setting the property.

[yelouafi]

In my opinion, this is pretty counter-intuitive - and inconsistent

I did it because setting props directly on SVG elements doesnt work. For example setting circleElem.cx = 20 won't work. cx is read only (it's not a simple number but a more complex prop of type SVGAnimatedLength). But using circleElem.setAttribute("cx", 20) will work because the browser knows how to reflect that simple attribute value into the underlying interface.

OTOH for html elements setting attributes work only for non interactive attributes . For interactive ones (like 'value', 'selected' ...etc) setting the attribute will work only on the initial setting, but after that we need to set the DOM property.

So either way it's difficult to keep a consistent approach and we need to account for exceptions.

It's easy enough to support both - just give priority to properties over attributes

The code will not work for a circleElem.cx property. 'cx' in circleElem returns true but circleElem.cx = 20 doesn't work. It may be restricted to only HTML elements but I'm not sure this is also fail-proof (in the case the property to be set is a complex object)

But I agree the approach I use isn't the most appropriate & I think it's better to set attributes by default and add special support for interactive elements

[mindplay-dk]

I did it because setting props directly on SVG elements doesnt work

They got around that in Hyperapp, and I just submitted a PR for Picodom for the same fix.

Basically, you just need to make sure that SVG elements are always updated with setAttribute.

[mindplay-dk]

(at least that's the cheap/easy fix - perhaps more correct would be to test for instanceof SVGAnimatedLength and conditionally set baseVal.value, which might perform better as well?)

[yelouafi]

the new release 0.1.0 release uses now attributes by default and props for interactive props. Which means that attributes like data-, aria- ... 'should' work out of the box now.

[mrjjwright]

Great timing, I was just running into this issue this morning. After upgrading, dataset attributes work for me. Thanks!!