[JSX] Desugar aria'foo to aria-foo for DOM elements

[chenglou]

Actual transformer's in the Reason repo.

[rafayepes]

Also data'foo to data-foo?

[yawaramin]

Yup, it should be a general transformation of foo'bar -> foo-bar.

[chenglou]

I'm thinking maybe we can just use ariaFoo -> aria-foo and dataFoo -> data-foo. Again, this is on DOM elements only, so no confusion of e.g. accepting ariaFoo in a custom component prop and do things with it.

[yawaramin]

Could DOM elements potentially not have camelCased attribute names?

[chenglou]

We’d only be doing it for aria and data

[yawaramin]

Oh, I was imagining it would be simpler to check for the foo'bar form and convert it directly into "foo-bar", am I missing something?

[chenglou]

Either is simple enough. But are you saying that we might as well go with ' since it generalizes to e.g. svg'set (nonexistent prop) instead of just data'foo or aria'foo? Though those other kebab-cased props don't exist, unless someone's trying to pass a custom dom prop

[yawaramin]

Exactly, we can fully support any potential kebab-cased prop names thanks to ', which is great since kebab-cased attribute names are valid HTML. This would bring JSX more in line with valid HTML.

[chenglou]

My main nitpicky concern is that it's ugly and we don't want to encourage over-usage of ' in prop names in Reason. But since this seems to be the only way to enable custom props, then let's do it.

[yawaramin]

I think real-world use will be minimal. Kebab-cased attrs are pretty rare (unless you're using Angular 1 or something). In practice this will be just a nice escape hatch, not a heavily-used feature.

[chenglou]

It's mandatory for a11y because of aria

[yawaramin]

Ya, but that's usually a minority of all JSX attributes. I think we won't see too much kebab'attrs in practice even after accounting for ARIA.

[chenglou]

Related: we might find a good aria-* solution anyway with rescript-lang/rescript#2614

[alex35mil]

I dunno, this doesn't seem ugly to me:

But when camelCase is used for everything:

dataTestid
onClick

it seems inconsistent a bit.

[bloodyowl]

how about not having to add a special syntax?

<div
  data={
    "name": "some string"
  }
  aria={
    "role": "button",
    "label": "Open in modal"
  }
/>

[chenglou]

Fixed in 0.4.2 thanks to bs.deriving abstract: https://github.com/reasonml/reason-react/blob/master/HISTORY.md#042

I'll open another issue for data-*, though I tend to just let folks use cloneElement

[alex35mil]

Please, desugar data-* too, otherwise it’s required to clone every control from standard components (like Button and other interactive elements) to make them findable in integration tests.