[anmonteiro]

Support arbitrary string-valued data attributes in DOM JSX, including optional attributes. Extends the coverage merged in #934 and #935 to exercise JSX support. Fixes #230; related to melange-re/melange#2004.

01a17001-efda-7320-8b10-3844e629533f

[davesnx]

Note on digits: server-reason-react's ppx converts data attribute names differently when a digit follows a letter.

JSX name	this PR	server-reason-react
dataVersion2	data-version2	data-version-2

server-reason-react puts a dash before every character that is unchanged by Char.uppercase_ascii, and digits are unchanged, so each digit gets a dash. This PR puts a dash only before A–Z.

If one component is rendered on the server with server-reason-react and hydrated with reason-react, the attribute names will not match. We should agree on one rule. data-version2 looks like the better choice to me (it matches how dataset maps data-version2 to version2), so we can change server-reason-react to match this PR.

[anmonteiro]

@davesnx to confirm, does it make sense to change server-reason-react, or will that be a breaking change for folks who may be using it?

[davesnx]

I prefer to keep the server-reason-react intact and update the logic here, there's no user of data-* aside from ahrefs afaik and this code is new.

[anmonteiro]

@davesnx for sure, open to that!

I was just confirming what you had said above:

data-version2 looks like the better choice to me (it matches how dataset maps data-version2 to version2), so we can change server-reason-react to match this PR.

It sounds like we'll change this one to match instead!

[davesnx]

The output of the attributes are often critical in some cases where a 3rd party integration asks for a specific name, and not being able to represent those are a big blocker.

I push a PR ml-in-barcelona/server-reason-react#412 to improve the output, maybe we can align? Sorry for the back and forth but I think it might be worth it