-
Notifications
You must be signed in to change notification settings - Fork 97
Update for Tailwind CSS v4 #164
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1 @@ | ||
@tailwind base; | ||
@tailwind components; | ||
@tailwind utilities; | ||
@import "tailwindcss"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It could be useful to open a separate PR just for this change, in case #164 takes a while to get merged. This would be useful right away, regardless of how you install tailwind. Those installation woes can be worked around in various ways, but this one can't be - you just have to replace it every time, all the time. The only question is: do we want to be backward compatible with 3.x? I don't think so (since all the generators I'm aware of are using However, if backward compatibility is not a concern, I don't see why this couldn't be accepted right away as a separate PR. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,6 @@ | |
"name": "app", | ||
"private": "true", | ||
"scripts": { | ||
"build:css": "tailwindcss -i ./app/assets/stylesheets/application.tailwind.css -o ./app/assets/builds/application.css" | ||
"build:css": "npx @tailwindcss/cli -i ./app/assets/stylesheets/application.tailwind.css -o ./app/assets/builds/application.css" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it really necessary to run There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my testing, something like To be honest, I'm not sure this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is necessary because cssbundling-rails and jsbundling-rails are enhancing some rake tasks like precompile:assets to call this build and build:css task present in the package.json There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @navidemad I mean the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What I meant is that by using I see you added There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Correct, this legacy file was added prior to this pull request. I updated it to match the main one that gets copied. I'm not sure we need this file at all. I am in favor of removing it unless there are objections. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Objection, your honor 🧑⚖️😅 If you are using importmaps, then But if you are using a JS bundler ( So yeah, please don't drop it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Thanks for chiming in! There's already one that gets copied at the root of the |
||
} | ||
} |
This file was deleted.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there even a
postcss.config.js
now? In my understanding, it was dropped in v4 (as a separate package) and merged into tailwind itself?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you can install postcss independently with cssbundling-rails (
bin/rails css:install:postcss
)