-
-
Notifications
You must be signed in to change notification settings - Fork 482
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
☂️ Tailwind class sorting lint rule #1274
Comments
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? |
@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? |
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. |
@ematipico I will do a bit of research and get back to you. |
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 {
padding: 1rem;
}
.pt-6 {
padding-top: 1.5rem;
} So the class order after sorting will match this: 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 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. |
To expand on the pros/cons of my proposed approach: Pros
Cons
|
Yet to begin, because of events in my personal life. |
Thanks for the update. I'm gonna give it a go then :) |
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? |
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. |
@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. |
@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. |
Here's my current thinking. Read my earlier messages on this thread for context. Tailwind CSS class sorting -> class sortingWhile 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. ApproachThe 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...
} VariantsVariants ( PresetsOf 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 configurationAs 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.
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. |
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.
What are the JSON snippets that you shared? I miss some context even though I read the previous messages.
I don't understand the context of this, can you expand a bit please?
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
|
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? |
@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. |
I wonder if it'd be possible to support UnoCSS transformer features like variant groups... |
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 |
@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. |
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. |
add support for rules for un-supported or invalid class of tailwind, inspired by eslint-plugin-tailwind |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Hi, I'm migrating my Astro project to Biome 1.8.3, and I love the simple setup, the only thing remaining for me is Tailwind class sorting. This is my biome.json
This is my settings.json for VSCode
However, when I run I don't see any error in my .astro file that has the invalid example of the documentation Can someone please help me to fix the issue? Also, ideally I would love to sort the classes when saving. |
Biome can't yet parse the html template section of Astro files, so nothing gets formatted/linted in there, including class names. THis will only work once biome has full support for Astro files |
After upgrading to biome 1.9.0 the Quick Fix for sorting the classes stopped working for me. Downgrading back to 1.8.2 worked, and I can quick fix my classes sorting again. |
@MarkLyck update to 1.9.1. That issue was reported already and fixed. |
This comment has been minimized.
This comment has been minimized.
Hi all, Implementing the issue is going slower than we wished, but that's open source, right? We're struggling to find people who have in-depth knowledge of the domain (tailwind) and coding skills (Rust)—of course, and time, too. We want to try a different experiment this time. We are still determining if it will work, but at worst, we won't make any progress. If you have in-depth knowledge of at least one of the missing features you require (check the first comment of the issue), and you have time and will to help the project, we would ask you the following:
Note I increased the task pledge to 1k USD. This is a mastodontic task that requires a lot of work. Also, 70% of the bounty will be distributed to the contributors. Unfortunately, Polar seems buggy and can't update the issue badge, but you can check the link of the page to confirm. Important If you decide to help us, and you have extensive knowledge about a topic, you leave a comment here to notify possible other contributors who want to help with the same topic. We want to try to avoid double issues, so you can collaborate on the same issue. |
Regarding submitting test cases, will the initial goal be to pass prettier-plugin-tailwindcss's own test suite, or is the approach alternative to that? Btw, once I donated more to the issue Polar updated the increased task pledge as well in the card 🥳 |
@silvenon Yes. It's my understanding that the correct ordering of classes is crucial in certain cases, so yes, we should match their testing suite |
@ematipico I'd be happy to do this, fwiw. I had a call a while back with @lutaok where I basically did that, going very deep into the details of every item in the list. I think a format that would work is if I record a video while sharing my screen, so I can explain everything properly. Typing would take me potentially weeks, but I can sit down and do a ~2 hour recording (what our call approximately took) going over everything. Would that help? |
@DaniGuardiola that's also another viable option, feel free to help. However, we still need to have valid and invalid cases for each feature that we must have, so we have some sort of harness. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This rule is great, but it should also trim whitespace. So no spaces at the start of the line, no spaces at the end, and only one space between each classname. |
I'm guessing fixtures for handling whitespace are already included in Note that this issue is for coordinating work on this rule, not reporting problems, read #1274 (comment) for info on how to submit a detailed issue about what this rule is currently not doing, but you believe should. |
@pvinis it already does this, whereas the original prettier plugin respects the original whitespace. This is something that might be worth reverting though, or making optional. I haven't given it that much thought, but there might be a good reason to leave whitespace intact. |
Hey is not clear for me if you are planning to revert or to make it optional the white space functionality? Is it possible to know for when is this planned? I'm doing a refactor to a project and there are a lot of literals that cause error when running biome because the space is removed and then all became a big string 😄 . Thanks in advice I love using this tool |
for what it's worth, I like whitespace being time automatically for me. |
Is it possible to make this whitespace time optionally? I'm trying to avoid using clsx or simillar and is giving me issues because all literals inside the className are being join becuase the white space between them is being removed |
I'm currently integrating Biome into a Symfony (PHP) project. Symfony uses the Twig template engine. We're using Tailwind CSS and I would love to be able to apply the Twig-Templates for HTML usually have the But is there any way to do this currently or is it planned to be able to do that in the future? The Symfony Demo project also uses twig with Tailwind CSS. An example twig-html file can be found at https://github.com/symfony/demo/blob/7169fb500af3be8fdcfb7badb38ac6ed88020882/templates/base.html.twig |
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
clsx
, etc).class
andclassName
)clsx
)md:
,max-md:
(simple config, px-only)!class
)tailwind.config.js
->biome.json
useSortedUtilityClasses
?)Upvote & Fund
The text was updated successfully, but these errors were encountered: