search

biomejs / biome

A toolchain for web projects, aimed to provide functionalities to maintain them. Biome offers formatter and linter, usable via CLI and LSP.
https://biomejs.dev
Apache License 2.0
13.19k stars 406 forks source link

📎 Tailwind class sorting lint rule #1274

Open DaniGuardiola opened 7 months ago

DaniGuardiola commented 7 months ago

Update: I'm working on this! Follow the updates in this Twitter thread: https://twitter.com/daniguardio_la/status/1739715412131238122

PRs

Description

Port the TailwindCSS Prettier plugin that sorts Tailwind CSS utility classes. In Biome, it is being implemented as a lint rule that formats classes in JavaScript files though auto-fixes (ref).

Feature support / tasks checklist

Features with an asterisk are not likely to be implemented as part of this task. They are listed for exhaustiveness, and considered in architectural decisions so that future support is possible, but they are not high priority.

Features with a question mark are ideas that I'm not sure about, or I don't know enough about to determine if they make sense or how they could be implemented.

Upvote & Fund

Fund with Polar

ematipico commented 7 months ago

Hey @DaniGuardiola , thank you for opening this.

If you have the knowledge, do you know how this sorting thing works? Does Biome need to read the tailwind configuration file?

DaniGuardiola commented 7 months ago

@ematipico yeah I'm like 99% sure the config file is read. JavaScript is involved. Not sure how it fits in the biome way of doing things.

Is running JavaScript a possibility, at least to load the config file and transfer it to the rust program (or however that'd work)? or is it considered too slow?

ematipico commented 7 months ago

What I am interested in is the business logic of the plugin and how the configuration is involved in the sorting to the CSS classes.

I don't know Tailwind that well (as well as other people), so if someone with more knowledge can shed some light, it would help us to implement the feature in a faster way.

DaniGuardiola commented 7 months ago

@ematipico I will do a bit of research and get back to you.

DaniGuardiola commented 7 months ago

Leaving these as notes for the future.

This is where it all ultimately happens: https://github.com/tailwindlabs/tailwindcss/blob/master/src/lib/setupContextUtils.js#L930-L969

This function defers to this other function: https://github.com/tailwindlabs/tailwindcss/blob/master/src/lib/generateRules.js#L885-L930

The way sorting works is by leveraging the exact same mechanism that tailwind uses to sort the outputs. For example, p-4 pt-6 and pt-6 p-4 result in the same css, something like this:

.p-4 {
  padding: 1rem;
}

.pt-6 {
  padding-top: 1.5rem;
}

So the class order after sorting will match this: p-4 pt-6.

This is explained here: https://tailwindcss.com/blog/automatic-class-sorting-with-prettier#how-classes-are-sorted

Regarding configuration. First let me disambiguate to avoid confusion. There are two separate concepts here:

Sorting options are trivial, and just act as input for the formatting tool. What I'm talking about here is the latter, the involvement of the tailwind config in this process.

First, about this config: it is a JavaScript file that needs to run and be "resolved" (e.g. plugins are functions that need to call stuff like addUtility and such to register stuff for Tailwind to use. So, the final config is created by "resolving", a.k.a. executing a bunch of stuff like that.

Its role in sorting spans multiple things:

The prettier plugin seems to "hook" into all of the existing logic, essentially reusing the exact same mechanism that is used to order the output. This makes sense, because why wouldn't they?

But I would argue that such complexity is not necessary, and biome could go with a way simpler approach based on simple heuristics and basic parsing/regexp.

I've done custom "reimplementations" of tailwind-like parsing for two projects now:

So I would propose an approach like this + extensive testing. Of course, only using actual tailwind can guarantee that there won't be any false positives, but I think it is a very good compromise.

Regarding tailwind config, plugins and such, I would suggest that those options are moved to the formatter options. I think this biome feature should be generalize as a "utility class sorting" thing, with a tailwind preset.

So, in line with that, it should be simple enough to just add the utilities and variants that a plugin might add through config.

Sorry if this got a bit long / chaotic, I'm just researching and brainstorming to get things rolling. I'd be happy to start playing with this in a PR.

DaniGuardiola commented 7 months ago

To expand on the pros/cons of my proposed approach:

Pros

Cons

estubmo commented 7 months ago

Yet to begin, because of events in my personal life.

DaniGuardiola commented 7 months ago

Thanks for the update. I'm gonna give it a go then :)

Reed-Anderson commented 7 months ago

Hey @DaniGuardiola , thanks so much for taking this on, it will be a huge upgrade for me. I hope this is an appropriate place to ask, is there any interest in supporting this popular tailwind/prettier request that was never implemented?

https://github.com/tailwindlabs/tailwindcss/discussions/7763

itsezc commented 7 months ago

Cons

  • Can't use tailwindcss config directly. The user would need to re-specify these settings for the biome feature. Honestly, I think options like prefix are largely unused so I don't think it's a big deal. Regarding plugin-provided utilities and variants, the simplest solution is to provide a way to define these in the config as well. All in all, not the worst thing imo.
  • False positives. Tailwind can tell the difference between something like hover:px-4 and hover:px-8268. The first one is valid, and the second is not. This is because 4 is in the theme scale, and 8268 is not. The proposed approach would count both as valid, and sort them both. This is the 0.2% I was talking about. Anyone using hover:px-8268 as a manually defined class should consider changing careers anyway. It's a non-issue.

First issue, can cause some friction for DX, and repeating configs is not ideal especially when there may be several devs in a team / project with an evolving tailwind config, but you're right its not the worst thing either.

Second, is a non issue as you've said. But, many people use arbitrary values i.e. hover:px-[8rem] or hover:px-extra if they're using a custom setup theme within their tailwind config. As long as these scenarios are handled to how the existing prettier plugin does, I don't think that'll be a huge issue.

DaniGuardiola commented 7 months ago

@Reed-Anderson thanks for bringing this up. While I share your pain, and think some tooling could definitely help, I think that's out of scope for this task.

I did see some approaches I liked in that thread, like one that turned multi-line strings into a normal, single line string on build (iirc). I don't think this task is the place to try something like that though.

DaniGuardiola commented 7 months ago

@itsezc good call-out. While I'm not going for perfection, I do wanna consider all of these cases, and I have some ideas to solve this that I'll share soon here for public feedback.

I'm a heavy tailwind user myself so I definitely want something that works across all reasonable use-cases.

DaniGuardiola commented 7 months ago

Here's my current thinking. Read my earlier messages on this thread for context.

Tailwind CSS class sorting -> class sorting

While the main goal of this task is to provide a good replacement for the Prettier Tailwind CSS plugin, generalizing just makes sense to me, because other tools use the concept of utility classes as well. It also just seems like something that could be generally helpful, the ability to sort classes according to a configuration.

Approach

The main challenge to solve is how the order of the classes is decided. Tailwind relies on its internal logic that also decides how the output is ordered, but we can't do that here, so we need a different approach. My thinking so far has been along the lines of having a config like this:

// simplified example, obviously
{
  "class-patterns": [
      // some kind of pseudo-regex / templating way of specifying these
     "p{y|x}-${scale}", // "special" value types like scale, numeric, etc
     "p-${scale}",
     "m{y|x}-${scale}",
     "m-${scale}",
     "grow$", // match "grow" without a suffix first ("exact")
     "grow-${numeric}" // then match other with a suffix
  ]
}

"Special" value types could be extended/customized:

{
  "types": {
    // add values
    "scale": { "extend": ["extra"] },
    // replace values
    "screens": { "values": ["md", "lg"] }
  }
}

The sorting algorithm would use this config to decide the order. Some other configs can be specified as well:

{
  "prefix": "tw-",
  // etc...
}

Variants

Variants (hover:active:focus:<utility class>) also need to be taken into consideration, and they are ordered as well iirc. The order could be specified similarly. The separator (colon by default) could also be configurable.

Presets

Of course, having and maintaining all of this in a config JSON file is annoying, so Biome would ship with presets for Tailwind and any other libraries that are popular enough.

{
  "preset": "tailwind-css",
}

Manual configuration

As I described in a previous comment, a drawback of this approach is that there's a bunch of manual configuration to maintain, which has to be kept in sync with the Tailwind config. To alleviate this, it'd be doable to create a package that automatically reads the Tailwind config and syncs the Biome config to it. Doesn't necessarily need to be part of Biome itself, but it can be a simple npm package. E.g.

$ npm i -D sync-tailwind-biome
$ sync-tailwind-biome

I think this should make that pain much more manageable.

These are my thoughts so far. On the development side of things, my focus is going to be creating a minimal POC that works for a default tailwind config. This is the first time using Rust so it's enough to keep me busy for a while :P

Feedback is super welcome though, I'd like to polish these ideas so that I can be confident when it's time to implement the advanced features.

ematipico commented 7 months ago

Tailwind CSS class sorting -> class sorting

Unfortunately I can't venture an opinion because I don't know much of the logic we are about to implement. Surely, I am a big fan of consistency, so it's definitely a good start.

Approach

What are the JSON snippets that you shared? I miss some context even though I read the previous messages.

Variants

I don't understand the context of this, can you expand a bit please?

Manual configuration

I would suggest a different approach. I like having a separated package that does the work, and I think we can also have it under our organization, maintained by some volunteers.

The package should return a JSON file via stdout, and then we can use this package in our command biome migrate The command can read the result of this package via stdout, and use it to update the configuration file. Here's why:

tcoopman commented 7 months ago
  • Honestly, I think options like prefix are largely unused so I don't think it's a big deal.

This comment is not completely clear to me. Does this mean that you won't provide support for prefixes or that they'll have to be specified in the biome config?

DaniGuardiola commented 7 months ago

@tcoopman the latter. In any case, the plan is to provide an automated way to update the config, and I'd expect that to be the most popular way to use this, so it's even less of a big deal.

XiNiHa commented 6 months ago

I wonder if it'd be possible to support UnoCSS transformer features like variant groups...

johnpyp commented 6 months ago

It might also be useful to look at https://github.com/avencera/rustywind, as this is a pure rust implementation of tailwind sorting which also doesn't look at the tailwind.config.js file afaict

an interesting approach it takes is optionally configuring the sort order based on the output css file of tailwind, not sure if that would really be applicable here though

DaniGuardiola commented 6 months ago

@XiNiHa it's possible, but it's a bit out of scope for now. Maybe in a future iteration.

@johnpyp thanks for the suggestion. I'm already very familiar with how Tailwind CSS sorting works, and an initial version is about to merge with only a couple of details missing, so there's no value in researching other projects anymore at this stage. Fwiw, I did take a look around that project but ended up using Tailwind CSS itself as the source of truth, which I believe is the best way to ensure correctness.

GustavoBonfimS commented 5 months ago

Could this rule be a safe action? Today we can only organize classes using --apply-unsafe. If it were a safe rule it could be automatically organized with the VS Code extension when the file is saved.

1714

jsonMartin commented 5 months ago

Could this rule be a safe action? Today we can only organize classes using --apply-unsafe. If it were a safe rule it could be automatically organized with the VS Code extension when the file is saved.

1714

+1, I also want to format classes on save in VSCode or Neovim

DaniGuardiola commented 5 months ago

This rule is a work in progress and there's still a lot of work to do before we can consider it stable. I like your linked suggestion about opting into unsafe fixes, and I understand that you want to use the rule with a nicer workflow, but please be patient.

ematipico commented 5 months ago

@jsonMartin @GustavoBonfimS

Thank you for the engagement, but these kinds of comments don't bring any value to the conversation, they are out of scope. We have a plan and want to stick to it.

You could help us by reporting possible bugs and improvements, or by implementing the missing pieces.

If you're not interested, that's also fine. We want to keep the conversation in this issue as technical as possible, about the logic and missing pieces.

Thank you for your patience and understanding

BarakChamo commented 5 months ago

@DaniGuardiola very exciting for this feature, sorting TW classes automatically is a big part of a predictable UI collaboration flow for us and many teams using TW.

Wanted to chime in to the discussion and mention that it appears the TW team is considering arbitrary value support for different utility classes without a square bracket format in TW 4. (i.e. there won't be a predefined scale of values but both p-4 and p-1000 would just work).

This would be in line with your suggested implementation and offer potential future proofing for dynamic values, while still auto-sorting current classes like, for example, p-865 in current tailwind, which is not part of the theme. Seems like an outlier issue though.

hilja commented 4 months ago

I’m sure this falls under the undone tasks, but I had a little Kafkaesque experience trying to get the sorting works for this abomination. Biome stuck the classes together, the space seems to break the sorting:

Screenshot 2024-03-09 at 16 16 26

If I used a + then the template literal rule kicks in :)

Screenshot 2024-03-09 at 16 33 28

The space again:

Screenshot 2024-03-09 at 16 20 30

This works though:

Screenshot 2024-03-09 at 16 29 57

Anyway this is a bad way of handling classes because now Tailwind won't know that color.bg.medium should be included in the final css. In general the sorting is a wonderful!

sourcec0de commented 4 months ago

Might be useful to this thread. Looks like Tailwind 4.0 built its new oxide engine on Rust. Which itself uses lightning.

https://youtu.be/CLkxRnRQtDE?t=2149 https://tailwindcss.com/blog/tailwindcss-v4-alpha https://github.com/tailwindlabs/tailwindcss/tree/master/oxide

ghost commented 4 months ago

Could Biome support class sorting for other languages like Vue or Svelte, instead of just JSX? Currently it doesn't seem to work, but it seems like it should be an easy fix since it's just class instead of className.

DaniGuardiola commented 4 months ago

@sourcec0de thanks for the suggestion. The rust part of tailwind v4 deals primarily with 1) parsing and 2) generation of CSS + the new CSS based config.

