Skip to content

[DOC]: Illegal use of dynamic Tailwind utility classes in docs #4129

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

Open
hesreallyhim opened this issue Apr 2, 2025 · 2 comments · May be fixed by #4307
Open

[DOC]: Illegal use of dynamic Tailwind utility classes in docs #4129

hesreallyhim opened this issue Apr 2, 2025 · 2 comments · May be fixed by #4307
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers

Comments

@hesreallyhim
Copy link
Contributor

Description

The docs use Tailwind utility classes for styling a lot ("h-5" (height), "pr-1" (padding-right), etc.), which is really convenient and nice. The problem is, because Tailwind produces optimized production code by only bundling the classes that are consumed in the components, which it determines based on static analysis, writing class-names that are composed from run-time variables, like React component props, is prone to failure. For instance, bg-${props.item.color} may not work, because Tailwind will not know what bg-<color> classes it needs to ship with the code, and so it turns out the class .bg-pink has no styles declared. Sometimes you can get away with it, because somewhere else in the codebase, bg-pink is used explicitly, and therefore gets bundled with the code, and so if your component gets .bg-pink at runtime, it will luckily find the classname and get the correct style.

For the most part, this is probably a pretty insignificant anti-pattern, but the reason I'm bringing it up is because of the Table of Contents in the Docs - I noticed that they don't do recursive indentation of sub-classes/methods/etc. For instance:

Image

The headers on the page are H1, H2, H2, and then H3, H3, H3. But as you can see, the H3's (Pull Requests, CIPs, and Discord) are de-indented, whereas a natural assumption would be that each sub-header gets indented more. Indeed, that appears to be the intention of the code, because the classes for the corresponding names in the ToC include .pl-1, (padding-left-1), .pl-2, and .pl-3.

The problem is these padding utility classes are defined with template strings based off props, where each item is dynamically assigned a "level" property through some code, and then the class is something like pl-${level}. Because, coincidentally, .pl-1 and .pl-2 are used elsewhere in the codebase in a static fashion, their styles are included in the production bundle - but .pl-3 isn't. So it does nothing. This is why the ### H3's have no padding/indentation.

For reference:

toc.map((item) => (
<div key={item.id} className={mt-1 **pl-${item.level * 1}**}>

Solution

There's probably some clever solutions to achieve dynamic utility classes, and there are a few other places in the code where this anti-pattern appears, but a really simple way to fix this is just to add .pl-1/2/3/4/5 to the globals.css in order to fix the ToC and call it a day. The only question is whether the maintainers actually would like to see progressive indentation for the Table of Contents. If so, I'd be happy to put together a PR that fixes this issue, so that, e.g. the last three items on the ToC in the screenshot above are actually indented further, and the hierarchical structure of the page is reflected in the ToC. Since the upper bound on heading levels is pretty small, we can just ship them all and it's almost no cost.

Although it's clearly been written in such a way that it's supposed to have recursive indentation, I'd just like an OK from a maintainer that this would be a minor cosmetic improvement, and I'll go ahead and submit a fix.

Either way, we should stop writing dynamic utility classes, because it's not supported by Tailwind.

@jeffchuber
Copy link
Contributor

@hesreallyhim do you want to give that a shot? really great write-up!

@jeffchuber jeffchuber added good first issue Good for newcomers documentation Improvements or additions to documentation labels Apr 15, 2025
@hesreallyhim
Copy link
Contributor Author

@hesreallyhim do you want to give that a shot? really great write-up!

Yeah no problem, it should be a quick patch if we just want to handle the table of contents issue I noted. I'll just add "pl-{1..5}" to the global styles

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants