-
Notifications
You must be signed in to change notification settings - Fork 12
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
A new direction perhaps? #66
Comments
Hi, thanks for reaching out! There are a few concerns, but overall I think this new direction could be interesting, as the current DX of loading the classes/variants isn't the greatest. And will cause issues with the new v4
One problem here is we support the case where someone concatenates the string, e.g.: <div class={"text-red-600 " <> if @active, do: "bg-blue-300"} /> But I think we could figure out whether to leave the space dangling or not, as trimming would be nice to have. Some additional thoughts:
If we can address the concatenation issue and the performance difference isn't that much noticeable, then this direction would be welcomed, as it solves the "v4" issue as well as "config extraction" grievances. I also notice some tests commented out in the diff which ideally we can get passing again, depending on the new constraints 👍 edit: I am also waiting on how the new v4 tailwind will be integrated with phoenix -- especially the heroicons plugin. phoenixframework/tailwind#107 What do you think about this new approach, @aptinio? It doesn't seem that too far off from #47, just eliminating the extraction part by running the executable instead |
Instead of using concatenation, I prefer to use the list shorthand which doesn't require leading or trailing spaces.
Yes, Tailwind does sort the classes correctly before outputting them. The few exceptions (.group and .peer) are handled within the
I noticed that the prettier plugin in JS doesn't support variant ordering so I removed this functionality.
That was my first approach but it didn't work out. I couldn't find a way to distinguish between the LSP formatting a single file vs This approach doesn't work with the LSP since Tailwind wouldn't see any new classes since they are not persisted to the filesystem yet (when using format on save). My opinion is that LSP performance is the priority for DX. In my experience the slowdown when using the LSP is generally less than 500ms. It's also important to note this delay only happens when new classes are detected. Most of the time you are re-using the same classes over again so formatting is instant. After I simplified my approach and ran Tailwind for each file, I realised |
Hi @100phlecs !
I have created a Tailwind formatter project forked from your project. At first, I was wanting to add some features, but then I realised I needed to take a difference approach to get the outcome I wanted.
Here is my current version I have been using locally as I've been developing my app (I haven't published this to hex). The code is a little messy but it has been working well for my use-case.
https://github.com/adamroyle/tailwind_formatter/tree/feat/refactor
Features
How it works
When a request for formatting comes in (either through the LSP or mix format) Tailwind runs on just that one file. The resulting css is parsed to determine the class order. The class order is persisted to ETS so subsequent format requests for the same file won't execute Tailwind unless new classes are present.
Caveats
Why am I writing all this?
I think the benefits easily outweigh the negatives and would prefer to contribute to this project rather than create and maintain my own hex package. What do you think?
The text was updated successfully, but these errors were encountered: