[BUG] Object keys are not detected in common calles

[kachkaev]

Describe the bug

There are several common callees which support objects with class names in keys. The existing implementation of a few rules only scans object keys in a function named classnames. This results in false negatives when a rule like tailwindcss/no-custom-classname is enabled.

https://github.com/francoismassart/eslint-plugin-tailwindcss/blob/6b6c0dd28e123cc118bff83654f951f736fa58e8/lib/rules/no-arbitrary-value.js#L109-L116

To Reproduce

// file=eslint.config.js
{
  "extends": ["next/core-web-vitals", "plugin:tailwindcss/recommended"],
  "settings": {
    "tailwindcss": {
      "config": "./tailwind.config.js",
      "callees": [
        "cva", // https://cva.style

        // a project is typically configured with one of these, the shape of arguments matches `classnames`
        "classnames",
        "classNames",
        "clsx",
        "cn",
        "cns",
        "cx"
      ]
    }
  },
  "rules": {
    "tailwindcss/no-custom-classname": "error"
  }
}

import { cva } from "class-variance-authority";
import classnames from "classnames";
import Image from "next/image";

cva("text-white", "bg-black", {
  someVariant: {
    s: "p-4",
    "p-oops": "p-10", // 🎉 no ESLint error here (key is not a class name in `cva`)
    anything: "p-oops", // 🎉 ESLint error here: `Classname 'p-oops' is not a Tailwind CSS class! tailwindcss/no-custom-classname`
  },
});

classnames("text-white", "bg-black", {
  "p-4": true,
  "p-oops": true, // 🎉 ESLint error here: `Classname 'p-oops' is not a Tailwind CSS class! tailwindcss/no-custom-classname`
});

// `classNames` is an alternative import name from `classnames`
const classNames = classnames;
classNames("text-white", "bg-black", {
  "p-4": true,
  "p-oops": true, // 🚨 no ESLint error here
});

// https://www.npmjs.com/package/clsx claims to be a faster alternative to `classnames`
const clsx = classnames;
clsx("text-white", "bg-black", {
  "p-4": true,
  "p-oops": true, // 🚨 no ESLint error here
});

// `cn` is a shorthand for `classnames`, seen used as a wrapper for `clsx` + `tailwind-merge`
// https://github.com/reactjs/react.dev/blob/842c24c9aefaa60b7ae9b46b002bd1b3cf4d31f3/src/components/Tag.tsx#L5
// https://github.com/shadcn-ui/ui/blob/33a5fc7966cfe8887dac732f64582f40162869c6/apps/www/lib/utils.ts#L4-L6
const cn = classnames;
cn("text-white", "bg-black", {
  "p-4": true,
  "p-oops": true, // 🚨 no ESLint error here
});

// Some people may prefer `cns` instead of `cn` as a shorthand for `classnames`
// https://github.com/francoismassart/eslint-plugin-tailwindcss/issues/99#issuecomment-1014361250
const cns = classnames;
cns("text-white", "bg-black", {
  "p-4": true,
  "p-oops": true, // 🚨 no ESLint error here
});

Expected behavior

I’d expect all my callees except cva to work just a classnames function. It seems more reasonable than to put cva in the deny-list rather than classnames into the allow list for keys.

Screenshots

Environment (please complete the following information):

• OS: macOS
• Softwares + version used:
  • "eslint-plugin-tailwindcss": "3.13.0"

Additional context

Support for object keys was requested in #99 and got implemented in #103. Then #135 was raised, requesting support for cva (which produced false positives at the time). This issue with cva was resolved in #185. When doing so, support for all classname function aliases got dropped.

If the sole goal of #185 was to support cva, we can probably avoid a new config option like calleesWithClassNamesInObjectKeys. 😬 Maybe this fix would do?

-const isUsedByClassNamesPlugin = node.callee && node.callee.name === 'classnames';
+const isUsedByCva = node.callee && node.callee.name === 'cva';

-const propVal = isUsedByClassNamesPlugin || isVue ? prop.key : prop.value;
+const propVal = !isUsedByCva || isVue ? prop.key : prop.value;

In the meantime, I will probably change the signature of my custom cn function (clsx + tailwind-merge) like this:

/**
 * @example
 *
 *   ```ts
 *   cn(
 *     'some-class some-other-class',
 *      conditionA && 'some-additional-class',
 *      conditionB ? 'some-class-foo' : 'some-class-bar'
 *     )
 *   ```
 *
 * @param inputs A list of strings (including conditional ones)
 *
 *               We don’t use `ClassValue` from `clsx` because it also allows objects.
 *               Object keys are not checked by `eslint-plugin-tailwindcss`.
 *
 * @see https://github.com/francoismassart/eslint-plugin-tailwindcss/issues/267
 */
export function cn(...inputs: (string | number | null | boolean | undefined)[]) {
  return mergeTailwindClasses(clsx(inputs));
}

eslint config file or live demo

MWE repo: kachkaev/eslint-plugin-tailwindcss-issue-267-mwe

[kachkaev]

I have managed to patch eslint-plugin-tailwindcss in https://github.com/kachkaev/eslint-plugin-tailwindcss-issue-267-mwe/pull/1. It seems to be doing what I expect it to. Can this be converted into a PR or am I missing a use case that can throw a spanner in the works?

I guess we can try hardcoding cva just as it currently done for classnames. If that causes issues (which I doubt because cva is quite unique), we can create an new settings option.

[joafeldmann]

I encountered the same issue, and I can confirm that @kachkaev's patch resolves it. It would be great if this fix could be incorporated!

[kachkaev]

I’ve stopped using this patch myself. Turns out that object syntax inside cn is not that useful and can be omitted:

cn("text-white bg-black", {
  "p-4": condition,
  "p-oops": anotherCondition, // 🚨 no ESLint error here
});

↓

cn(
  "text-white bg-black",
  condition && "p-4",
  anotherCondition && "p-oops", // 🎉 ESLint error here: `Classname 'p-oops' is not a Tailwind CSS class! tailwindcss/no-custom-classname`
);

My cn function:

/**
 * Same as `ClassNameValue` from `tailwind-merge`, but without support for arrays
 * (to keep arguments simple)
 */
export type ClassNameValue = string | null | undefined | 0 | false;

/**
 * @example `cn('foo', condition && 'bar', condition && 'baz')`
 */
export function cn(...inputs: ClassNameValue[]): string {
  return twMerge(inputs);
}

This function used clsx, this dependency can be uninstalled.

Because objects inside cva are supported by eslint-plugin-tailwindcss anyway, all class names in my projects are covered even without a patch. Here is what I have in .eslintrc.js:

  settings: {
    tailwindcss: {
      callees: ['cn', 'cva'],
      ignoredKeys: ['defaultVariants'],
      config: `${__dirname}/tailwind.config.ts`,
    },
  },

This issue is still valid, but one can bypass it by stepping a bit to the side.

[asportnoy]

@francoismassart would you be able to add @kachkaev's patch above to this repo? Would be great to have.

[zegs21]

I am giving a little bump to this as my team has a huge codebase that uses keys as class names and we ran into this issue, @kachkaev patch would be great to have

[kachkaev]

@zegs21 consider dropping object arguments completely, as described in https://github.com/francoismassart/eslint-plugin-tailwindcss/issues/267#issuecomment-1907798102. I’m quite happy with just &&s for conditional styles after a few months of banning { "class": condition } syntax. There is now only one way of doing one thing instead of two, which reduces cognitive load and PR discussions 👍 The initial migration from {} to && was quite smooth too (and our team’s codebase is not small).