1 is redundant since Biome has its own parser which we use, and the tailwind specific lexer is already in place and works well.

2 is not necessary for the most part since we don't have any need to generate CSS. For the CSS based config, it might be useful. I'm waiting for things to stabilize in that area before doing anything about it, and I'm already in touch with the tailwind team so rest assured that if there's a chance to share any logic, it'll happen.

Most of the logic that is relevant for sorting is still JavaScript, so we need to reimplements and adapt it into Rust, as we've already done for some of it.

DaniGuardiola commented 4 months ago

@aarvinr class is already supported, and you can add any extra attributes/props through the options.

What is not supported is the Vue and Svelte languages as a whole. Once those are supported, we will make sure that this rule works with them.

MH4GF commented 4 months ago

Thanks for working on this issue 😄 Please allow me to report the bug. Starting with v1.6.0, it appears that whitespace with placeholders in template literals will be removed. 

./apps/web/app/_components/Time.tsx:7:35 lint/nursery/useSortedClasses  FIXABLE  ━━━━━━━━━━━━━━━━━━━

  × These CSS classes should be sorted.

    5 │ export const Time = ({ children, className, ...props }: PropsWithChildren<Props>) => {
    6 │   return (
  > 7 │     <time className={`${className} text-sm text-zinc-500`} {...props}>
      │                                   ^^^^^^^^^^^^^^^^^^^^^^
    8 │       {children}
    9 │     </time>

  i Unsafe fix: Sort the classes.

    7 │ ····<time·className={`${className}·text-sm·text-zinc-500`}·{...props}>
      │                                   -

If this space is removed, the correct class name cannot be retained. This problem did not occur in 1.5.3-nightly.24fcf19.

Attached is the CI job log for my project. I hope it will be helpful! ref: https://github.com/MH4GF/mysite/actions/runs/8248800629/job/22559888722?pr=201

ematipico commented 4 months ago

Tagged templates and object properties (#2215) are now supported

ghost commented 4 months ago

@DaniGuardiola In my environment, if I apply this rule and try to combine class names with a half-width space using join(), the half-width space will be deleted, so I would like to report this just in case.

スクリーンショット 2024-03-31 12 40 35
DaniGuardiola commented 4 months ago

@reowl666 thank you for the report. Seems like we should just ignore any strings that don't contain any text, or that don't include at least one tailwind class.

hangaoke1 commented 2 months ago

Could it detect the jsxQuoteStyle configuration ? 

"formatter": {
    "quoteStyle": "single",
    "jsxQuoteStyle": "double",
}

image

ematipico commented 2 months ago

@hangaoke1 I think that's done already, maybe not for JSX though

ematipico commented 2 months ago

I removed @DaniGuardiola from the assignees because he doesn't have time anymore. If anyone wants to step up and help take this to the finish line, that would be great.

You can help us with the logic, and we can help with the Rust code.

DaniGuardiola commented 2 months ago

Happy to guide wherever possible. Also, no promises, but I'm hoping I'll be able to contribute to this further at some point.

lutaok commented 2 months ago

Hi, I would like to take this on!

Mardoxx commented 2 months ago

Template literals are still broken same as this

e.g. className={`relative flex-grow ${isVisible? "visible" : "hidden"}`}

will fail and correct to className={`relative flex-grow${isVisible? "visible" : "hidden"}`} which is wrong.

Can't use twin.macro because remix

ematipico commented 2 months ago

@Mardoxx the fix will be released in the next release. If you can't wait, you can try the latest nightly. If the bug persists, please open a new issue, as comments here should be about developments and tend to be less visible unless someone is subscribed.

afonsojramos commented 1 month ago

Would be probably good to also add to biome all of the rules that https://github.com/francoismassart/eslint-plugin-tailwindcss supports, such as: enforces-negative-arbitrary-values: make sure to use negative arbitrary values classname without the negative classname e.g. -top-[5px] should become top-[-5px] enforces-shorthand: merge multiple classnames into shorthand if possible e.g. mx-5 my-5 should become m-5 no-arbitrary-value: forbid using arbitrary values in classnames (turned off by default) no-custom-classname: only allow classnames from Tailwind CSS and the values from the whitelist option no-contradicting-classname: e.g. avoid p-2 p-3, different Tailwind CSS classnames (pt-2 & pt-3) but targeting the same property several times for the same variant. no-unnecessary-arbitrary-value: e.g. replacing m-[1.25rem] by its configuration based classname m-5

Which I would say are rather important!

MrOxMasTer commented 1 month ago

I am the only one with such a problem that he complains about the wrong order of classes (although in my opinion they were sorted by eslint before) and at the same time he does not format not through vs code, not through the cli

  ! These CSS classes should be sorted.

  > 21 │             className="w-full mt-6"
       │                       ^^^^^^^^^^^^^
    23 │           >

  i Unsafe fix: Sort the classes.

    21    │ - ············className="w-full·mt-6"
       21 │ + ············className='mt-6·w-full'
    23 23 │             >

Checked 1 file in 4ms. No fixes applied.
Found 1 warning.

image

pnpm biome format src\...\component.tsx
Checked 1 file in 2ms. No fixes applied.
Gomah commented 1 month ago 
pnpm biome format src\...\component.tsx
Checked 1 file in 2ms. No fixes applied.

@MrOxMasTer You're running the formatter, not the linter, here's what you're looking for:

biome lint . --write --unsafe

Or if you want to run the formatter, linter and import sorting:

biome check . --write --unsafe
DaniGuardiola commented 1 month ago

Update: variant sorting (with some caveats) will be supported in the next version of Biome, thanks to the excellent work by @lutaok :)

MrOxMasTer commented 1 month ago 
biome check . --write --unsafe

unsafe really worked. How can I set unsafe in the extension itself for formatting? image

DaniGuardiola commented 1 month ago

@MrOxMasTer it's not possible. Related: https://github.com/biomejs/biome/issues/1274#issuecomment-1937834636

Please keep the discussion here on topic, for bug reports and specific discussion about the feature. Feel free to use Discord or GitHub Discussions for questions and discussion of more general topics.

ematipico commented 1 month ago 
biome check . --write --unsafe

unsafe really worked. How can I set unsafe in the extension itself for formatting? image

@MrOxMasTer

Please, search the website! We have everything in there: https://biomejs.dev/linter/#configure-the-rule-fix

@DaniGuardiola this is now possible since v1.8.0

hangaoke1 commented 1 month ago

Whether it is possible to be compatible with Spaces image

MrOxMasTer commented 1 month ago

Please, search the website! We have everything in there: https://biomejs.dev/linter/#configure-the-rule-fix

If you are talking about the "fix" setting, which takes the values "safe" or "unsafe" for each rule, then I can say that I used it and it did not work for me

ematipico commented 1 month ago

@MrOxMasTer I advise you to open a new issue with a reproduction, this issue is for developments only and we don't want to ping subscribers with messages that are unrelated