From 78b59e6710814eade863f8a0f256b385bbae30a2 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Fri, 11 Aug 2023 16:49:45 -0400 Subject: [PATCH] fix #3226: unwrap nested duplicate `@media` rules --- CHANGELOG.md | 22 +++++++++ internal/css_parser/css_parser.go | 62 +++++++++++++++++++++++--- internal/css_parser/css_parser_test.go | 11 +++++ 3 files changed, 89 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ec035ff9bd..f21226307d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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)) diff --git a/internal/css_parser/css_parser.go b/internal/css_parser/css_parser.go index 2b17bab45fb..797fd3bdda2 100644 --- a/internal/css_parser/css_parser.go +++ b/internal/css_parser/css_parser.go @@ -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 @@ -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 @@ -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 @@ -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 { @@ -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, @@ -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{} diff --git a/internal/css_parser/css_parser_test.go b/internal/css_parser/css_parser_test.go index ba0263f6d5b..9571d6f9b87 100644 --- a/internal/css_parser/css_parser_test.go +++ b/internal/css_parser/css_parser_test.go @@ -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", "")