BUG: TW4 + TW merge + CVA some base classes are not applied

[sebalaini]

Describe the bug

I opened the issue in cva 335 but after some digging, I think is related to TW merge 🤔 looks like some classes are removed from the final class list.

The classes I'm having a problem with are font-xxx & leading-xxx

To Reproduce

This is my config:

import { extendTailwindMerge } from 'tailwind-merge'

export const customTwMerge = extendTailwindMerge({
  extend: {
    theme: {
      spacing: ['0', 'xs', 'sm', 'md', 'lg', 'xl', '2xl', '3xl', '4xl', 'auto'],
      // leading: ['sm', 'md', 'lg'],
      radius: ['none', 'sm', 'md', 'lg', 'xl', 'round', 'pill'],
    },
  },
})

import clsx, { type ClassValue } from 'clsx'
import { customTwMerge } from './tailwind-merge'

// export function cn(...inputs: ClassValue[]) {
//  return customTwMerge(clsx(inputs))
// }
export function cn(...inputs) {
  return customTwMerge(inputs)
}

This is the new way I have to do it to make it work, which doesn't seem correct.

The commented out way or even the one posted in the above issue doesn't work anymore with TW4 and the new version of TW Merge

export const Typography = ({
  as,
  size,
  bold,
  light,
  className,
  children,
  ...props
}: Props) => {
  const TypographyTag = React.createElement<any>(
    as,
    {
      className: `font-secondary font-normal leading-md tracking-md mb-md ${cn(typographyVariants({ size, bold, light }), className)}`,
      // className: `${cn(`font-secondary font-normal leading-md tracking-md mb-md ${typographyVariants({ size, bold, light })}`, className)}`,
      ...props,
    },
    children
  )

  return TypographyTag
}

Especially because I can override one of those classes that doesn't work, like this:

            <Typography
              as='span'
              className={cn(
                `font-tertiary mx-auto ${index === 2 ? 'text-red-500' : 'text-neutral-white'}`
              )}
            >
              {price ? price : '/'}
            </Typography>

Expected behavior

both font-xxx & leading-xxx should be applied to the final class list.

[paleite]

I'm having the same issue with "leading-" not being set when it's inside the first argument.

[sebalaini]

🤔 Given the situation, I added some tests, and it seems TW merge is the problem.

I added these 4 simple tests to check the output, and TW merge with some classes looks like it doesn't return the correct ones.

describe('Link', () => {
  it('clsx returns correct classes', () => {
    const classes = 'leading-md font-secondary font-normal tracking-md mb-md'
    expect(clsx(classes)).toBe(classes)
  })

  it('clsx returns correct classes without removal', () => {
    const classes =
      'leading-md font-secondary font-normal text-sm font-bold text-grey-200 tracking-md mb-md'
    expect(clsx(classes)).toBe(classes)
  })

  it('cn returns correct classes', () => {
    const classes = 'leading-md font-secondary font-normal tracking-md mb-md'
    expect(cn(classes)).toBe(classes)
  })

  it('cn returns correct classes without removal', () => {
    const classes =
      'leading-md font-secondary font-normal text-sm font-bold text-grey-200 tracking-md mb-md'
    expect(cn(classes)).toBe(
      'leading-md font-secondary text-sm font-bold text-grey-200 tracking-md mb-md'
    )
  })
})

The last one is the failing one:

Expected: "leading-md font-secondary text-sm font-bold text-grey-200 tracking-md mb-md"
Received: "font-secondary text-sm font-bold text-grey-200 tracking-md mb-md"

It doesn't matter if you have this configuration or not inside the extend.theme

leading: ['sm', 'md', 'lg'],

[sebalaini]

It depends on the class order.

for example, this work:

'font-secondary text-sm font-normal font-bold text-grey-200 tracking-md leading-md mb-md'

but this doesn't

'font-secondary font-normal font-bold text-grey-200 tracking-md leading-md text-sm mb-md'

The former can be found when you have compoundVariants that you change the text size, like this:

