Make files with class names configurable

[faqndo97]

Summary

Currently if we want to add or change the files_with_class_names we need to instantiate again the compressor class on our application configuration class. Instead of that would be nice if we can hide that information on the gem and just set those values through configuration.

Currently if we want to add a new directory of views we need to do:

config.assets.css_compressor = Tailwindcss::Compressor.new(files_with_class_names: Rails.root.glob("app/views/**/*.*") + Rails.root.glob("app/helpers/**/*.rb") + Rails.root.glob("app/custom_views/**/*.*"))

So basically we need to re-configure the base locations (views and helpers) if we want to just add a new view location. And as I said we expose the name of the compressor that if tomorrow we need to change by x reason, will generate a bunch of errors in repos using this.

So I'm proposing to do this:

module Dummy
  class Application < Rails::Application
    config.tailwind.files_with_class_names += Rails.root.glob("app/custom_views/**/*.*")
  end
end    

So in this way we can just add new folder or create our own array of pathnames that we want to use.

I test it manually using the keeping_class_names. Before adding the configuration

$ rake tailwindcss:keeping_class_names | wc -l
      40

After configuration was added

$ rake tailwindcss:keeping_class_names | wc -l
      45

This seems a huge PR mostly because I added a test/dummy application to verify this change. We can remove it if we don't want to add it here, but probably it can help us to verify changes.

Comment

1. I added a test on the compressor verifying the instance variable options to validate the files_with_class_names value. Maybe we have a better approach to test configurations that I don't know so if you point me on that direction I can improve the test.

2. I set the name of the base configuration key as tailwind. Probably we would like to change it by tailwindcss but I would like to verify everything else first and then I can change that if we want to.

[dhh]

Good stuff! I think we can slim down the dummy files that were added to just what we need. Otherwise just a note about defaults and I think we're good.

[dhh]

Don't think we need to set a default twice? Since you're doing it in the initializer, that should be enough.

[dhh]

Cut extra CR

[dixpac]

Nice!

Agree with @dhh that we could slim down the dummy.
As I can recall dummy slimming was done also in the ActiveStorage rails/activestorage#70 so maybe that can help you 😄

[dixpac]

Indentation

private
  def files_with_class_names

[faqndo97]

@dhh @dixpac Thanks for the reviews, I'll tackle the comments this week.

[faqndo97]

@dhh @dixpac I fixed the comments, and rebased with main, everything seems working as before. Sorry for the delay I was in the middle of a travel. Let me know if I need to improve something else.

Cheers.

[dixpac]

Looks good to me! Maybe we need to add explanation in docs about this new addition, so folks know this exists.

There is two points I'm not entirely sure about:

• Should this be a tailwindcss initializer instead of config.tailwind.files_with_class_names, so we can possibly add more settings in the future
• tailwind.files_with_class_names for some reason doesn't feel right to me, I would rather see something like tailwind.purging_locations or tailwind.files_to_purge, but I'm not a native English speaker so 🤷🏼

[faqndo97]

@dixpac Thanks for the review.

With the initializer do you mean to create an initializer file that can be generated into the project? Can you point me to an example? @dixpac

I'm happy to change the name of the configuration is that's needed. So the proposal is purging_locations any other other proposal @dhh ?

I'll add some documentation around this too on my next changes. I'll wait until we define the configuration name and then I'll push the changes.

[dixpac]

I think this is good and useful as is.

My only concern re config was that we could end up with multiple tailwind config options down the line (and bloat the environment files)

config.tailwind.files_with_class_names += Rails.root.glob("app/custom_views/**/*.*")
config.tailwind.option_x = ...
config.tailwind.option_y = ...

hence I proposed to have an initializer:

# config/initializers/tailwindcss.rb

Tailwindcss.setup do |config|
  config.tailwind.files_with_class_names += Rails.root.glob("app/custom_views/**/*.*")
  config.tailwind.option_x = ...
  config.tailwind.option_y = ...
end

But, both will work just fine.

Excited to have this merged 🎉 🙏🏼

[dixpac]

@dhh should we merge this. Feels like super useful thing to have, we can improve from this in the future if necessary.

[dixpac]

@dhh when we merge TW 3 binaries this is not going to be necessary, so I think we can close this PR.