andreipfeiffer / css-in-js

A thorough analysis of all the current CSS-in-JS solutions with SSR & TypeScript support for Next.js
829 stars 23 forks source link

Include TailwindCSS? #4

Closed styxlab closed 3 years ago

styxlab commented 3 years ago

Not strictly CSS but I would love to see it compared with the rest. Have you considered including it?

andreipfeiffer commented 3 years ago

Hey @styxlab , I have thought briefly about it, but ditched the idea because it felt to me that it's not quite a CSS-in-JS approach.

However, looking at Tailwind's popularity, probably it will make sense to try it out, and see if it fills the gaps I'm looking for. It would also help better understand its tradeoffs and use-cases.

styxlab commented 3 years ago

I agree, it's a different approach. However, you are also writing Tailwind classes directly into JS, so it's basically TailwindCSS-in-JS. Glad that you'll have a look at it, because it is an alternative to the other packages you compared. Thanks for your consideration, you can close the issue.

andreipfeiffer commented 3 years ago

I'll leave it open, cause I have another open issue about adding another library.

andreipfeiffer commented 3 years ago

@styxlab has you used TW with TS? I'm banging my head around trying to use it, and can't figure out how.

What I don't know how to do is for example:

Here are the methods I've tried:

Also, wherever you search for tailwind, react, typescript you find only the setup and a silly hello fucking world example, so no luck with that.

The official docs also have zero information about TS.

styxlab commented 3 years ago

Hey @andreipfeiffer!

Yes, I am using tailwind in pure typescript projects, but it seems that I do not use it in the same way as you are trying to. First off, I don't convert tailwind.config.js into a .ts file (I leave it as is) and I also don't try to make types from the tailwind descriptors nor do I pass tailwind variables around as props.

For me, tailwind just provides a couple of descriptors that are basic strings which I pass to the className attribute. So, I rely on the eslint-plugin-tailwind package for getting the descriptors right and not on a typed system. I may pass around className or className fragments which are again simple strings.

What you are trying to do is definitively a huge effort and I would not do that without being backed by the tailwindcss team. The Prisma team did something similar for their database interface, which might serve as an inspiration if you really want to get into that. Please ping me on twitter, if you want to discuss further.

andreipfeiffer commented 3 years ago

@styxlab I've tried DM-ing you on TW, but your DMs are closed.

Dynamic styles

With static types it's easy, but let me see of I understand how you deal with dynamic styles:

const padding = `p-${some_boolean_prop ? '2' : '4'}`; // dynamic styles based on logic
const color = `text-${color_prop}`; // dynamic styles based on props
return <p className={`text-large ${color} ${padding}`}></p>

If this is the case, then you basically don't get any type-safety in regards to styling, so TS has zero impact for styling, as far as I can tell.

About eslint-plugin-tailwind

Can you please elaborate what does the ESLint plugin do?

  1. does it check that you use TW classes in a specific order?
  2. does it check that you use the correct TW classes? ie: restricts you to use non-existing classes?
  3. does it work with custom values, as well?

About Tailwind in general

  1. Do you use it exclusively, without any other method of styling?
  2. What types of applications do you build? Do they have a lot of reusable components, like a admin panel? or is it more like a client facing UI, with high customisations?

About Prisma

Do you have any insights on how they manage this?

styxlab commented 3 years ago

Dynamic styles

I don't use all variants you show, mostly because partial substitution inhibits purging but I do use dynamic selection of TW classes. I agree that TS has no impact here, except for trivially checking that you pass a string to className.

About eslint-plugin-tailwind

Currently the plugin only checks for correct ordering but not for for correct classes. However, I also use a vscode plugin with tailwindcss intellisense, so correct classes are shown while typing and can be easily selected.

About Tailwind in general

I use it preferably, but I do combine it with regular css stylesheets or regular inline styles where needed. Type of applications range from admin panel to front-end UI, with varying degree of customization.

About Prisma

Prisma came to my mind as they are solving similar problems.

TailwindCSS and Typescript

While there is no official TS support at the moment, TS support is on its way:

andreipfeiffer commented 3 years ago

Got it, not I kinda get how purging works. I'll also check out the Prisma video.

I saw there is a PR on Definitely Typed, I was curious if it's only for built-in styles, or you could also provide your own typings for customisations. But I'll wait and see after it's merged.

andreipfeiffer commented 3 years ago

Review available here: https://github.com/andreipfeiffer/css-in-js/blob/main/README.md#tailwind Would probably revisit this when the TS PR will be merged.

styxlab commented 3 years ago

Thanks a lot for your preliminary review. Just one comment with respect to purging, you write:

we have to be aware of purging to not get missing design tokens in production builds

This is not accurate. Purging is for eliminating unused design tokens in an effort to reduce bundle size. You can opt-out of purging altogether, if you want to.

andreipfeiffer commented 3 years ago

As far as I understand (but please correct me if I'm wrong):

  1. purging is needed if you want to reduce the CSS bundle size, to contain only the classes that are actually used
  2. this can be skipped if you make sure that config file only contains the required tokens, right?
  3. however, if you misuse dynamic styling (like text-${color}, which I don't know how you could enforce and prevent), purging will remove those styles from the production bundle, so you might end up with missing styles in production, but they would work in development.
styxlab commented 3 years ago
  1. purging is optional, but highly recommended if you want to reduce CSS bundle size, to contain only the classes/tokens that are actually used
  2. contained in 1 (your sentence is true, but a highly unlikely use case as you would have to change your config anytime you add a new token).
  3. yes, the risk of purging is that it is too aggressive eliminating tokens that you actually need. But there are some guidelines that help in avoiding such trouble.
  4. The risk described in 3. is a result of a very simple purging function. You can deploy your own purging function and tailor it to your needs.

The whole point of purging is to give you a rich set of tokens during development, and eliminate the unused later to reduce bundle size.

andreipfeiffer commented 3 years ago

Completely agree with you.

However, one of our initial goals was to look for a solution that's (type) safe and have a somewhat low(ish) learning curve (which is debatable, of course).

Using all the other CSS-in-JS solutions, it was pretty easy to figure out what you have to do to get the work done (took me about 2 hours on average to figure out how a library fully works). Using TW, it took me (until now) more than 5 times the effort to understand how it works, and I still haven't fully figured it out, so it's definitely not something that's solid.

It might be easy to pick up, in regard to "choosing class names" and attaching them on HTML elements. But there is so much more tooling needed on top of it, at least considering our development workflow, or expectations.

styxlab commented 3 years ago

No worries. I am not affiliated with Tailwind, just a happy user. Totally agree that it currently does not fit well to your selection criteria.

andreipfeiffer commented 3 years ago

Anyway, thank you for all this thread @styxlab as it forced me to look into TW and better understand its weak & strong points.