search

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

store acceptable classnames in file #76

Closed tatchi closed 4 years ago

tatchi commented 4 years ago

Still would like to figure out something but it should already be in a usable state.

dylanirlbeck commented 4 years ago

Nice! I'll pull this down later this evening and take a look. Super excited to get this feature merged.

tatchi commented 4 years ago

Thanks for the review :)

  1. Did the pre-commit refmt hook work for you? I configured it recently and was curious.

I didn't pay too much attention but haven't seen anything special in the console so I'm not sure it worked.

The active classnames are now cached in a .tailwind_ppx_cache directory (similarly to what graphql_ppx does). When running the ppx, the current directory is in lib/bs folder. I don't know if it's a good place to store this file. From what I understood, graphql_ppx look higher in the hierarchy up until they find the graphql_schema.json file (or whatever it's called) and store the cache directory at the same level.

We don't have such a file, so for now I stored the cache folder 2 steps higher ../../ which is where I think it's the best place to store it. The logic might be fragile though; an improvement might be to look up until we find a bsconfig.json file? Let me know what do you think, I can create another PR to implement that :)

dylanirlbeck commented 4 years ago

@tatchi I'm currently still reviewing this, but the pre-commit formatting hook is done with husky and lint-staged, so can you try running yarn in the project root and then committing a change to a Reason file again? It should work after you've ran yarn to install dependencies.

tatchi commented 4 years ago

I should have addressed the reviews.

Just one idea about where we're putting the cache. I feel like it'd be best to always have the cache stored at the project root, since simply going two directories up might be a bit flaky. This will be especially important when we publish tailwind-ppx to opam, since the PPX may be in a different place.

Perhaps we could recursively look for an esy.json or package.json? Then we may have the problem if there's a misplaced package.json in some weird directory. In any event, let's have this discussion in a separate PR. I want to get this merged, and I think it works totally fine for normal BuckleScript projects.

I agree that looking for an esy.json or package.json file should be better (although probably not perfect). Let's indeed get this merged and improve that in another PR 😃