Disabling "smart formatting" of HTML class attribute #10918
Comments
thorn0
added
lang:html
This sounds like a good trade-off to me, but probably it can be improved. I guess developers who don't use Tailwind and have longer class names would prefer to keep the new behavior. So can Prettier probably somehow detect whether a single-line |
just interesting, what is the problem with multi line, I found it more readable |
|
I find the single line approach problematic when having many classes (like in a tailwind app with multiple breakpoints)
|
Nonetheless, I'd say we're almost there because so far looks like the current OOB solution is good enough for everyone else except Tailwind users. So we only need to find a way to improve it for Tailwind. @luukdv Could you please replace the image in your post with a code block, so that it would be easier to experiment with reformatting that code? |
|
Just another brain storm comment, how about grouping all ungrouped classes at the end of the class list, after all the prefixed groups (only if there are prefixed groups)? |
|
Sometimes there are different needs/preferences vs. keep properties for the same purpose (in this case breakpoint) together Perhaps a viable solution is to break it into single chunks if the line is to long, and leave it as is as long as line length is not exceeded. |
@alexander-akait I guess it's a matter of preference then. For me, if I consider the following: [...]
<div class="grid grid-cols-2 sm:grid-cols-3 gap-2 sm:gap-4 md:gap-8 max-w-screen-xl mx-auto px-5 sm:px-10">
<a href="" class="relative bg-gray-200 group block transform border-2 border-white rounded-lg">
<img src="" alt="" class="z-10 object-cover absolute top-0 left-0 w-full h-full block h-auto w-full" />
<div class="absolute z-30 overflow-hidden hidden lg:flex flex-col font-medium justify-center text-white px-8 top-0 left-0 w-full h-full transition-opacity ease-cubic duration-400 opacity-0 lg:group-hover:opacity-100">
<p class="mb-4 last:mb-0 text-gray-800 italic inline-block text-sm px-2 leading-relaxed z-10">
Some text
</p>
</div>
</a>
</div>
[...]I'd see what's going on HTML-wise at a glance. With the new approach, it's less readable, especially since there's even more of these blocks above and below: [...]
<div
class="
grid
grid-cols-2
sm:grid-cols-3
gap-2
sm:gap-4
md:gap-8
max-w-screen-xl
mx-auto
px-5
sm:px-10
"
>
<a
href=""
class="
relative
bg-gray-200
group
block
transform
border-2
border-white
rounded-lg
"
>
<img
src=""
alt=""
class="
z-10
object-cover
absolute
top-0
left-0
w-full
h-full
block
h-auto
w-full
"
/>
<div
class="
absolute
z-30
overflow-hidden
hidden
lg:flex
flex-col
font-medium
justify-center
text-white
px-8
top-0
left-0
w-full
h-full
transition-opacity
ease-cubic
duration-400
opacity-0
lg:group-hover:opacity-100
"
>
<p
class="
mb-4
last:mb-0
text-gray-800
italic
inline-block
text-sm
px-2
leading-relaxed
z-10
"
>
Some text
</p>
</div>
</a>
</div>
[...]
@thorn0 Sure thing: <div
class="
rounded-t
lg:rounded
overflow-hidden
border
border-gray-200
group-hover:border-transparent
group-hover:ring-2
ring-offset-2
ring-secondary-500
object-cover
w-full
h-full
absolute
inset-0
cursor-pointer
z-1
"
/>
<div
class="
bg-secondary-500
text-white
cursor-pointer
hidden
lg:block
lg:invisible
lg:group-hover:visible
rounded
shadow-lg
px-6
py-2
font-medium
absolute
position-center
z-3
"
>
<span>View</span>
</div>
<div
class="
lg:hidden
bg-white
bg-opacity-90
rounded
shadow-lg
px-2
py-1
text-sm
font-medium
absolute
right-[5px]
top-[5px]
"
/> |
|
@thorn0 looks reasonable to have this in one line |
|
Firstly, thanks for improving DX by bringing such brilliant features in prettier! Cheers! Right now, I'm facing issues while saving the code.. ⌘ + S causes the linted file to revert the css classes, have to again run the linter manually to save the styles as per the new prettier "smart formatting" rules. Demo: Screen.Recording.2021-05-22.at.1.27.54.AM.movThere could/should be a way one can turn this feature on or make it work with the eslint-plugin //edit: The issue was caused due to Headwind heybourn/headwind#127, disabling headwind fixes the smart formatting <3 |
I would say the HTML is much easier to read in the first example while the css is much easier to read in the next. I think this a tough problem for many Tailwind users (myself included). Netlify solution was to go multiline css so I don't think all tailwind users agree this "smart formatting" is "bad". Tough problem however. My proposed solution would be what Prettier has already implemented on multi lined objects: https://prettier.io/docs/en/rationale.html#multi-line-objects (i.e. if a newline exists, make multiline, else single line up to the line length). I believe this would keep everyone happy? And best of all no added options :) And if people still have a gripe, once #5919 gets attention, it would be a simple plugin to reverse this behavior. |
|
I, too, am blocked from upgrading to 2.3.0 because of #7865. We use HTL (formerly Sightly), a templating language based on HTML. Unfortunately we use quite a few expressions in the <div class="cmp-accordion__button${item.name in accordion.expandedItems || accordion.extendedItems.size == 1 ? ' cmp-accordion__button--expanded' : ''}"></div>which prettier 2.3 will reformat as <div
class="
cmp-accordion__button${item.name
in
accordion.expandedItems
||
accordion.extendedItems.size
==
1
?
'
cmp-accordion__button--expanded'
:
''}
"
></div> |
|
Just wanted to share my two cents from the Tailwind team — I think there are definitely interesting things that could be done formatting-wise but I worry it's going to be hard to do within Prettier itself because I think the best solutions require more semantic knowledge of the classes being used. I can say my personal preference would be that Prettier did nothing with the class attribute, but if it's going to do anything I would really prefer if it doesn't do anything to the order of the classes. Lots of folks in the community use tools like Headwind to sort their classes for them in a Tailwind-aware way, and we really want to build functionality like this into our own editor extension as well in the near future. Personally I've always been very satisfied with just enabling word-wrap in my editor, but totally get that others like to have things split up. Is this a big issue for people outside Tailwind projects? If not, maybe our team could prioritize working on a Tailwind plugin for Prettier and make that the solution vs. forcing Prettier to care too much about Tailwind's naming conventions and stuff? If there are any questions I can answer or input I can provide that would be helpful please let me know! Absolutely love Prettier and really appreciate the work of everyone that contributes to this project. Especially grateful that people are taking it upon themselves to try and come up with solutions to stuff like this long class attribute problem and that people are taking Tailwind into consideration when working on it ❤️ |
|
Hi! Adding a comment regarding the impact of this prettier feature inside of Vue 2 (using laravel and laravel mix, with Vue is processing everything inside of class as plain text and printing it like that straight to production, making our classes occupy much more precious bytes than they should: {staticClass:"\n flex\n items-center\n font-bold\n uppercase\n text-h6\n focus:outline-none\n "} // Yep, that's 10 individual space chars per class :(This happens regardless of the configuration of whitespace preservation in mix.vue({
options: {
compilerOptions: {
whitespace: "condense", // still doesn't condense classes :(
},
},
});Therefore, having the option to disable this feature would solve cases where compilers interpret anything as plain text. Thank you :) |
|
Thanks for the feedback. For now, the plan to improve this situation is to apply the new formatting only if:
How does the plan above sound to you? The idea is to keep this new behavior at least for some developers that don't use Tailwind. I noticed that Tailwind doesn't use
Prettier never reorders anything. It's one of the project's principles.
The answer seemed to be no, but turns out there is also an issue with Vue (not sure how big it is, see the previous comment).
Unfortunately, Prettier currently lacks an API for such a plugin. We have an open issue about that: #10803 If your team could help us with moving that issue forward, it'd be awesome. I know there already exist multiple Tailwind plugins for Prettier, but they use hacks instead of the official API and thus are fragile. E.g., such plugins conflict with each other. We need a real solution for this. |
thorn0
added
type:enhancement
|
@thorn0 Any fix is welcome :D ! I'm not familiar with how prettier works under the hood, so I have to ask: is that approach easier than adding a config option for this feature? Or is it easier to validate it on a case by case relying on the parser and syntax? I ask this because the issue could be affecting other users with other stacks, for example this comment about htl on the tread from @sabberworm . Thank you for your response :) |
|
@thorn0 Thanks for the update!
What do you think about simplifying this and only applying the formatting if the class attribute already contains a line break? To me that feels a lot simpler than having a very specific rule about double dashes or double underscores, and makes it very easy for people to opt-in to the behavior if they prefer it, and would be very learnable/easy-to-understand what's triggering that formatting without having to read any documentation or anything to learn about the special |
It is not heuristically, and kills the whole point of formatter |
|
That is what was recommended by @thorn0 though, no?
If their suggestion is against the philosophy of Prettier that is okay but it was not my suggestion, I was only suggesting to not base it on the presence of |
|
Maybe #10803 is real good solutions without any hacks... |
The idea is to make it "just work" for everybody without a need to read anything. My hypothesis is that people with these long class names with
Not adding options is an important principle of Prettier. See https://prettier.io/docs/en/option-philosophy.html |
I agree with Adam regarding only considering the linebreak as that should be a good enough indicator of user intent. I personally don't want prettier to mess with my class strings at all. If I want to see everything I'll toggle wrapping in my editor, if you're already dealing with monster html horizontal scrolling is oft inevitable. Thanks for your hard work on this project and hope I don't come off as entitled or rude, cheers. |
|
@iKlsR One of Prettier's principles is to leave as little as possible up to the user and another one is formatting reversibility. If we can't fully stick to them in this case, we should at least try to minimize the digression. If we can prevent at least some nitpicky PR reviews like "please add a line break here for Prettier to reformat these classes", we should do this. BTW, do you use Tailwind? Do you use |
I'm afraid this is not a good idea, we must not rely on user input, we should rely on string length, as you can see some examples are very long and it is not readable... |
|
@alexander-akait Hey Alexander!
This is how Prettier behaved prior to this recent change though, it never added newlines into classes for you. It did this: <!-- Input -->
<div class="mt-8 w-full bg-indigo-600 border border-transparent rounded-md py-3 px-8 flex items-center justify-center text-base font-medium text-white hover:bg-indigo-700 focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-indigo-500">
Hello world
</div>
<!-- Output -->
<div
class="mt-8 w-full bg-indigo-600 border border-transparent rounded-md py-3 px-8 flex items-center justify-center text-base font-medium text-white hover:bg-indigo-700 focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-indigo-500"
>
Hello world
</div>...so the class list stayed as one long string (which I think is good), but it moved the actual attribute to its own line (which I personally don't think is useful). The major complaint here (and the reason this issue was opened) is that generally people (myself included) seem to prefer the really long class names staying on one line, because breaking it up onto multiple lines makes templates very difficult to work with because every HTML element can take up like 12 lines in your editor. The comment here about "styles are collapsed by default" is relevant: For our HTML stuff at Tailwind Labs we are pinned to Prettier 2.2 and actually set |
If user input is not a desired behaviour, you could instead break the lines based on print width, so instead of having 20 lines it would be like 4, that is much more manageable, unless there is already a break so we know that the user wants the behaviour of breaking those lines, falling down to @adamwathan ideas. |
|
@adamwathan So the main problem is a lot of lines inside the |
|
@alexander-akait Yeah correct, I think the community finds the code on the right much more manageable than the code on the left:
|
|
@adamwathan Yep, sounds reasonable @thorn0 @sosukesuzuki @fisker Thoughts? |
I agree and it's worth considering most code editors have "word wrap" feature, so editing a long line of class attributes isn't too bothersome. Interestingly ; I never even paid attention before ; but both VSCode and Sublime Text even re-indent after wordwrap even through it is a single line:
There are several updates if that helps related to the change of behaviour:
|
|
prettier's philosophy is not about respecting deliberately set newlines, this is too much mental drain. just let prettier do the change without being directed by the users or some newlines within the class attribute. I want opinionated defaults. to the comparisons between one class per line vs all in one line, this comparisons are jarring, there is still some middle ground between these options. I think the maintainer should decide on its own. prettier is for a reason popular, because this guy made smart design choices in the past. |
you should compare to wrapped at print width not all in one line, then people would vote differently |
|
Why not just sort the classes and wrap them at I think these examples would fit well within 2 lines.
|
I don’t think sorting them in any automated, non-framework-specific way makes sense, especially when using atomic CSS like Tailwind. There you would much rather group related classes like |
|
Guys, guys, let's make small steps: Talking about reordering and word-wrapping is too much at once. These are two different features with entirely different requirements. I'd suggest to find an agreement, first, on the word-wrapping and once we are there we can think about reordering (if at all since reordering is something not really in the realm of prettier), ok? For the sake of simplification, we have currently two parties in this thread: 🎄 The Xmas-Tree Hardliners (one class per line)
🥞 The Pancake Party (just wrap at print width and intend next line accordingly)
Did I miss any party? If not, let's start to find an agreement just on this topic before we move on to reordering, ok? We could now vote somehow but since everybody will start to give reasons, mark you reply with the respective emoji ONCE, so we can quickly see and search the 'votes'/emojis, if you think we need more option, add them with a new emoji!! @thorn0 I think you got fed up with this thread, but let's at least try to get closer to some solution. The current state of matters is really hard to work with. Thanks! [1] i find this pro really negligible, I never really read git diffs of some css classes, just to start the discussion already |
|
The third option is (as a first simple step at least) to revert and go back to leaving class strings alone, like @fisker suggested. |
|
It's been reverted 👍🏻 just need to wait for a new release. |
|
Cf #11827 |
|
Thanks for the update. Definitely more usable. I would still like to see a way to leave intentional newli es (in a html attribute). |
|
It has been released: https://github.com/prettier/prettier/releases/tag/2.5.0 |
|
Let's close, feel free to feedback |
IDK, the latest release is a good baseline which works for all us, nobody will die and it's better than the Xmas tree. But are we there yet? Closing the issue would be feel a bit like giving up. I mean the future of CSS will be for a significant part about utility CSS. And maybe the solution is that the Tailwind team or new teams need to come up with a more terse API which fits most of the time into one line (Tachyons was better with this btw). Tailwind is in some areas terse but in many parts not much shorter than the og style equivalent. Or, the Tailwind team just forks prettier and focuses more on IDE tooling than some ui toolkits. Former is way more crucial than another set of random ui components. /cc @adamwathan Or, there is a solution in better formatting which works for all of us, can be implemented by the og prettier team, without fragmentation of the community. I don't want to get again into the topic and repeat my view like a broken record. We need instead another take how to discuss with a proper moderation. Maybe we start with listing requirements of everyone before we sketch another solution. Some love git diffs, some have huge code blocks within, some need ordering but not alphabetically and so on. Utility CSS is too important that we just do nothing or play laissez faire. The current solution is ok-ish but far from what I would expect from prettier (It.Just.Works™). |
I don't think anyone's giving up; you can find the follow-up discussion in #7863 as mentioned in the 2.5.0 release notes. |
You seem passionate about it, you could fork it also? Those UI components pay for the many full time devs on team tailwind and bring me more value that re ordering css classes. "Crucial" is subjective |
|
I was very upset by this change in version 2.5.0. It is much more convenient for me to work with classes when they are arranged in a column, and not in one row. Maybe you can add a setting for this instead of changing the default behavior? It would be very convenient and would allow everyone to work as it is convenient for him. |
|
Continuing the discussion here since the issue referenced in the release notes is still locked.
I agree that this would be a really elegant solution that bridges the gap between the two extremes of one giant line and one line per class, neither of which are appealing to me as a Tailwind user. |
github-actions
bot
added
the
locked-due-to-inactivity
and others
"We get a lot of requests for adding options, but Prettier is built on the principle of being opinionated about code formatting."
I totally understand this, and so I apologize beforehand for opening this issue. The reason I think this is a valid issue is because the opinion about this seems to have changed from the perspective of Prettier, even though it was mentioned that there's no community interest for this change (and a lot of pushback) in the introducing pull request.
The issue
When using frameworks like Tailwind, readability has lowered drastically since this feature:
References
I'd love to still be able to use Prettier, but this is a blocking issue for me and many others.
The solution
Respecting the
classattribute to stay on a single line, and only applying formatting it when a user already has a multi-line approach forclass. Alternatively, an option to disable this feature.Thanks in advance for considering this!
The text was updated successfully, but these errors were encountered: