100phlecs / tailwind_formatter

Sorts tailwind classes within elixir projects
https://hexdocs.pm/tailwind_formatter
MIT License
111 stars 11 forks source link

Class interpolation breaks sorting #14

Closed darioghilardi closed 1 year ago

darioghilardi commented 1 year ago

Hi, thanks a lot for this, awesome work!

I found an issue with class sorting when interpolating a variable:

<div class={"lg:grid-cols-#{@cols}"}>

is formatted to

<div class={"#{@cols} lg:grid-cols-"}>

I haven't looked into the code yet to see where it breaks but I will hopefully be able to do that in the next days.

100phlecs commented 1 year ago

I've never considered this usecase before. Interesting.

Out of curiosity, how do you make sure Tailwind doesn't purge the col length that you need? When you interpolate values and it isn't explicitly written out in the source code, Tailwind may leave it out. i.e. if @col = 5 and there's no grid-cols-5 anywhere else in the code, then it'll be purged when you deploy to prod.

Other than that, if this is a desired use case then I think there's a solution for it (just need to look for a - before the #{}

darioghilardi commented 1 year ago

I keep the valid alternatives listed into the Tailwind safelist key of the configuration. In that case, with Tailwind JIT, the class is still generated.

My example above is a little weird, it's definitely not that likely to use interpolation with arbitrary numeric values as it doesn't make sense to list all of them, it's more for cases like the following where you switch between a set of predefined values:

<div class="alert alert-#{type}">

Now I understand that if you write this:

<div class="alert #{if type == :success, do: "alert-success"}">

or call a function you get the same outcome without relying on the safelist, but it's less straightforward and harder to read (counterargument: it's also easy to forget about adding classes to the safelist 😄).

Anyway if it's not too complex I would avoid interpolation to be broken by the formatter, but I also understand if you decide to not support this feature.

Thanks a lot for your reply!

100phlecs commented 1 year ago

Thank you for the report! Please let me know if you run into any other issues, or reopen this issue if you find it not working. I've the example you've provided working, but there may be edge cases I'm not accounting for. Thanks!

darioghilardi commented 1 year ago

This was crazy fast âš¡, thank you! I will update the formatter next week and run it against the whole codebase, I'll ping you here if I find something else. Thanks again!