Skip to content

Do not emit empty rules/at-rules #16121

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

Merged
merged 9 commits into from
Jan 31, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Prevent camelCasing CSS custom properties added by JavaScript plugins ([#16103](https://github.com/tailwindlabs/tailwindcss/pull/16103))
- Do not emit `@keyframes` in `@theme reference` ([#16120](https://github.com/tailwindlabs/tailwindcss/pull/16120))
- Discard invalid declarations when parsing CSS ([#16093](https://github.com/tailwindlabs/tailwindcss/pull/16093))
- Do not emit empty CSS rules and at-rules ([#16121](https://github.com/tailwindlabs/tailwindcss/pull/16121))

## [4.0.1] - 2025-01-29

Expand Down
7 changes: 1 addition & 6 deletions integrations/cli/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -368,12 +368,7 @@ describe.each([
`,
)

await fs.expectFileToContain('project-a/dist/out.css', [
css`
:root, :host {
}
`,
])
await fs.expectFileToContain('project-a/dist/out.css', [css``])
},
)

Expand Down
90 changes: 90 additions & 0 deletions packages/tailwindcss/src/ast.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { expect, it } from 'vitest'
import { context, decl, optimizeAst, styleRule, toCss, walk, WalkAction } from './ast'
import * as CSS from './css-parser'

const css = String.raw

it('should pretty print an AST', () => {
expect(toCss(optimizeAst(CSS.parse('.foo{color:red;&:hover{color:blue;}}'))))
.toMatchInlineSnapshot(`
Expand Down Expand Up @@ -95,3 +97,91 @@ it('should stop walking when returning `WalkAction.Stop`', () => {
}
`)
})

it('should not emit empty rules once optimized', () => {
let ast = CSS.parse(css`
/* Empty rule */
.foo {
}

/* Empty rule, with nesting */
.foo {
.bar {
}
.baz {
}
}

/* Empty rule, with special case '&' rules */
.foo {
& {
&:hover {
}
&:focus {
}
}
}

/* Empty at-rule */
@media (min-width: 768px) {
}

/* Empty at-rule with nesting*/
@media (min-width: 768px) {
.foo {
}

@media (min-width: 1024px) {
.bar {
}
}
}

/* Exceptions: */
@charset "UTF-8";
@layer foo, bar, baz;
@custom-media --modern (color), (hover);
@namespace 'http://www.w3.org/1999/xhtml';
`)

expect(toCss(ast)).toMatchInlineSnapshot(`
".foo {
}
.foo {
.bar {
}
.baz {
}
}
.foo {
& {
&:hover {
}
&:focus {
}
}
}
@media (min-width: 768px);
@media (min-width: 768px) {
.foo {
}
@media (min-width: 1024px) {
.bar {
}
}
}
@charset "UTF-8";
@layer foo, bar, baz;
@custom-media --modern (color), (hover);
@namespace 'http://www.w3.org/1999/xhtml';
"
`)

expect(toCss(optimizeAst(ast))).toMatchInlineSnapshot(`
"@charset "UTF-8";
@layer foo, bar, baz;
@custom-media --modern (color), (hover);
@namespace 'http://www.w3.org/1999/xhtml';
"
`)
})
18 changes: 15 additions & 3 deletions packages/tailwindcss/src/ast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,9 @@ export function optimizeAst(ast: AstNode[]) {
for (let child of node.nodes) {
let nodes: AstNode[] = []
transform(child, nodes, depth + 1)
parent.push(...nodes)
if (nodes.length > 0) {
parent.push(...nodes)
}
}
}

Expand All @@ -271,7 +273,9 @@ export function optimizeAst(ast: AstNode[]) {
for (let child of node.nodes) {
transform(child, copy.nodes, depth + 1)
}
parent.push(copy)
if (copy.nodes.length > 0) {
parent.push(copy)
}
}
}

Expand All @@ -297,7 +301,15 @@ export function optimizeAst(ast: AstNode[]) {
for (let child of node.nodes) {
transform(child, copy.nodes, depth + 1)
}
parent.push(copy)
if (
copy.nodes.length > 0 ||
copy.name === '@layer' ||
copy.name === '@charset' ||
copy.name === '@custom-media' ||
copy.name === '@namespace'
Copy link
Member

@philipp-spiess philipp-spiess Jan 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this just be all at-rules? 🤔 Nevermind me here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@media (foo) {}

Should be removed imo

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah i noticed this too, you just responded before i could update my comment heh

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hehe, I think in theory we could invert the condition and don't touch everything but the @media (but then we need to check for @suspports as well.

In a perfect world @layer foo {} is also removed, (because you should use @layer foo; but we don't make that distinction.

) {
parent.push(copy)
}
}

// AtRoot
Expand Down
2 changes: 0 additions & 2 deletions packages/tailwindcss/src/compat/plugin-api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3167,8 +3167,6 @@ describe('addUtilities()', () => {
color: red;
}
}
}
:root, :host {
}"
`,
)
Expand Down
6 changes: 1 addition & 5 deletions packages/tailwindcss/src/compat/screens-config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -655,9 +655,5 @@ test('JS config `screens` can overwrite default CSS `--breakpoint-*`', async ()
// currently.
expect(
compiler.build(['min-sm:flex', 'min-md:flex', 'min-lg:flex', 'min-xl:flex', 'min-2xl:flex']),
).toMatchInlineSnapshot(`
":root, :host {
}
"
`)
).toMatchInlineSnapshot(`""`)
})