`data-hovered` props is being eaten

[unional]

Provide a general summary of the issue here

When setting the data attribute prop data-hovered, the prop is eaten and not showing up in the DOM element.

🤔 Expected Behavior?

<Button data-hovered/> should yield <button data-hovered/>

😯 Current Behavior

<Button data-hovered/> yields <button/>

💁 Possible Solution

Haven't look into the code, but likely there is a spread const { 'data-hovered': dataHovered, ...rest } = props and missing <button data-hovered={dataHovered}/>

another possibility is that since those data-attributes are managed by rac, a merge is missing to honor the initial values.

🔦 Context

This make it not possible to set the hover state without some hack/workaround.

This could be the root cause of #6198 #4411

Related to #6522 #6403

🖥️ Steps to Reproduce

codesandbox

In the repro, you can see that data-hover is being recognized:

This is because data-hover is not a data attribute managed by react-aria-components. So the prop is added to the DOM:

<button data-hover="true" type="button" class="data-[hover]:bg-blue-300" data-rac="">data-hover set</button>

When hovering on the one with data-hovered CSS, it is working. i.e. :hover is applying data-hovered (and data-focused) correctly:

But looking at the first button you will see the data-hovered prop is missing:

<button type="button" class="data-[hovered]:bg-red-500" data-rac="">data-hovered set</button>

while the code defined it:

      <Button
        data-hovered
        className="data-[hovered]:bg-red-500"
        onPress={() => alert("Hello world!")}
      >
        data-hovered set
      </Button>

As a side note, setting element state from dev tool doesn't work:

That's probably a limitation to the data-attribute approach, I guess.

Version

react-aria-components: 1.3.1

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

macOS

🧢 Your Company/Team

Palo Alto Networks

🕷 Tracking Issue

No response

[snowystinger]

I think the first question we need to answer is, what is the use case here? Is it just visual regression testing?

Thoughts on a possible implementation below, but not going to consider further until we answer if this is something we actually want to support.

We could add something like this inside useRenderProps ``` let keys = Object.keys(dataAttributes || {}); let deps = keys.map(k => props[k]).concat(keys.map(k => dataAttributes[k])); let mergedDataAttributes = useMemo(() => { let dataAttrs = {} as Record; for (let key of keys) { dataAttrs[key] = props[key] !== undefined ? props[key] : dataAttributes[key]; } return dataAttrs; }, [...deps]); ``` Where `dataAttributes` is an object containing what we currently explicitly place as props on the target element. The keys in dataAttributes matter, we wouldn't want this set to change, ever, or memoing will be harder. This would default to our own value for the data attribute first, then would fallback to any on the props. I'm a little cautious about this because while the data-attributes are documented, there's no way to warn a user if they are about to pass one into a component which is in conflict with one that we are going to use. We wouldn't be able to warn users about it either because they may be doing it on purpose like in your example. I'm also not convinced which one should win, what I've outlined above is a blend, since we can't enforce "not" setting it if we do this. Making it so the user always wins is a lot of power (with great power comes great responsibility) on the flip side. Also, since we don't currently type these as specific props, users could override with possibly invalid values. Finally, it does make useRenderProps a little slower. We could move it out to another hook to work around it though.

[unional]

My use case is visual regression testing, as well as specification and documentation. I want the engineer and designer going to our website and storybook to see exactly how the component should look like on various state. And also have it match the Figma defined by the design system team.

In terms of code and consistency, it is also nice and natural to see what you defined shows up on the DOM for data-*, just like aria-* attributes.

[snowystinger]

In terms of code and consistency, at it also nice and natural to see what you defined shows up on the DOM for data-, just like aria- attributes.

But this isn't true, we control many aria-* attributes, so if you send them in as props, they may or may not come out on the other side and they may be merged/altered. That's a majority of the reason for this library.

I don't think consistency applies here as we are consistent in our handling of these. We have certain properties that we are going to apply regardless of what a user tries to send it. Right now, that applies to a certain set of documented data-* attributes we consider to be API.

[unional]

It seems like I somehow didn't mentioned that specifying data-hover in @headlessui/react works as expected. It is also using react-aria under the hood.

[snowystinger]

It appears they are using React Aria, not React Aria Components https://github.com/tailwindlabs/headlessui/blob/main/packages/%40headlessui-react/src/components/button/button.tsx

React Aria isn't responsible for dealing with the data- attributes, that's not part of its API. So you could build your own components from the hooks and make the data- attributes whatever you like.

I'm going to close this issue as working as intended. I think we can continue discussions over on https://github.com/adobe/react-spectrum/issues/6403 if there's more as I believe that's really what you're trying to do.