primitives icon indicating copy to clipboard operation
primitives copied to clipboard

Allow customizing the `mergeProps` behavior for `Slot`

Open bengry opened this issue 1 year ago • 2 comments

Feature request

Add a prop or otherwise allow customize the way <Slot> handles prop merging. Specifically, it would be beneficial for tailwind users who often use https://github.com/dcastil/tailwind-merge to remove conflicting class names.

Overview

I'd like to be able to control the way className is merged. Right now the behavior is naive, resembling https://github.com/lukeed/clsx (but simpler). When using Tailwind, something like this would create a conflict:

// abbreviated implementation
function Overflow(
  { children, asChild }: { children: React.ReactNode, asChild?: boolean }
) {
  return (
    <Slot className="truncate break-normal text-md">
      {asChild ? children : <span>{children}</span>}
    </Slot>
  );
}

// usage

<Overflow asChild>
  <span className="text-sm">Excepteur Lorem adipisicing ut culpa commodo</span>
</Overflow>

This would end up rendering this HTML:

<span className="truncate break-normal text-md text-sm">
  Excepteur Lorem adipisicing ut culpa commodo
</span>

Here, text-sm and text-md conflict, and it's not clear who should/would win. When using tailwind-merge, the last class in precedence wins, in this case - the final rendered HTML should be this:

<span className="truncate break-normal text-sm">
  Excepteur Lorem adipisicing ut culpa commodo
</span>

Because it was provided on the child given to <Slot>.

Right now the only way to change this is to patch or fork the <Slot> implementation, replacing L117 with twMerge.

This in turn also means there are now two implementations of Slot in practice, given that someone uses more than just the Slot component from Radix - since other libraries depend on @radix-ui/react-slot as a peer dependency.

Two ways I thought about for solving this:

  1. Provide an optional prop for mergeProps, so one could use Slot as a primitive and build on of their own on top of it. a. Or maybe even just a classNameMerger (of type (parentClassName?: string, childClassName?: string) => string)
  2. Allow customizing the mergeProps for all <Slot> instances using an optional Context provider. Similar to how <Tooltip> can be globally configured. a. 1.a. above also applies here. I'm not sure what the performance cost would be for either option.

Examples in other libraries

None that I could find, but this blog post showcases how to implement a Radix-like <Slot> component, and touches on this very issue, incorporating tailwind-merge into their solution.

Who does this impact? Who is this for?

Tailwind users

Additional context

Related, but not the same: https://github.com/radix-ui/primitives/issues/1216, https://github.com/radix-ui/primitives/pull/2234, https://github.com/radix-ui/primitives/pull/2336

bengry avatar Jan 04 '24 22:01 bengry

I had the same issue, The way i solve it is by using a custom component as a child, not span

function Child({ className, ...props }: React.HTMLAttributes<HTMLSpanElement>) {
  return <span className={cn("text-sm", className)} {...props} />;
}

function Overflow({ className, asChild, ...props }: React.HTMLAttributes<HTMLElement> & { asChild?: boolean }) {
  const Component = asChild ? Slot : "span";
  return <Component className={cn("truncate break-normal text-md", className)} {...props} />;
}

and then use it like:

<Overflow asChild>
  <Child>Excepteur Lorem adipisicing ut culpa commodo</Child>
</Overflow>

But IMO the classes from the parent (Overflow) should "win". The resulting HTML contains only text-md.

nklhtv avatar Jan 18 '25 10:01 nklhtv

In addition to customizing className merging, which is essential when using Tailwind as OP stated even at the beginning of a project using Radix UI and Tailwind, there are other props that are important to handle in large codebases.

For example:

  • React Aria's mergeProps handles multiple id props
  • Instead of naively chaining event handlers, one might want to use composeEventHandlers where if preventDefault() is called in the first event handler, the second isn't triggered
  • The list goes on--prop merging is too subjective to the project imo to be locked behavior, especially className merging

imhoffd avatar May 19 '25 15:05 imhoffd