Skip to content

Commit

Permalink
fix #3226: unwrap nested duplicate @media rules
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Aug 11, 2023
1 parent 009beb1 commit 78b59e6
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 6 deletions.
22 changes: 22 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,28 @@

Previously esbuild's generated names for local names in CSS were only unique within a given entry point (or across all entry points when code splitting was enabled). That meant that building multiple entry points with esbuild could result in local names being renamed to the same identifier even when those entry points were built simultaneously within a single esbuild API call. This problem was especially likely to happen with minification enabled. With this release, esbuild will now avoid renaming local names from two separate entry points to the same name if those entry points were built with a single esbuild API call, even when code splitting is disabled.

* Unwrap nested duplicate `@media` rules ([#3226](https://github.com/evanw/esbuild/issues/3226))

With this release, esbuild's CSS minifier will now automatically unwrap duplicate nested `@media` rules:

```css
/* Original code */
@media (min-width: 1024px) {
.foo { color: red }
@media (min-width: 1024px) {
.bar { color: blue }
}
}

/* Old output (with --minify) */
@media (min-width: 1024px){.foo{color:red}@media (min-width: 1024px){.bar{color:#00f}}}

/* New output (with --minify) */
@media (min-width: 1024px){.foo{color:red}.bar{color:#00f}}
```

These rules are unlikely to be authored manually but may result from using frameworks such as Tailwind to generate CSS.

## 0.19.1

* Fix a regression with `baseURL` in `tsconfig.json` ([#3307](https://github.com/evanw/esbuild/issues/3307))
Expand Down
62 changes: 56 additions & 6 deletions internal/css_parser/css_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type parser struct {
globalScope map[string]ast.LocRef
nestingWarnings map[logger.Loc]struct{}
tracker logger.LineColumnTracker
enclosingAtMedia [][]css_ast.Token
layers [][]string
enclosingLayer []string
anonLayerCount int
Expand Down Expand Up @@ -591,8 +592,9 @@ func (p *parser) mangleRules(rules []css_ast.Rule, isTopLevel bool) []css_ast.Ru
}

// Remove empty rules
mangledRules := make([]css_ast.Rule, 0, len(rules))
var prevNonComment css_ast.R
n := 0
next:
for _, rule := range rules {
nextNonComment := rule.Data

Expand Down Expand Up @@ -629,6 +631,42 @@ func (p *parser) mangleRules(rules []css_ast.Rule, isTopLevel bool) []css_ast.Ru
continue
}

// Unwrap "@media" rules that duplicate conditions from a parent "@media"
// rule. This is unlikely to be authored manually but can be automatically
// generated when using a CSS framework such as Tailwind.
//
// @media (min-width: 1024px) {
// .md\:class {
// color: red;
// }
// @media (min-width: 1024px) {
// .md\:class {
// color: red;
// }
// }
// }
//
// This converts that code into the following:
//
// @media (min-width: 1024px) {
// .md\:class {
// color: red;
// }
// .md\:class {
// color: red;
// }
// }
//
// Which can then be mangled further.
if r.AtToken == "media" {
for _, prelude := range p.enclosingAtMedia {
if css_ast.TokensEqualIgnoringWhitespace(r.Prelude, prelude) {
mangledRules = append(mangledRules, r.Rules...)
continue next
}
}
}

case *css_ast.RSelector:
if len(r.Rules) == 0 {
continue
Expand Down Expand Up @@ -663,19 +701,17 @@ func (p *parser) mangleRules(rules []css_ast.Rule, isTopLevel bool) []css_ast.Ru
prevNonComment = nextNonComment
}

rules[n] = rule
n++
mangledRules = append(mangledRules, rule)
}
rules = rules[:n]

// Mangle non-top-level rules using a back-to-front pass. Top-level rules
// will be mangled by the linker instead for cross-file rule mangling.
if !isTopLevel {
remover := MakeDuplicateRuleMangler(ast.SymbolMap{})
rules = remover.RemoveDuplicateRulesInPlace(p.source.Index, rules, p.importRecords)
mangledRules = remover.RemoveDuplicateRulesInPlace(p.source.Index, mangledRules, p.importRecords)
}

return rules
return mangledRules
}

type ruleEntry struct {
Expand Down Expand Up @@ -1550,6 +1586,14 @@ prelude:
matchingLoc := p.current().Range.Loc
p.expect(css_lexer.TOpenBrace)
var rules []css_ast.Rule

// Push the "@media" conditions
isAtMedia := atToken == "media"
if isAtMedia {
p.enclosingAtMedia = append(p.enclosingAtMedia, prelude)
}

// Parse the block for this rule
if context.isDeclarationList {
rules = p.parseListOfDeclarations(listOfDeclarationsOpts{
canInlineNoOpNesting: context.canInlineNoOpNesting,
Expand All @@ -1559,6 +1603,12 @@ prelude:
parseSelectors: true,
})
}

// Pop the "@media" conditions
if isAtMedia {
p.enclosingAtMedia = p.enclosingAtMedia[:len(p.enclosingAtMedia)-1]
}

closeBraceLoc := p.current().Range.Loc
if !p.expectWithMatchingLoc(css_lexer.TCloseBrace, matchingLoc) {
closeBraceLoc = logger.Loc{}
Expand Down
11 changes: 11 additions & 0 deletions internal/css_parser/css_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2151,6 +2151,17 @@ func TestMangleDuplicateSelectorRules(t *testing.T) {
expectPrintedMangle(t, "c { color: green } a { color: red } /*!x*/ /*!y*/ a { color: red }", "c {\n color: green;\n}\na {\n color: red;\n}\n/*!x*/\n/*!y*/\n", "")
}

func TestMangleAtMedia(t *testing.T) {
expectPrinted(t, "@media screen { @media screen { a { color: red } } }", "@media screen {\n @media screen {\n a {\n color: red;\n }\n }\n}\n", "")
expectPrintedMangle(t, "@media screen { @media screen { a { color: red } } }", "@media screen {\n a {\n color: red;\n }\n}\n", "")
expectPrintedMangle(t, "@media screen { @media not print { a { color: red } } }", "@media screen {\n @media not print {\n a {\n color: red;\n }\n }\n}\n", "")
expectPrintedMangle(t, "@media screen { @media not print { @media screen { a { color: red } } } }", "@media screen {\n @media not print {\n a {\n color: red;\n }\n }\n}\n", "")
expectPrintedMangle(t, "@media screen { a { color: red } @media screen { a { color: red } } }", "@media screen {\n a {\n color: red;\n }\n}\n", "")
expectPrintedMangle(t, "@media screen { a { color: red } @media screen { a { color: blue } } }", "@media screen {\n a {\n color: red;\n }\n a {\n color: #00f;\n }\n}\n", "")
expectPrintedMangle(t, "@media screen { .a { color: red; @media screen { .b { color: blue } } } }", "@media screen {\n .a {\n color: red;\n .b {\n color: #00f;\n }\n }\n}\n", "")
expectPrintedMangle(t, "@media screen { a { color: red } } @media screen { b { color: red } }", "@media screen {\n a {\n color: red;\n }\n}\n@media screen {\n b {\n color: red;\n }\n}\n", "")
}

func TestFontWeight(t *testing.T) {
expectPrintedMangle(t, "a { font-weight: normal }", "a {\n font-weight: 400;\n}\n", "")
expectPrintedMangle(t, "a { font-weight: bold }", "a {\n font-weight: 700;\n}\n", "")
Expand Down

0 comments on commit 78b59e6

Please sign in to comment.