Prefer incoming data-* attributes, over the ones set by Headless UI (…

…#3035)

* prefer `data-*` props that already exist

If the component already had `data-foo`, and we set `data-foo` then the
`data-foo` that was already there should stay.

* update changelog

packages/@headlessui-react/CHANGELOG.md

@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Attempt form submission when pressing `Enter` on `Checkbox` component ([#2962](https://github.com/tailwindlabs/headlessui/pull/2962))
 - Allow setting custom `tabIndex` on the `<Switch />` component ([#2966](https://github.com/tailwindlabs/headlessui/pull/2966))
 - Forward `disabled` state to hidden inputs in form-like components ([#3004](https://github.com/tailwindlabs/headlessui/pull/3004))
+- Prefer incoming `data-*` attributes, over the ones set by Headless UI ([#3035](https://github.com/tailwindlabs/headlessui/pull/3035))
 
 ### Changed
 

packages/@headlessui-react/src/utils/__snapshots__/render.test.tsx.snap

@@ -109,6 +109,22 @@ exports[`Default functionality should forward all the props to the first child w
 </div>"
 `;
 
+exports[`Default functionality should forward boolean values from \`slot\` as data attributes 1`] = `
+"<div
+  data-testid=\\"wrapper\\"
+>
+  <div
+    data-a=\\"\\"
+    data-c=\\"\\"
+    data-headlessui-state=\\"a c\\"
+  >
+    <span>
+      Contents
+    </span>
+  </div>
+</div>"
+`;
+
 exports[`Default functionality should not error when we are rendering a Fragment with multiple children when we don't passthrough additional props 1`] = `
 "<div
   data-testid=\\"wrapper\\"
@@ -122,6 +138,19 @@ exports[`Default functionality should not error when we are rendering a Fragment
 </div>"
 `;
 
+exports[`Default functionality should prefer user provided data attributes over the ones we set automatically 1`] = `
+"<div
+  data-testid=\\"wrapper\\"
+>
+  <span
+    data-accept=\\"always\\"
+    data-headlessui-state=\\"accept\\"
+  >
+    Contents
+  </span>
+</div>"
+`;
+
 exports[`Features.RenderStrategy Hidden render strategy should be possible to render an \`unmount={false}\` dummy component (show = false) 1`] = `
 "<div
   data-testid=\\"wrapper\\"

packages/@headlessui-react/src/utils/render.test.tsx

@@ -153,6 +153,50 @@ describe('Default functionality', () => {
     expect(contents()).toMatchSnapshot()
   })
 
+  it('should forward boolean values from `slot` as data attributes', () => {
+    function Dummy<TTag extends ElementType = 'div'>(
+      props: Props<TTag> & Partial<{ a: any; b: any; c: any }>
+    ) {
+      return (
+        <div data-testid="wrapper">
+          {render({
+            ourProps: {},
+            theirProps: props,
+            slot: { a: true, b: false, c: true },
+            defaultTag: 'div',
+            name: 'Dummy',
+          })}
+        </div>
+      )
+    }
+
+    testRender(<Dummy>{() => <span>Contents</span>}</Dummy>)
+
+    expect(contents()).toMatchSnapshot()
+  })
+
+  it('should prefer user provided data attributes over the ones we set automatically', () => {
+    function Dummy<TTag extends ElementType = 'div'>(
+      props: Props<TTag> & Partial<{ a: any; b: any; c: any }>
+    ) {
+      return (
+        <div data-testid="wrapper">
+          {render({
+            ourProps: {},
+            theirProps: props,
+            slot: { accept: true },
+            defaultTag: 'div',
+            name: 'Dummy',
+          })}
+        </div>
+      )
+    }
+
+    testRender(<Dummy as={Fragment}>{() => <span data-accept="always">Contents</span>}</Dummy>)
+
+    expect(contents()).toMatchSnapshot()
+  })
+
   it(
     'should error when we are rendering a Fragment with multiple children',
     suppressConsoleLogs(() => {

packages/@headlessui-react/src/utils/render.ts

@@ -213,12 +213,35 @@ function _render<TTag extends ElementType, TSlot>(
 
         let classNameProps = newClassName ? { className: newClassName } : {}
 
+        // Merge props from the existing element with the incoming props
+        let mergedProps = mergePropsAdvanced(
+          resolvedChildren.props as any,
+          // Filter out undefined values so that they don't override the existing values
+          compact(omit(rest, ['ref']))
+        )
+
+        // Make sure that `data-*` that already exist in the `mergedProps` are
+        // skipped.
+        //
+        // Typically we want to keep the props we set in each component because
+        // they are required to make the component work correctly. However, in
+        // case of `data-*` attributes, these are attributes that help the end
+        // user.
+        //
+        // This means that since the props are not required for the component to
+        // work, that we can safely prefer the `data-*` attributes from the
+        // component that the end user provided.
+        for (let key in dataAttributes) {
+          if (key in mergedProps) {
+            delete dataAttributes[key]
+          }
+        }
+
         return cloneElement(
           resolvedChildren,
           Object.assign(
             {},
-            // Filter out undefined values so that they don't override the existing values
-            mergePropsAdvanced(resolvedChildren.props as any, compact(omit(rest, ['ref']))),
+            mergedProps,
             dataAttributes,
             refRelatedProps,
             { ref: mergeRefs((resolvedChildren as any).ref, refRelatedProps.ref) },