-
Notifications
You must be signed in to change notification settings - Fork 24
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?
Changes from 6 commits
ee9c360
77fb6f5
6011aa1
6cbfdab
a544f04
793d976
7a2bbd9
a6f4164
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 |
---|---|---|
|
@@ -44,11 +44,25 @@ protected function tearDown(): void | |
} | ||
} | ||
|
||
protected function isTailwindBinaryEqualOrGreaterThan4(): bool | ||
{ | ||
$versionBuilder = new TailwindBuilder( | ||
'', | ||
[], | ||
'', | ||
new ArrayAdapter(), | ||
); | ||
|
||
return $versionBuilder->isBinaryVersionEqualOrGreaterThan4(); | ||
} | ||
|
||
public function testIntegrationWithDefaultOptions(): void | ||
{ | ||
$cssFilesSuffix = $this->isTailwindBinaryEqualOrGreaterThan4() ? '-v4' : ''; | ||
|
||
$builder = new TailwindBuilder( | ||
__DIR__.'/fixtures', | ||
[__DIR__.'/fixtures/assets/styles/app.css'], | ||
[__DIR__.'/fixtures/assets/styles/app'.$cssFilesSuffix.'.css'], | ||
__DIR__.'/fixtures/var/tailwind', | ||
new ArrayAdapter(), | ||
null, | ||
|
@@ -59,17 +73,19 @@ public function testIntegrationWithDefaultOptions(): void | |
$process->wait(); | ||
|
||
$this->assertTrue($process->isSuccessful()); | ||
$this->assertFileExists(__DIR__.'/fixtures/var/tailwind/app.built.css'); | ||
$this->assertFileExists(__DIR__.'/fixtures/var/tailwind/app'.$cssFilesSuffix.'.built.css'); | ||
|
||
$outputFileContents = file_get_contents(__DIR__.'/fixtures/var/tailwind/app.built.css'); | ||
$this->assertStringContainsString("body {\n background-color: red;\n}", $outputFileContents, 'The output file should contain non-minified CSS.'); | ||
$outputFileContents = file_get_contents(__DIR__.'/fixtures/var/tailwind/app'.$cssFilesSuffix.'.built.css'); | ||
$this->assertStringContainsString("body {\n background-color: #ef4444;\n}", $outputFileContents, 'The output file should contain non-minified CSS.'); | ||
} | ||
|
||
public function testIntegrationWithMinify(): void | ||
{ | ||
$cssFilesSuffix = $this->isTailwindBinaryEqualOrGreaterThan4() ? '-v4' : ''; | ||
|
||
$builder = new TailwindBuilder( | ||
__DIR__.'/fixtures', | ||
[__DIR__.'/fixtures/assets/styles/app.css'], | ||
[__DIR__.'/fixtures/assets/styles/app'.$cssFilesSuffix.'.css'], | ||
__DIR__.'/fixtures/var/tailwind', | ||
new ArrayAdapter(), | ||
null, | ||
|
@@ -80,52 +96,59 @@ public function testIntegrationWithMinify(): void | |
$process->wait(); | ||
|
||
$this->assertTrue($process->isSuccessful()); | ||
$this->assertFileExists(__DIR__.'/fixtures/var/tailwind/app.built.css'); | ||
$this->assertFileExists(__DIR__.'/fixtures/var/tailwind/app'.$cssFilesSuffix.'.built.css'); | ||
|
||
$outputFileContents = file_get_contents(__DIR__.'/fixtures/var/tailwind/app.built.css'); | ||
$this->assertStringContainsString('body{background-color:red}', $outputFileContents, 'The output file should contain minified CSS.'); | ||
$outputFileContents = file_get_contents(__DIR__.'/fixtures/var/tailwind/app'.$cssFilesSuffix.'.built.css'); | ||
$this->assertStringContainsString('body{background-color:#ef4444}', $outputFileContents, 'The output file should contain minified CSS.'); | ||
} | ||
|
||
public function testBuildProvidedInputFile(): void | ||
{ | ||
$cssFilesSuffix = $this->isTailwindBinaryEqualOrGreaterThan4() ? '-v4' : ''; | ||
|
||
$builder = new TailwindBuilder( | ||
__DIR__.'/fixtures', | ||
[__DIR__.'/fixtures/assets/styles/app.css', __DIR__.'/fixtures/assets/styles/second.css'], | ||
[__DIR__.'/fixtures/assets/styles/app'.$cssFilesSuffix.'.css', __DIR__.'/fixtures/assets/styles/second'.$cssFilesSuffix.'.css'], | ||
__DIR__.'/fixtures/var/tailwind', | ||
new ArrayAdapter(), | ||
null, | ||
'v3.4.17', | ||
null, | ||
__DIR__.'/fixtures/tailwind.config.js' | ||
); | ||
$process = $builder->runBuild(watch: false, poll: false, minify: true, inputFile: 'assets/styles/second.css'); | ||
$process = $builder->runBuild(watch: false, poll: false, minify: true, inputFile: 'assets/styles/second'.$cssFilesSuffix.'.css'); | ||
$process->wait(); | ||
|
||
$this->assertTrue($process->isSuccessful()); | ||
$this->assertFileExists(__DIR__.'/fixtures/var/tailwind/second.built.css'); | ||
$this->assertFileExists(__DIR__.'/fixtures/var/tailwind/second'.$cssFilesSuffix.'.built.css'); | ||
|
||
$outputFileContents = file_get_contents(__DIR__.'/fixtures/var/tailwind/second.built.css'); | ||
$this->assertStringContainsString('body{background-color:blue}', $outputFileContents, 'The output file should contain minified CSS.'); | ||
$outputFileContents = file_get_contents(__DIR__.'/fixtures/var/tailwind/second'.$cssFilesSuffix.'.built.css'); | ||
$this->assertStringContainsString('body{background-color:#3b82f6}', $outputFileContents, 'The output file should contain minified CSS.'); | ||
} | ||
|
||
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 commentThe 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 commentThe 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. So unfortunately it will not be available in the future. 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. 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 commentThe 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 commentThe 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 commentThe 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... |
||
} | ||
|
||
$builder = new TailwindBuilder( | ||
__DIR__.'/fixtures', | ||
[__DIR__.'/fixtures/assets/styles/app.css'], | ||
[__DIR__.'/fixtures/assets/styles/app'.$cssFilesSuffix.'.css'], | ||
__DIR__.'/fixtures/var/tailwind', | ||
new ArrayAdapter(), | ||
null, | ||
'v3.4.17', | ||
null, | ||
__DIR__.'/fixtures/tailwind.config.js', | ||
__DIR__.'/fixtures/postcss.config.js', | ||
); | ||
$process = $builder->runBuild(watch: false, poll: false, minify: false); | ||
$process->wait(); | ||
|
||
$this->assertTrue($process->isSuccessful()); | ||
$this->assertFileExists(__DIR__.'/fixtures/var/tailwind/app.built.css'); | ||
$this->assertFileExists(__DIR__.'/fixtures/var/tailwind/app'.$cssFilesSuffix.'.built.css'); | ||
|
||
$outputFileContents = file_get_contents(__DIR__.'/fixtures/var/tailwind/app.built.css'); | ||
$outputFileContents = file_get_contents(__DIR__.'/fixtures/var/tailwind/app'.$cssFilesSuffix.'.built.css'); | ||
$this->assertStringContainsString('.dummy {}', $outputFileContents, 'The output file should contain the dummy CSS added by the dummy plugin.'); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
@import "tailwindcss"; | ||
|
||
body { | ||
background-color: #ef4444; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,5 +3,5 @@ | |
@tailwind utilities; | ||
|
||
body { | ||
background-color: red; | ||
background-color: #ef4444; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
@import "tailwindcss"; | ||
|
||
body { | ||
background-color: #3b82f6; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,5 +3,5 @@ | |
@tailwind utilities; | ||
|
||
body { | ||
background-color: blue; | ||
background-color: #3b82f6; | ||
} |
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.