const titleVariants = cva([], {
  variants: {
    level: {
      1: [
        'font-primary',
        'text-2xl',
        'sm:text-3xl',
        'font-bold',
        'leading-sm',
        'tracking-xs',
        'mb-xl',
      ],

...

  compoundVariants: [
    {
      level: 1,
      large: true,
      class: ['text-4xl', 'sm:text-landing', 'md:text-2landing'],
    },
...

or even when you have shared classes like this:

className: `${cn('font-secondary font-normal tracking-md leading-md mb-md', typographyVariants({ size, bold, light }), className)}`

[dcastil]

Hey @sebalaini! 👋

The reason why the leading-md class gets removed is because text-sm sets a line-height CSS property, overriding line-height classes if you place it afterwards. If you want to change this overriding behavior and don't use line-heights in your font-size classes, you can override the conflict between the font-size and line-height classes:

import { extendTailwindMerge } from 'tailwind-merge'

const twMerge = extendTailwindMerge({
    override: {
        conflictingClassGroups: {
            // In the default config the value is ['leading']
            // See https://github.com/dcastil/tailwind-merge/blob/v3.0.2/src/lib/default-config.ts#L2095
            'font-size': []
        }
    },
    // … rest of your config
})

---

Related: #503, #482, #446, #401, #257, #218, #187, #59

[sebalaini]

hey @dcastil ty for your response.

With that snippet seems to look properly now :)

[dcastil]

Great! Let me close this issue. If there is still a problem, just let me know and I'll reopen.

[unional]

Hi, I have the same issue but it's a bit more complicated.

I'd working on a new design system replacing the existing one.

In the old system, I have:

--text-sm: 0.875rem;
--text-sm--line-height: 1.25rem;

i.e. the class text-sm affects both font-size and line-height, as how Tailwind does it.

In the new design system, I remove that connection to better support CJK languages:

--text-75: 0.875rem;

--body-text-sm: var(--text-75);

I need to support backward compatibility, so if I do conflictingClassGroups then the old classes would not work.

Any suggestion?

[dcastil]

@unional you could create a complete separate class group for your new font-size classes and specify with which other class groups that new group is in conflict.

Just keep in mind that when you use extendTailwindMerge, you need to specify the new class group specifically via type arguments:

import { extendTailwindMerge } from 'tailwind-merge'

const twMerge = extendTailwindMerge<'font-size-new'>({
    extend: {
        classGroups: {
            'font-size-new': ['text-75'],
        },
        conflictingClassGroups: {
            'font-size-new': ['font-size'],
            'font-size': ['font-size-new'],
        },
    },
})

[unional]

Hi @dcastil, I tried what you have suggested. It works for some case but I found it doesn't work for the following:

@utility body-emphasized {
  font-style: italic;
  font-weight: 400;
}

@utility body-strong {
  font-style: normal;
  font-weight: 700;
}

const twMerge = extendTailwindMerge<'font-style-weight'>({
  extend: {
    classGroups: {
      'font-style-weight': ['body-emphasized', 'body-strong']
    },
    conflictingClassGroups: {
      'font-style-weight': ['font-style', 'font-weight'],
      'font-style': ['font-style-weight'],
      'font-weight': ['font-style-weight']
    }
  }
}

// works: { fontStyle: normal, fontWeight: 400 }
<div className={twMerge('body-emphasized', 'not-italic')}/>
// does not work: { fontStyle: normal, fontWeight: 800 }
<div className={twMerge('body-emphasized', 'font-extrabold')}/>

// does not work: { fontStyle: italic, fontWeight: 400 }
<div className={twMerge('body-strong', 'italic')}/>
// works: { fontStyle: normal, fontWeight: 800 }
<div className={twMerge('body-strong', 'font-extrabold')}/>

I am using the latest 4.0.15.

Am I defining it wrong? Do you want me to create a new issue?

UPDATE:
Never mind, figured it out. The config should be:

const twMerge = extendTailwindMerge<'font-style-weight'>({
  extend: {
    classGroups: {
      'font-style-weight': ['body-emphasized', 'body-strong']
    },
    conflictingClassGroups: {
      'font-style-weight': ['font-style', 'font-weight']
    },
    conflictingClassGroupModifiers: {
      'font-style-weight': ['font-style', 'font-weight']
    }
  }
}

[dcastil]

@unional yeah the problem is that with

const twMerge = extendTailwindMerge<'font-style-weight'>({
  extend: {
    // …
    conflictingClassGroups: {
      'font-style': ['font-style-weight'],
      'font-weight': ['font-style-weight'],
      // …
    }
  }
}

you define that the font-style-weight classes should be removed when a font-style or font-weight class comes after it.

I recommend to not create utilities like this that bundle multiple CSS values together when using tailwind-merge and instead use variables in JS that hold multiple classes. Then you don't need to configure tailwind-merge which can become quite cumbersome otherwise.

More here: https://github.com/dcastil/tailwind-merge/blob/v3.0.2/docs/recipes.md#extracting-classes-with-tailwinds-apply

[unional]

Yeah, it is tedious. Thanks for your tips.
I'm trying to figure out the right approach as I'm building a design system with it.
Maybe it is better to keep a 1-1 map between the design tokens (CSS variables) and the Tailwind classes.

[dcastil]

When I want to create fixed groups with certain styles, I create a file like typography.ts in my design system where these groups get defined.

export const TYPOGRAPHY_BODY_SMALL = 'text-sm font-medium …'
export const TYPOGRAPHY_BODY_MEDIUM = 'text-base font-normal …'

Imagine that e.g. TYPOGRAPHY_BODY_SMALL replaces a class like

.body-small {
    font-size: …;
    font-weight …;
    …
}

Then in my components, I use the variable instead of the class essentially.

import { twMerge } from 'tailwind-merge'
import { TYPOGRAPHY_BODY_SMALL } from '…'

function MyComponent({ className }) {
    return <div className={twMerge(TYPOGRAPHY_BODY_SMALL, '…', className)} />
}

This way you can still use a predefined set of styles like you would in a design system but don't need to add any config with complex conflict resolution to tailwind-merge.