Variant Typings Don't Respect Composing

[BorisZubchenko]

Describe the bug
User Story:

1. I have a Button:

const Button = classed('button', {
  defaultVariants: {
    size: 'default',
    variant: 'default',
  },
  variants: {
    size: {
      default: '...',
      sm: '...',
    },
    variant: {
      default: '...',
      ghost: '...',
    },
  },
})

2. I want to style a Toggle component
3. I realize that a Toggle is basically a Button with its own logic. It should be small and ghost by default.
4. So, regarding Composing Components, I'm creating a Toggle based on a Button:

const Toggle = classed(TogglePrimitive, Button, {
  defaultVariants: {
    size: 'sm', // <-- ERROR HERE, see below
    variant: 'ghost',
  },
  variants: {
    variant: { // additional Toggle styles to be merged with Button styles
      default: '...'
      ghost: '...',
    },
  },
})

5. Oops! I can't set defaultVariant#size since the variants prop doesn't have size. However, I should be able to do this since the "composable" Button has it.

To Reproduce
Copy these two components into your project (it would be great to have a StackBlitz template).

Expected behavior
The error should not appear.

Environment? (please complete the following information):

• Version 1.7.0 (latest at the moment)

[BorisZubchenko]

Also, with the following Toggle:

const Toggle = classed('div', Button, {
  defaultVariants: {
    size: 'sm',
    variant: 'ghost',
  },
  variants: {
    size: {
      sm: '',
    },
    variant: {
      ghost: '',
    },
  },
})

I can't use default variants:

Toggle({
  variant: 'default' // ERROR, only `'ghost' | undefined`
})

However, maybe it's even good, because we can limit variant set for the extended component.

[BorisZubchenko]

And the most unavoidable:

Providing Tailwind classes to the extended component overrides the base ones:

const Button = classed('button', {
  defaultVariants: {
    variant: 'default',
  },
  variants: {
    variant: {
      default: '1 2 3',
    },
  },
})

const Toggle = classed(TogglePrimitive, Button, {
  variants: {
    variant: {
      default: '4 5'
    },
  },
})

expect(
  Toggle().className
).toEqual(
  '1 2 3 4 5' // actual: 4 5
)

[asanger]

I'm trying to use this in the same way and am running into the same issues here.

[sannajammeh]

Providing Tailwind classes to the extended component overrides the base ones:

This is intended behavor

[sannajammeh]

Describe the bug User Story:

1. I have a Button:

const Button = classed('button', {
  defaultVariants: {
    size: 'default',
    variant: 'default',
  },
  variants: {
    size: {
      default: '...',
      sm: '...',
    },
    variant: {
      default: '...',
      ghost: '...',
    },
  },
})

2. I want to style a Toggle component
3. I realize that a Toggle is basically a Button with its own logic. It should be small and ghost by default.
4. So, regarding Composing Components, I'm creating a Toggle based on a Button:

const Toggle = classed(TogglePrimitive, Button, {
  defaultVariants: {
    size: 'sm', // <-- ERROR HERE, see below
    variant: 'ghost',
  },
  variants: {
    variant: { // additional Toggle styles to be merged with Button styles
      default: '...'
      ghost: '...',
    },
  },
})

5. Oops! I can't set defaultVariant#size since the variants prop doesn't have size. However, I should be able to do this since the "composable" Button has it.

To Reproduce Copy these two components into your project (it would be great to have a StackBlitz template).

Expected behavior The error should not appear.

Environment? (please complete the following information):

• Version 1.7.0 (latest at the moment)

This should not error, you are correct. The rest is intended

[sannajammeh]

Got a fix locally, will see to it tomorrow

[BorisZubchenko]

Providing Tailwind classes to the extended component overrides the base ones:

This is intended behavor

Thus, there's no way to actually extend components with additional classes.

If we had the proposed extension, we would be able to remove (reset to initial) some of the base classes:

merge('text-lg', 'text-initial') // results 'text-initial'
merge('text-lg', 'font-italic') // results 'text-lg font-italic'

However, now we can only overwrite.

[sannajammeh]

Providing Tailwind classes to the extended component overrides the base ones:

This is intended behavor

Thus, there's no way to actually extend components with additional classes.

If we had the proposed extension, we would be able to remove (reset to initial) some of the base classes:

merge('text-lg', 'text-initial') // results 'text-initial'
merge('text-lg', 'font-italic') // results 'text-lg font-italic'

However, now we can only overwrite.

Yes there is. We have the base prop in the config object for this exact purpose.

 {
   base: "base classes",
   variants: {
    ...
   }
 },
{
  base: "base 2"
}

This is extendable and combined with merger as twMerge in createClassed allows for what you're describing

[BorisZubchenko]

Yes, but the same doesn't apply with the variants. Look at this comment. So, the bases are merged but the variants aren't.

[sannajammeh]

Yes, but the same doesn't apply with the variants. Look at this comment. So, the bases are merged but the variants aren't.

I see. I will likely add this as a flag in V2 release.

export const { classed } = createClassed({ merger: twMerge, mergeVariants: true });

The reason is that some, including me, rely on variants being able to completely override.