Allow data-* in DOM elements

[chenglou]

aria-foo is supported through ariaFoo, because we exhaustively list all the aria options. data-* is dynamic. I think it's fine to use cloneElement to add them as an escape hatch.

[alex35mil]

It would be very unfortunate b/c I will need to clone all interactive elements in components library to make them findable in integration tests.

[alex35mil]

One more case I faced today: I'm almost finished bindings to react-beautiful-dnd and I ended up cloning every droppable & draggable element on application side b/c this lib requires specific data- attribute to be set. I don’t think it’s going to be a perf win.

[bloodyowl]

maybe having a little abstraction could be enough?

let withDataAttributes = (~data, element) =>
  ReasonReact.cloneElement(
    element,
    ~props=Obj.magic(Js.Dict.fromList(data)),
    [||],
  );

let div =
  withDataAttributes(
    ~data=[("data-foo", "bar"), ("data-bar", "baz")],
    <div />,
  );

ReactDOMRe.renderToElementWithId(div, "preview");

[rafayepes]

We make heavy use of data-* due automation purposes. Telling our more designer oriented developers that now they need to use cloneElement whenever they have to add some data-* attribute seems that is going to be a hard sell. So some form of sugar specific for data-* attributed will be very welcome.

Will something https://github.com/reasonml/reason-react/issues/196#issuecomment-375663193 be possible?

[kennetpostigo]

I have the same issue @rafayepes describes

[bloodyowl]

looked a bit at what we could do right now and bumped into a few issues that'd make it non-trivial.

I tried to wrap the createElement FFI in a function that checks if a dataSet field exists in props and overloads the object, but the [@bs.splice] constrain complains about that because it requires an syntactic array as last argument. would there be a way to "transfer" the splice properties to a non-FFI ? (cc @bobzhang)

Another way would be to allow passing a transformer function to a field in [@bs.deriving abstract], not sure how difficult that would be though.

And a possible third alternative I see would be to allow an "unsafe spread" directly in JSX:

<div
   ...{"data-foo": "bar"}
/>

[cullophid]

Could you not add an unsafe option for edgecases? <div prop=("data-stuff", "some_stuff") /> There are lots of scenarios where people are using non standard html properties for all sorts of things.

[bloodyowl]

this would require some changes on bs.deriving abstract or FFI propagation AFAIK

[apolishch]

Is there at present a recommended solution for using data attributes?

[rudolfs]

Ran into the problem with cypress needing custom data-* attributes for e2e testing. It would be really great if we didn't have to rely on cloneElement for this.

[rusty-key]

Is there a reason not to add something like data: Js.Obj.t?

<button data={"attrName": "attrValue"}> </button>

[alex35mil]

Guessing not zero cost.

Workaround from @yawaramin works pretty well until there’s official solution:

module WithTestId = {
  [@react.component]
  let make = (~id: string, ~children) =>
    ReasonReact.cloneElement(children, ~props={"data-test-id": id}, [||]);
};

<WithTestId id="foo"> <button /> </WithTestId>

[peterpme]

Hey @alexfedoseev

Would you mind creating a PR and adding that to the docs? This is an awesome workaround for now.

Thank you!

[peterpme]

Hey everyone, we've added an example here: https://reasonml.github.io/reason-react/docs/en/adding-data-props

[heygema]

I was thinking of making its own abstraction to Spread component? @peterpme for instance like special ownProps attribute which takes the object and spread it on creation? but idk

[peterpme]

Hey @heygema open up a PR and we can take a look :smile: