dylanirlbeck / tailwind-ppx

A Reason/OCaml Pre-Processor eXtension (PPX) that validates your Tailwind classes at compile-time.
MIT License
152 stars 15 forks source link

Allow expressions inside the ppx #4

Closed dylanirlbeck closed 4 years ago

dylanirlbeck commented 4 years ago

Instead of taking in a Pconst_string (which means you can only use the ppx if you know the class names at compile-time), we'd be better off taking in an expression that evaluates to a string (if that's possible) so people can make function calls or use identifiers inside the ppx.

ManasJayanth commented 4 years ago

Wouldn't accepting expressions leave some of the type checking for the runtime? Is there anything planned for that?

dylanirlbeck commented 4 years ago

@prometheansacrifice You bring up a great point! After thinking about this more, I should clarify what I meant (and maybe you can help!). Lots of people use re-classnames with Tailwind, so my goal was to somehow support using Cn.make inside of the PPX to validate the class names. For example, I'd tailwind-ppx like to support:

<Component className=[%tw Cn.make(["flex", "bg-mono-100"->Cn.ifTrue(someBool)])] />

or something similar. We still know the class names at compile-time (since they're hardcoded), and the compiler gives us the type of the Cn.make function call (a string), so the type is determined before runtime.

zth commented 4 years ago

First off all, excellent project, thank you so much for this! Really been waiting for something like this (and this also means I can finally let https://github.com/zth/bs-tailwind-ppx rest without feeling bad 😉).

I actually think only using Pconst_string is a great feature. In fact, I think it could be taken as far as only allowing quotation mark strings and not {||} and friends.

The reason is that if you know that each [%tw] node will always contain a list of valid Tailwind classes without expressions or interpolation, you know that there's no way to output code that's hard/impossible to purge unused classes from. It also makes slapping on a configuration for purging unused classes from the Tailwind output really easy. You could even include the regex extractor function for it in your package:

// postcss.config.js
const purgecss = require('@fullhuman/postcss-purgecss')({

  // Specify the paths to all of the template files in your project 
  content: [
    './src/**/*.re',
  ],
  defaultExtractor: require("tailwind-ppx").extractor
})

module.exports = {
  plugins: [
    require('tailwindcss'),
    require('autoprefixer'),
    ...process.env.NODE_ENV === 'production'
      ? [purgecss]
      : []
  ]
}

This would make the Cn case look like this:

<Component className={Cn.make([[%tw "flex"], [%tw "bg-mono-100"]->Cn.ifTrue(someBool)])} />

Also, keeping that part of the PPX pretty uncomplicated would perhaps be nice in the long run 😃

Just my 5 cents!

dylanirlbeck commented 4 years ago

@zth Thanks for the suggestion! I think for now the consensus seems to be leave the only accepted type as Pconst_string, but I'll explore enforcing quotations. I'm not a super experienced web developer, so I'm actually not familiar with PurgeCSS and/or the whole concept of purging unused classes.

I'd have to look into what this extractor function would look like, and it also appears I'd need to create a tailwind-ppx NPM package (though I may be wrong). It'd be great if you could give me some advice or point me to some resources to better understand PurgeCSS! Thanks again, and I'll go ahead and close this issue.

zth commented 4 years ago

If you want to do that, I think you can enforce only quotations setting quotation_delimiter: None on Pconst_string.

Oh, no worries, and sorry, let me explain what I'm after a bit better!

By default, Tailwind will generate a huge CSS blob containing all its generated classes, regardless of whether you use them in your code or not. This results in an absolutely huge CSS file that in the typical case will contain 80-90% unused classes, but that'll end up shipped to the user anyway.

The common approach of dealing with this, as described in the official docs, is to use a PostCSS plugin called purgecss. It purges the unused CSS classes by scanning your code and figuring out what classes you're using. It then removes any class from the resulting CSS that isn't actually used.

However, the classical approach to this has a ton of flaws:

With all that said though, I think this PPX has a shot at removing virtually all of those issues 😃 If you can enforce using a regular quoted string with no expressions, the risk of expressions breaking the extraction is gone. Since you'll also know that running a regex matching [%tw "<content>"] on the users code will always extract all Tailwind classes, you can also (like I mentioned before) ship a default extractor function that people can use easily.

You wouldn't have to ship a new package called tailwind-ppx, I was just mistaken in what the package is called now. Basically, everything you'd have to do is to create an index.js roughly like:

module.exports = {
  extractor: function(source) {
    return someLogicForExtractingAnArrayOfAllClassNamesUsedInSource(source);
  }
}

And then people could use that in their postcss.config.js to enable dead class elimination by simply setting their extractor to: require('@dylanirlbeck/tailwind-ppx').extractor.

Does this make sense?

dylanirlbeck commented 4 years ago

@zth That explanation was wonderful! Thank you very much for being so thorough. I'm going to make this feature the next big one on the roadmap, and I'll probably be consulting you for advice along the way 😄

zth commented 4 years ago

@dylanirlbeck Awesome! 😄 I actually couldn't resist the urge, so I went ahead and did an initial PR with an extractor function as outlined above: https://github.com/dylanirlbeck/tailwind-ppx/pull/45/files 😬