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] size is not always equivalent to h- and w- #315

Closed
bodograumann opened this issue Jan 30, 2024 · 13 comments
Closed

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

bodograumann opened this issue Jan 30, 2024 · 13 comments
Labels
bug Something isn't working feedback needed good first issue Good for newcomers work in progress

Comments

@bodograumann
Copy link

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):

  • OS: Arch Linux
  • Softwares + version used:
    • eslint-plugin-tailwindcss@3.14.1
    • tailwindcss@3.4.1
@bodograumann bodograumann added the bug Something isn't working label Jan 30, 2024
@francoismassart
Copy link
Owner

@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 francoismassart added the good first issue Good for newcomers label Feb 1, 2024
@francoismassart
Copy link
Owner

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

@bodograumann
Copy link
Author

Thanks for looking at this so quickly, @francoismassart .
I can confirm that the change in 228f3b7 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
Copy link
Owner

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
Copy link
Owner

@bodograumann Please try this beta version:

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

and give me feedbacks.

@bodograumann
Copy link
Author

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

  • Lookup w- in the width scale.
  • Lookup h- in the heigth-scale.
  • Lookup size- in the size-scale.
  • Check that all three have identical values.

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
Copy link
Owner

@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
Copy link
Author

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

@francoismassart
Copy link
Owner

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

@francoismassart
Copy link
Owner

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

@bodograumann
Copy link
Author

Thanks for all your effort!

@nguyenleccss361
Copy link

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
Copy link
Owner

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feedback needed good first issue Good for newcomers work in progress
Projects
None yet
Development

No branches or pull requests

3 participants