Skip to content
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

[BUG] Classes not detected inside objects with computed property (using clsx callee) #331

Open
redocean10 opened this issue Mar 31, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@redocean10
Copy link

Describe the bug
The plugin won't identify classes inside objects that have a computed property (object[property] or object.property), so doesn't check for any rules (classnames-order in this case).

To Reproduce

// .eslintrc.cjs
module.exports = {
  extends: [
    "plugin:tailwindcss/recommended",
  ],
  settings: {
    tailwindcss: {
      callees: ["clsx"],
    }
  },
  rules: {
    "tailwindcss/no-custom-classname": "off",
     },
  }
// Alert.jsx
import clsx from 'clsx'

let type = 'error'
const alertClass = clsx(
  {
    success: 'border-green-400 bg-green-100 text-green-700',
    warning: 'text-yellow-700 border-yellow-400 bg-yellow-100',
    error: 'bg-red-100 border-red-400 text-red-700',
  }[type],
)

Expected behavior
The plugin should detect the classes and warn of the invalid order.

Screenshots
With the computed property (no linting):
image

Without property (linting works fine):
image

Environment (please complete the following information):

  • OS: Linux Mint 21.2 x86_64
  • Softwares + version used:
    • VSCode 1.87.2
    • "eslint-plugin-tailwindcss": "3.15.1"

Current fix/ workaround:

I managed to fix this issue for the classnames-order rule, by updating the callExpressionVisitor function inside classnames-order.js:

const callExpressionVisitor = function (node) {
  const calleeStr = astUtil.calleeToString(node.callee);
  if (callees.findIndex((name) => calleeStr === name) === -1) {
    return;
  }

  node.arguments.forEach((arg) => {
    if (arg.type === 'MemberExpression' && arg.object.type === 'ObjectExpression') {
      arg.object.properties.forEach((prop) => {
        if (prop.value.type === 'Literal') {
          sortNodeArgumentValue(node, prop.value);
        }
      });
    } else {
      sortNodeArgumentValue(node, arg);
    }
  });
};
@redocean10 redocean10 added the bug Something isn't working label Mar 31, 2024
@kachkaev
Copy link
Contributor

kachkaev commented Apr 2, 2024

Hmm not sure if creating an object and then picking a single element from it is an efficient way of achieving what you are after. If there is ever a fix, it will encourage odd-looking clsx calls which may look confusing for collaborators.

Either way, you can do this in the meantime:

const alertClassName = clsx(
  "", // ← common class names go here
  type === "success" && "border-green-400 bg-green-100 text-green-700",
  type === "warning" && "text-yellow-700 border-yellow-400 bg-yellow-100",
  type === "error" && "bg-red-100 border-red-400 text-red-700"
);

Or, even better, you can leverage cva:

import { cva } from "class-variance-authority";

const alertVariants = cva(
  "", // ← common class names go here
  {
    variants: {
      type: {
        success: "border-green-400 bg-green-100 text-green-700",
        warning: "text-yellow-700 border-yellow-400 bg-yellow-100",
        error: "bg-red-100 border-red-400 text-red-700",
      },
    },
  }
);

const alertClassName = alertVariants({ type });

Both alternatives are covered by this plugin.

@redocean10
Copy link
Author

Thanks for your suggestions. It seemed like a simple way to handle variants to me, but I agree it doesn't look right. I'm using a cleaner approach now that works with the plugin, so maybe this bug was my error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants