Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support multiple Tailwind css files in a project. #242

Closed
wants to merge 6 commits into from

Conversation

delynn
Copy link

@delynn delynn commented Jan 27, 2023

This is an attempt at enhancing the current build and watch tasks to better support multiple Tailwind files in a project. If the project is setup in the vanilla Rails default, then nothing changes.

If a project adds multiple *.tailwind.css files into the app/assets/stylesheets directory, the build task switches to using a new compile command that supports specifying the file to compile and uses the convention of removing .tailwind from the source file name when generating the build file.

The watch task is also enhanced to fork multiple watch processes when multiple files are detected to improve the development experience.

An additional enhancement made is that the -c option is not included if the file being compiled contains the @config directive (a new feature that was added in Tailwind 3.2.

Finally, I considered modifying the existing compile_command to support a file argument, but figured it would be best to try and keep the existing behavior alone and just introduce new functionality. If there's interest in trying to fit this into the existing implementation, I'd be happy to rework this as well.

Hopefully these changes will address the issue outlined in #151.

@flavorjones
Copy link
Member

I don't like the idea of the rake task managing several long-running processes, especially since that's what the Procfile is intended to handle.

I'm also not sure about the hidden convention you're introducing here, which is that the mapping is something like:

  • app/assets/stylesheets/application.tailwind.css -> app/assets/builds/tailwind.css (current)
  • app/assets/stylesheets/admin.tailwind.css -> app/assets/builds/admin.css (new)

I think that decision deserves some discussion, especially since it doesn't support Engine asset directories (which I think is how Rails might recommend structuring things?).

An alternative idea I'll throw out there: if we found a way to inject a non-default input and output files into the rake task invocation, then users could update their Procfile to manage those processes. Something like:

# Procfile.dev
web: bin/rails server -p 3000
css: bin/rails tailwindcss:watch

# this would run:
#  tailwindcss \
#    -i app/assets/stylesheets/other.tailwind.css \
#    -o app/assets/builds/other.css \
#    -c config/tailwind.config.js
css-other: bin/rails tailwindcss:watch::file[app/assets/stylesheets/other.tailwind.css,app/assets/builds/other.css]

Would that something like that be sufficient for your needs today, until a longer-term decision is made about the needs described in #151 or the upstream tailwindlabs/tailwindcss#7698?

@flavorjones flavorjones requested a review from dhh January 27, 2023 14:18
@delynn
Copy link
Author

delynn commented Jan 27, 2023

Thanks for the review Mike.

I'm not a big fan of the rake task forking off other processes either, but given watch is only meant to be run in development, my goal was to create a better developer experience through convention instead of requiring additional lines be added to Procfile.dev. Your proposal for creating a new task to handle individual files is great if the preference ends up being to let Procfile.dev explicitly list out all of the files that need to be watched.

I admit that I didn't think of the engine asset directory case when building this, but the watcher doesn't need to be looking out for those files as those won't be changing, right? I'm happy to attempt to take engine assets into account, but it seems like the engine story is still a bit unclear and may need some additional tooling built as well?

With regard to the new mapping convention, I was honestly confused to initially see application.tailwind.css get turned into tailwind.css instead of application.css given the existing rendering conventions in Rails. I understand that asset generation isn't like rendering, but it's unclear to me why application is in the name of the source file as it doesn't seem to convey an instruction to the compilation.

Just to be clear, with this PR, the existing convention of renaming application.tailwind.css to tailwind.css only continues in the case where there's only one *.tailwind.css file in the project, and that's simply to support the existing behavior that's in place. If the project has multiple *.tailwindcss files, then application.tailwind.css gets turned into application.css and other.tailwind.css gets turned into other.css. I know this breaks with the existing convention, but maybe this is an opportunity to reconsider the existing convention for all cases as well?

Finally, while the bulk of this review/discussion has been around watch, the goal of #151 is really about the build step when it comes time to precompile the assets. While I like the idea of tailwindlabs/tailwindcss#7698, even if the Tailwind CLI ends up supporting multiple input/output file mappings the build task will need to either leverage a convention like the one I'm proposing, or provide a way for developers to specify those files in order to be able to pass them to the CLI.

@DaveSanders
Copy link

@flavorjones does the technique you outline for the procfile.dev still work? I'm trying to use it now with 2.0.29, and rails complains

Don't know how to build task 'tailwindcss:watch::file[app/assets/stylesheets/service.tailwind.css,' (See the list of available tasks with `rails --tasks`)

@flavorjones
Copy link
Member

@DaveSanders my comment above was imagining a alternative to this PR. The command you're running doesn't exist. Apologies for the confusion.

@caifara
Copy link

caifara commented Jun 7, 2023

This is exactly what we need!

I tried to install this gem with a git source, but I run into platform problems for the executable. Anyone knows how to solve that? (the solution in the readme and in the error message only seems to work when installing a normal gem)

@dhh
Copy link
Member

dhh commented Jun 20, 2023

Can I query a bit about the intention here? This seems like purely needed for a bit of optimization. Whether the app TW file contains utility classes for both admin and app at the same time, what does it matter? Do you have a case where it's a substantial difference? It's a substantial complication to add this, so before we explore the specific implementation further, I'd be curious to prod whether it's actually needed.

@DaveSanders
Copy link

Maybe its because of how we've set up our project, but we have one Rails 7 app with three different "sites" driven out of the same backend: A back office, a mobile view for technicians working from the road, and a public facing customer service site. Each has fairly different specialized layouts and while we do use some base CSS files for forms and other basic elements, by and large the visual schemes are quite different, and with different purposes.

We also tend to NOT use tailwinds class salad in our HTML but prefer to set up systems using @apply in CSS and use singular class names (or no classes at all if we can get away with it structurally) to keep HTML cleaner.

To facilitate this we are currently using one stylesheet but are having to "namespace" our CSS by adding a tag level class, which then helps us keep our different styles separated. Because we're using CSS, not SASS, that makes the CSS more cumbersome as we have to repeat the namespace anywhere there are difference - and when you have a lot of nested styling, that's a PITA.

Maybe we're approaching the problem incorrectly, or maybe we're a complex enough project that we should just be handling our CSS outside of rails. But we really like the current rapid workflow of not needing additional tooling. If I had SASS I could probably at least get rid of the tediousness of the flat CSS. Having the CSS merged into one file isn't THAT big of deal in my case, other than maybe some increase in the file size, but that doesn't really affect us.

For me this is a "nice to have" feature as it would definitely help me out, but there are at least a few workarounds, and once you get past the initial setup it really doesn't affect us THAT badly, just an annoyance.

@delynn
Copy link
Author

delynn commented Jun 20, 2023

Do you have a case where it's a substantial difference?

I freely admit that my use cases are at the far edges of what a typical project has, but they do result in multiple CSS files that are radically different in their content.

In the first case, we provide our users a WYSIWYG editor for creating printable badges/labels. A different layout that includes just the HTML and CSS needed to render the badge is sent to a headless Chrome instance to generate an image that can then be sent to a printer. In the second case, we have a WYSIWYG document editor and just the CSS that's needed to display the HTML document is included into a config file that an iPad uses in offline mode. In both of these cases we could send the entire application's CSS, but it felt unnecessary.

What originally lead me to find #151 was that I also ran into the issue of needing to build multiple Tailwind files when deploying my app. I initially got around this by redefining the tailwindcss:build in my project to compile the three CSS files I have. When I decided to wrap that work up into this PR I decided to also include my local changes to the tailwind:watch task as that improved the developer experience.

In my mind, the larger question is whether or not the goal of this project is to support multiple Tailwind files in a project?

If there's not an appetite for adding multiple file support, then I think creating a wiki page to provide guidance on how one could add the ability to build multiple files in a project would help people out in the future.

If there is an appetite, then I do think a discussion around what the convention for input and output file names will be necessary.

@RolandStuder
Copy link
Contributor

RolandStuder commented Jul 20, 2023

We could also need this. We have different brands served from the same app, towards customers we show different colors than towards. So text-green-500 for a customer facing page should be different than text-green-500 on a merchants page.
I wouldn't mind at all to adjust the procfile rather than having rake to handle multiple files.

@archangel519
Copy link

👍 in support of this PR. We also have a use case with a multi-tenancy application. Similar to @RolandStuder, we would like to have multiple files per tenant whereby we could have text-green-500 and similar variables resolve to different values based on the brands of the various tenants.

@davidwinter
Copy link

davidwinter commented Nov 24, 2023

I envisage a similar need where my app is creating an embeddable widget for other sites to include, and I simply want to prefix the CSS for that with something custom so that it does not clash with any other sites that are already using Tailwind, and also so that I can develop my apps main site and styling without having to have the prefix everywhere.

@eadz
Copy link

eadz commented Feb 19, 2024

Is there a workaround to get this working at the moment?

Rails has always supported multiple stylesheets, so you can seperate them for various parts of your app, or use global styles differently in different stylesheets. tailwindcss-rails seems to break this ability?

@jfi
Copy link

jfi commented Mar 5, 2024

Also adding my 👍 in support; having two files (one from an Engine) both called tailwind.css does, unsurprisingly, override styles...

For what it's worth, I've ended asking my IDE to listen on save and run a variation on "build:css": "tailwindcss --postcss --minify -i ./app/assets/stylesheets/application.tailwind.css -o ./app/assets/builds/application.css", in package.json, which seems similar to what @HashNotAdam has done in #151 (comment). For one reason or another, there are a few of us having this issue – see #108 as well.

@flavorjones
Copy link
Member

Let's consolidate this conversation into one place: #355

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants