francoismassart / eslint-plugin-tailwindcss

ESLint plugin for Tailwind CSS usage
https://www.npmjs.com/package/eslint-plugin-tailwindcss
MIT License
1.47k stars 70 forks source link

[BUG] size is not always equivalent to h- and w- #315

Closed bodograumann closed 8 months ago

bodograumann commented 8 months ago

Describe the bug While in general it is a good idea to use size- instead of h- and w-, those two definitions are not the same in all situations. In particular if the selected tokens do not com from the spacing scale, the same names can mean different things. It could also happen, that the size- utility does not exist at all.

To Reproduce Steps to reproduce the behavior:

  1. Define the following tailwind settings:
    export default {
        theme: {
            width: {
                dialog: "26rem"
            },
            height: {
                dialog: "30rem"
            }
        }
    };
  2. Use the tokens like this: class="h-dialog w-dialog". This works as intended.
  3. Run eslint autofixes with tailwindcss-plugin installed. This replaces the above with class="size-dialog".
  4. The size-dialog class does not actually exist, because dialog does not exist in the spacing nor in the size scale. In fact the area is not even square.

Expected behavior Ideally the linting rules would detect, that the two variants are not equivalent and not suggest the replacement.

Environment (please complete the following information):

francoismassart commented 8 months ago

@bodograumann thank you for reporting this issue.

I made a basic test on Tailwind Play https://play.tailwindcss.com/PS0lBwtsJc

In the test the config I use has the same value for the square key on both width and height

      width: {
        custom: '160px',
        square: '100px',
      },
      height: {
        custom: '90px',
        square: '100px',
      },

Yet, the size-square classname is not generated by Tailwind... The size classnames are in fact based on the spacing key of the configuration...

I'll work on it soon, for now you can simply disable the entire rule or just specific lines where it is needed.

francoismassart commented 8 months ago

Please try it via npm i eslint-plugin-tailwindcss@3.14.2-beta.0 -D and let me know how it goes. 😉

bodograumann commented 8 months ago

Thanks for looking at this so quickly, @francoismassart . I can confirm that the change in https://github.com/francoismassart/eslint-plugin-tailwindcss/commit/228f3b78d399d69225df4490d9cf1a6c7ee635c5 indeed avoids the problem.

It is a bit too conservative though, I feel. There are some values allowed for size- that should work well. Not sure about all the details of the tailwind implementation, but e.g. size-full is the same as h-full w-full, as long as the user has not overriden any of those utils.

francoismassart commented 8 months ago

I thought (reading their doc) that it was only based on the spacing config...

But I can see a dedicated entry in the config for size in the Tailwind CSS repository.

I'll release a new beta later today

francoismassart commented 8 months ago

@bodograumann Please try this beta version:

npm i eslint-plugin-tailwindcss@3.14.2-beta.1 -D

and give me feedbacks.

bodograumann commented 8 months ago

Yes, now it works for h-full and w-full. :heart: Still to make completely sure, that the replacement does not change the design, I think the following should be done:

I mean it is an edge case, that someone decides to define a size- value which does not conform to the w- and h- values, but you never know. Given that the change is done automatically with --fix, I think we should rather be careful.

francoismassart commented 8 months ago

@bodograumann please take the time to configure this edge case inside a Tailwind Play and share its URL so that I can test it out.

bodograumann commented 8 months ago

Here you go: https://play.tailwindcss.com/XPnP68FJ85

francoismassart commented 8 months ago

@bodograumann => npm i eslint-plugin-tailwindcss@3.14.2-beta.2 -D

francoismassart commented 8 months ago

Fixed in the latest release eslint-plugin-tailwindcss@3.14.2

bodograumann commented 8 months ago

Thanks for all your effort!

nguyenleccss361 commented 7 months ago

Does anyone have this problem: after eslint converts all to size, somehow tailwind does not build into CSS this size- class so my UI is all broken because of lacking width and height. So I have to manually convert all size- back to h- and w-

francoismassart commented 7 months ago

@nguyenleccss361 for now disable the enforce shorthand rule. And if you want me to take a look at it, share a minimal project exposing the issue.