Skip to content

Class getting stripped out unnecessarily #218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
brandonmcconnell opened this issue Apr 12, 2023 · 12 comments
Closed

Class getting stripped out unnecessarily #218

brandonmcconnell opened this issue Apr 12, 2023 · 12 comments
Labels
context-v1 Related to tailwind-merge v1

Comments

@brandonmcconnell
Copy link

brandonmcconnell commented Apr 12, 2023

I have this, roughly—

let computedClasses = cls(
  'inline-block text-base text-left font-semibold leading-tight',
  'text-xl'
);

console.log(computedClasses); // -> 'inline-block text-left font-semibold text-xl'

I can't figure out why leading-tight is getting removed.

Here is my implementation of cls:

import clsx, { type ClassValue } from 'clsx';
import { extendTailwindMerge, validators } from 'tailwind-merge';
import { spreadKeys } from '@carevoyance/calculations';

const trbl = ['t', 'r', 'b', 'l'];
const xy = ['x', 'y'];
const dirs = { trbl, xy, all: [...trbl, ...xy] };

const twMerge = extendTailwindMerge({
  classGroups: {
    shadow: [{
      shadow: [{
        ...spreadKeys(dirs.trbl, ['', 'inner', validators.isTshirtSize]),
        border: [
          '',
          validators.isNumber,
          validators.isArbitraryLength,
          spreadKeys(dirs.all, [validators.isNumber, validators.isArbitraryLength]),
        ],
      }],
    }],
  },
});

export const cls = (...inputs: ClassValue[]) => twMerge(clsx(...inputs));

Thanks for all the help, as always 🙏🏼

@dcastil
Copy link
Owner

dcastil commented Apr 15, 2023

Hey @brandonmcconnell! 👋

This is because text-xl also sets line-height (line-height: 1.75rem; to be exact). If you want to preserve leading-tight, you need to set it after text-xl.

It's similar to how px-4 p-5 gets reduced to p-5 because you want to override the paddings set by px-4 but p-5 px-4 stays intact because tailwind-merge sees that as a refinement to p-5.

@dcastil
Copy link
Owner

dcastil commented Apr 15, 2023

But I know that this is confusing. There were already several issues about this exact problem. Not many people know that Tailwind CSS is doing this. 🤔

@brandonmcconnell
Copy link
Author

@dcastil Thanks for clarifying that! I believe—though I could be wrong—if you have both a font-size utility like text-xs and a line-height utility like leading-tight, leading-_ will override the line-height regardless of the order (in pure TailwindCSS).

It would be great if tailwind-merge offered a setting to allow utilities like these and those you mentioned to not be considered conflicts and to allow TailwindCSS to handle their overrides naturally.

@brandonmcconnell
Copy link
Author

Also, if they're considered conflicts and I set leading-x after text-x, does that mean that leading would be prioritized and the text utility would be removed instead, making it difficult/impossible to have both?

@dcastil
Copy link
Owner

dcastil commented Apr 15, 2023

If you put leading-x after text-x, both stay in there since you want to override the line-height of text-x but keep its font-size. The override happens through the CSS order in that case.

But e.g. think of a component that defines a line-height with leading-x inside and then you want to do <MyComponent className="text-xl" /> (which then results in twMerge('… leading-x', props.className)). The dev might want to override leading-x here with the line-height in text-xl, so tailwind-merge needs to remove that.

But you can change this behavior by the way! You can remove this conflict from the config (this one).

const twMerge = extendTailwindMerge({ /* your config */ }, config => {
    delete config.conflictingClassGroups['font-size']
    return config
})

@brandonmcconnell
Copy link
Author

@dcastil Thanks!

@dcastil
Copy link
Owner

dcastil commented Apr 18, 2023

@brandonmcconnell I'm closing this issue as resolved. Let me know if your issue persists. 😊

@dcastil dcastil closed this as completed Apr 18, 2023
@brandonmcconnell
Copy link
Author

@dcastil Just wanted to confirm— using delete config.conflictingClassGroups['font-size'] will still remove conflicting font-size utilities of the same type, correct? For exmaple, twMerge('text-lg text-sm') would still compile to text-sm, right?

@dcastil
Copy link
Owner

dcastil commented Apr 29, 2023

Yes! config.conflictingClassGroups['font-size'] only holds the information that font-size classes clash with line-height classes. Classes from the same group always conflict with each other.

@brandonmcconnell
Copy link
Author

@dcastil Awesome, thanks!

@dcastil
Copy link
Owner

dcastil commented May 16, 2023

Mentioned this issue in #226 (reply in thread).

@mrienstra
Copy link

mrienstra commented May 23, 2023

Simple TypeScript transformer which spits something out on the console (process.env.NODE_ENV === "development" only) when a class is dropped.

import clsx, { type ClassValue } from "clsx";
import { twMerge } from "tailwind-merge";

const missingWords = (before: string, after: string) => {
  const afterArr = after.split(" ");
  return before
    .split(" ")
    .filter((w) => !afterArr.includes(w))
    .join(" ");
};

const twM = (...inputs: ClassValue[]) => {
  const joinedClasses = clsx(...inputs);
  const mergedClasses = twMerge(joinedClasses);
  if (
    process.env.NODE_ENV === "development" &&
    joinedClasses !== mergedClasses
  ) {
    console.trace(
      `twM: "${joinedClasses}" lost "${missingWords(
        joinedClasses,
        mergedClasses
      )}"`
    );
  }

  return mergedClasses;
};

export default twM;

Sample output:
image


If you want a shorter stack trace, you can use this approach, but you won't be able to click through to the source file (or at least I haven't been able to figure out how):

const getCaller = () => {
  const err = Error("");
  const callerLine = err?.stack?.split("\n")[4];
  return callerLine?.trim().replace(/^at /, "from ");
};

Replace the console.trace call with:

    console.log(
      `twM: "${joinedClasses}" lost "${missingWords(
        joinedClasses,
        mergedClasses
      )}"
      ${getCaller() || ""}`
    );

Sample output:
image

getCaller based on https://stackoverflow.com/questions/1340872/how-to-get-javascript-caller-function-line-number-and-caller-source-url#answer-3806596

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
context-v1 Related to tailwind-merge v1
Projects
None yet
Development

No branches or pull requests

3 participants