Skip to content
This repository has been archived by the owner on Aug 17, 2021. It is now read-only.

Change PPX to look at index.css instead of tailwind.css by default #62

Open
dylanirlbeck opened this issue Apr 29, 2020 · 6 comments
Open
Assignees
Labels

Comments

@dylanirlbeck
Copy link
Owner

Per some good comments on my call for features, we should modify the PPX to instead look at the file where the Tailwind code is included (i.e. wherever the user puts @import tailwindcss/...).

This feature could either be a configuration or be the default behavior --- this issue can serve as the discussion for what the implementation would look like.

@dylanirlbeck
Copy link
Owner Author

dylanirlbeck commented Apr 29, 2020

At the moment, I'm of the opinion that the PPX should instead be given the path to the index.css in its -path parameter, and do some internal logic to pre-process the file through the Tailwind CLI. This feature would almost certainly be tied to #25 , since we'll need to implement some sort of cache so we don't rebuild the tailwind.css file every single time.

@zth Do you have any initial ideas of how this feature could be implemented? Thanks in advance!

@azkane
Copy link
Contributor

azkane commented Apr 30, 2020

Hey, sorry if I'm missing something, but isn't this already the behavior when using the generator feature in bsconfig + specify -path?

@dylanirlbeck
Copy link
Owner Author

@azkane So you're correct in that it will essentially be the same behavior, though my idea was that this feature will complement having the generator in your bsconfig. Let me try to explain explain (though I may be wrong b/c I recently learned about generators!).

Most people, and any production applications, will likely be using PostCSS in tandem with Tailwind. This solves a few issues, but most importantly it means you don't necessarily have to commit your tailwind.css file to version control. It also allows you to cleanly use PurgeCSS for purging unused classes, for which tailwind-ppx provides a custom extractor function!

I have the opinion that by default, we should be encouraging people to use PostCSS instead of using the Tailwind CLI directly: this is why I think having the -path provided should lead do the base index.css/styles.css/whatever.css that includes the tailwind css, rather than the generated Tailwind itself. One further implication of the current architecture is that it makes it hard to play well with tools like Next.js --- see this comment.

Finally, it seems to me that having the generator rule in your bsconfig is great for when your working in dev mode locally, so having both the generator defined and PostCSS is the best combination. Please let me know your thoughts, and if I missed something 😄

@zth
Copy link
Contributor

zth commented May 1, 2020

@dylanirlbeck my initial idea, that isn't that thought through yet, is to have the PPX call the Tailwind CLI itself using the index.css (or whatever it's called by the user) to produce the CSS containing all the classes available. The results of that + whatever processing you're doing with the CSS parser could then be cached in some local directory (node_modules/.cache/tailwind-ppx perhaps, or .tailwind-ppx-cache like how graphql_ppx does it), and the cached contents re-used by the PPX.

Calling the Tailwind CLI could either by done by running npx tailwindcss or trying to go through the locally installed version directly, if you're able to figure out where that's located. Going through the locally installed version would probably be a lot faster than using npx.

Cache invalidation could be handled by hashing the contents of tailwind.config.js (which would potentially then also need its location passed to the PPX somehow if it's not in the root directory) and whatever CSS file the user points to for the Tailwind CSS. If the hashes match, use the cached contents, otherwise re-run the Tailwind CLI to produce the new output. I believe this is how graphql_ppx is doing things, although I might be mistaken.

There's a few potential issues:

  • Running the Tailwind CLI is likely to be slow, at least compared to what you're used to with Reason. Unsure how much of a problem this will be as it won't happen often, but still worth considering
  • This will be great for people using Tailwind with a bundler (should be most people) but potentially redundant for people who produce their Tailwind CSS file manually

The main benefits are:

  • Caching the processed contents means that the performance will potentially be a lot better
  • This would remove unnecessary steps for people using Tailwind through a bundler, since they wouldn't need to produce a "compiled" Tailwind file manually and keep track of when it changes

These are my initial thoughts, although they might all be wrong as usual ;)

@azkane
Copy link
Contributor

azkane commented May 2, 2020

Hey!

I've been giving it some thought and experimenting with the demo project. I've found that configuring generators with postcss covers mostly all the points you both guys raised.

No need to commit the final build file, by defining a generator step that runs postcss-cli at build time that generates the final css file. This has the advantage of tailwind-ppx not picking up only the rules defined by tailwindcss but by any other plugin of postcss thats on the pipeline. (which makes tailwind-ppx validate all the project's css rules at compile time, is this out of scope?).

Since the index.css/main.css (file where the tailwind rules are applied) is specified in the generator definition, BS will set a watcher on it when running with -w and rebuilding the css file on any changes to said file. (This doesn't apply for tailwind.config.js or postcss.config.cs, if those are modified you need to restart the build to pick up the changes.). This frees tailwind-ppx from doing some sort of cache invalidation.

The downside of this approach is that the first time BS runs the build, it does the generators last, so on the first run tailwind-ppx is unable to find the tailwind.css file. The build watcher keeps running and upon doing touch src/main.css or any other change in the project it retriggers the build and does it correctly.

As the configuration for the tailwind build is defined in postcss.config.js and tailwind.config.js any bundler should be able to pick it up for a production build without much issue.

The speed of rebuilding the css is bound to project size/rules/etc, but on the demo project on my machine it takes about 2s every time I modify main.css.

I'm not saying this is the only path to achieve these goals, I'm just trying to show how can these be achieved with whats already available. The demo project with those changes is here

@dylanirlbeck
Copy link
Owner Author

dylanirlbeck commented May 4, 2020

@zth @azkane What do you both think about the PPX having two different "configurations" by passing in certain parameters. Let me explain:

Use 1: Passing in a -raw ../path/to/tailwind.css parameter that tells the PPX exactly where to find the generated tailwind.css file. This option is for those who aren't using PostCSS. We would highly recommend using the generator in the bsconfig.json to watch for changes to the user's index.css file.

Use 2: Passing in a -postcss ../path/to/index.css parameter that indicates this user is using PostCSS, and tailwind-ppx should handle building the tailwind.css for you in a smart way (i.e. by hashng index.css or tailwind.config.js). Alternatively, we could encourage user's to take advantage of the built-in generator for postcss that @azkane mentioned and linked to.

Does having these options make sense? Please give me your thoughts!

@dylanirlbeck dylanirlbeck self-assigned this May 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants