-
Notifications
You must be signed in to change notification settings - Fork 23
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
Proposal to fix Tailwind CSS v4 support #86
base: main
Are you sure you want to change the base?
Conversation
Notice that if the bundle is already installed, |
Oups! I'll fix the tests as soon as I can. |
…ed anymore by the standalone version
Hi, |
} | ||
|
||
public function testIntegrationWithPostcss(): void | ||
{ | ||
$cssFilesSuffix = $this->isTailwindBinaryEqualOrGreaterThan4() ? '-v4' : ''; | ||
if ($cssFilesSuffix) { | ||
$this->markTestSkipped('Postcss seems not compatible with Tailwind CLI v4!'); |
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.
Hm, if so I think we should add a note about that in https://symfony.com/bundles/TailwindBundle/current/index.html#using-a-postcss-config-file . WDYT? IIRC Symfony docs have some special syntax for versions, probably we could leverage it to let it know that this does not work since v4 of tailwind
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.
I agree. I opened an issue on the TailwindCSS repo and I've got an answer :
"The CLI does not use postcss anymore so we don't have a plan to making this work.
If you rely on postcss I recommend using the postcss-cli directly with the @tailwindcss/postcss extension."
So unfortunately it will not be available in the future.
I'll check the SF docs formats and standards to update it.
Thanks.
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.
I just put a warning block in the documentation as the deprecated one seems more appropriate for the bundle version that the binary version. What do you thing about it ?
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.
Actually, we could raise and test an exception in case of using PostCSS with v4+ binary. WDYT?
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.
hm, throwing an excepting with a clear message explaining that it does not work in Tailwind v4 anymore sounds good to me. Though what happens in the Tailwind binary if when you continue using postcss? Does it also throw an error? Probably the best way would be to do the same to be consistent. But wait, if we just pass the same option to the Tailwind binary, won't it fail upstream? I mean, in theory, we should do nothing because it should fail anyway when the Tailwind binary will run, or am I missing something?
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.
No, actually nothing happens! You can add any fake argument to the binary while executing it, no error is thrown and the css is built correctly but without PostCSS processing...
…d anymore in versions highter than 4.0.0
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.
Great start here! I think we need to address the different binaries now available in tailwind 4.
@@ -70,7 +70,7 @@ public function getConfigTreeBuilder(): TreeBuilder | |||
->end() | |||
->scalarNode('binary_version') | |||
->info('Tailwind CLI version to download - null means the latest version') | |||
->defaultValue('v3.4.17') | |||
->defaultNull() |
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.
This should be kept. I think we need to enforce a version going forward so we don't unexpectedly upgrade.
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, I agree. The fact that future versions will not support PostCSS, it's better to enforce the last 3.X.X version to let developer choose to upgrade or not.
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.
I put back the default binary version to v3.4.17 and added info in the doc.
I pushed the exception in the case of using PostCSS with v4 or higher.
…ecify it in the doc
Hi,
I faced the Tailwind CSS v4 issue with the bundle and noticed the last #83.
Here is a proposal to fix it and allow v4 support.
It just put the new Tailwind CSS directive if the version used is equal or higher to v4, replace old directives if exist.