From c5cf17bd081b49c7b6445c50b149427852626464 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Mon, 8 Nov 2021 22:53:01 -0500 Subject: [PATCH] fix #1755: merge adjacent selectors with same body --- CHANGELOG.md | 16 +++++++ internal/css_ast/css_ast.go | 58 ++++++++++++++------------ internal/css_parser/css_parser.go | 54 +++++++++++++++++------- internal/css_parser/css_parser_test.go | 26 +++++++----- 4 files changed, 103 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0047ddc767..113eac7abf9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,22 @@ So using `var exports = {}` should have the same effect as `exports = {}` because the variable `exports` should already be defined. However, esbuild was incorrectly overwriting the definition of the `exports` variable with the one provided by CommonJS. This release merges the definitions together so both are included, which fixes the bug. +* Merge adjacent CSS selector rules with duplicate content ([#1755](https://github.com/evanw/esbuild/issues/1755)) + + With this release, esbuild will now merge adjacent selectors when minifying if they have the same content: + + ```css + /* Original code */ + a { color: red } + b { color: red } + + /* Old output (with --minify) */ + a{color:red}b{color:red} + + /* New output (with --minify) */ + a,b{color:red} + ``` + ## 0.13.12 * Implement initial support for simplifying `calc()` expressions in CSS ([#1607](https://github.com/evanw/esbuild/issues/1607)) diff --git a/internal/css_ast/css_ast.go b/internal/css_ast/css_ast.go index abb7bc758ac..4ba2a0c91ad 100644 --- a/internal/css_ast/css_ast.go +++ b/internal/css_ast/css_ast.go @@ -369,35 +369,11 @@ type RSelector struct { func (a *RSelector) Equal(rule R) bool { b, ok := rule.(*RSelector) if ok && len(a.Selectors) == len(b.Selectors) { - for i, ai := range a.Selectors { - bi := b.Selectors[i] - if len(ai.Selectors) != len(bi.Selectors) { + for i, sel := range a.Selectors { + if !sel.Equal(b.Selectors[i]) { return false } - - for j, aj := range ai.Selectors { - bj := bi.Selectors[j] - if aj.HasNestPrefix != bj.HasNestPrefix || aj.Combinator != bj.Combinator { - return false - } - - if ats, bts := aj.TypeSelector, bj.TypeSelector; (ats == nil) != (bts == nil) { - return false - } else if ats != nil && bts != nil && !ats.Equal(*bts) { - return false - } - - if len(aj.SubclassSelectors) != len(bj.SubclassSelectors) { - return false - } - for k, ak := range aj.SubclassSelectors { - if !ak.Equal(bj.SubclassSelectors[k]) { - return false - } - } - } } - return RulesEqual(a.Rules, b.Rules) } @@ -497,6 +473,36 @@ type ComplexSelector struct { Selectors []CompoundSelector } +func (a ComplexSelector) Equal(b ComplexSelector) bool { + if len(a.Selectors) != len(b.Selectors) { + return false + } + + for i, ai := range a.Selectors { + bi := b.Selectors[i] + if ai.HasNestPrefix != bi.HasNestPrefix || ai.Combinator != bi.Combinator { + return false + } + + if ats, bts := ai.TypeSelector, bi.TypeSelector; (ats == nil) != (bts == nil) { + return false + } else if ats != nil && bts != nil && !ats.Equal(*bts) { + return false + } + + if len(ai.SubclassSelectors) != len(bi.SubclassSelectors) { + return false + } + for j, aj := range ai.SubclassSelectors { + if !aj.Equal(bi.SubclassSelectors[j]) { + return false + } + } + } + + return true +} + type CompoundSelector struct { HasNestPrefix bool // "&" Combinator string // Optional, may be "" diff --git a/internal/css_parser/css_parser.go b/internal/css_parser/css_parser.go index fbb32bfb2d1..df9f481f3f6 100644 --- a/internal/css_parser/css_parser.go +++ b/internal/css_parser/css_parser.go @@ -252,7 +252,7 @@ loop: } if p.options.MangleSyntax { - rules = removeEmptyAndDuplicateRules(rules) + rules = mangleRules(rules) } return rules } @@ -266,7 +266,7 @@ func (p *parser) parseListOfDeclarations() (list []css_ast.Rule) { case css_lexer.TEndOfFile, css_lexer.TCloseBrace: list = p.processDeclarations(list) if p.options.MangleSyntax { - list = removeEmptyAndDuplicateRules(list) + list = mangleRules(list) } return @@ -285,20 +285,14 @@ func (p *parser) parseListOfDeclarations() (list []css_ast.Rule) { } } -func removeEmptyAndDuplicateRules(rules []css_ast.Rule) []css_ast.Rule { +func mangleRules(rules []css_ast.Rule) []css_ast.Rule { type hashEntry struct { indices []uint32 } - n := len(rules) - start := n - entries := make(map[uint32]hashEntry) - - // Scan from the back so we keep the last rule -skipRule: - for i := n - 1; i >= 0; i-- { - rule := rules[i] - + // Remove empty rules + n := 0 + for _, rule := range rules { switch r := rule.Data.(type) { case *css_ast.RAtKeyframes: // Do not remove empty "@keyframe foo {}" rules. Even empty rules still @@ -316,16 +310,46 @@ skipRule: } } + rules[n] = rule + n++ + } + rules = rules[:n] + + // Remove duplicate rules, scanning from the back so we keep the last duplicate + start := n + entries := make(map[uint32]hashEntry) +skipRule: + for i := n - 1; i >= 0; i-- { + rule := rules[i] + + // Merge adjacent selectors with the same content + // "a { color: red; } b { color: red; }" => "a, b { color: red; }" + if i > 0 { + if r, ok := rule.Data.(*css_ast.RSelector); ok { + if prev, ok := rules[i-1].Data.(*css_ast.RSelector); ok && css_ast.RulesEqual(r.Rules, prev.Rules) { + nextSelector: + for _, sel := range r.Selectors { + for _, prevSel := range prev.Selectors { + if sel.Equal(prevSel) { + // Don't add duplicate selectors more than once + continue nextSelector + } + } + prev.Selectors = append(prev.Selectors, sel) + } + continue skipRule + } + } + } + + // For duplicate rules, omit all but the last copy if hash, ok := rule.Data.Hash(); ok { entry := entries[hash] - - // For duplicate rules, omit all but the last copy for _, index := range entry.indices { if rule.Data.Equal(rules[index].Data) { continue skipRule } } - entry.indices = append(entry.indices, uint32(i)) entries[hash] = entry } diff --git a/internal/css_parser/css_parser_test.go b/internal/css_parser/css_parser_test.go index 730ec89bf4f..770b619d6e1 100644 --- a/internal/css_parser/css_parser_test.go +++ b/internal/css_parser/css_parser_test.go @@ -11,13 +11,6 @@ import ( "github.com/evanw/esbuild/internal/test" ) -func assertEqual(t *testing.T, a interface{}, b interface{}) { - t.Helper() - if a != b { - t.Fatalf("%s != %s", a, b) - } -} - func expectParseError(t *testing.T, contents string, expected string) { t.Helper() t.Run(contents, func(t *testing.T) { @@ -29,7 +22,7 @@ func expectParseError(t *testing.T, contents string, expected string) { for _, msg := range msgs { text += msg.String(logger.OutputOptions{}, logger.TerminalInfo{}) } - test.AssertEqual(t, text, expected) + test.AssertEqualWithDiff(t, text, expected) }) } @@ -50,11 +43,11 @@ func expectPrintedCommon(t *testing.T, name string, contents string, expected st text += msg.String(logger.OutputOptions{}, logger.TerminalInfo{}) } } - assertEqual(t, text, "") + test.AssertEqualWithDiff(t, text, "") result := css_printer.Print(tree, css_printer.Options{ RemoveWhitespace: options.RemoveWhitespace, }) - assertEqual(t, string(result.CSS), expected) + test.AssertEqualWithDiff(t, string(result.CSS), expected) }) } @@ -1459,3 +1452,16 @@ func TestMangleAlpha(t *testing.T) { // An alpha value of 100% does not use "rgba(...)" expectPrintedLowerMangle(t, "a { color: #000000FF }", "a {\n color: #000;\n}\n") } + +func TestMangleDuplicateSelectorRules(t *testing.T) { + expectPrinted(t, "a { color: red } b { color: red }", "a {\n color: red;\n}\nb {\n color: red;\n}\n") + expectPrintedMangle(t, "a { color: red } b { color: red }", "a,\nb {\n color: red;\n}\n") + expectPrintedMangle(t, "a { color: red } div {} b { color: red }", "a,\nb {\n color: red;\n}\n") + expectPrintedMangle(t, "a { color: red } div { color: red } b { color: red }", "a,\ndiv,\nb {\n color: red;\n}\n") + expectPrintedMangle(t, "a { color: red } div { color: red } a { color: red }", "a,\ndiv {\n color: red;\n}\n") + expectPrintedMangle(t, "a { color: red } div { color: blue } b { color: red }", "a {\n color: red;\n}\ndiv {\n color: #00f;\n}\nb {\n color: red;\n}\n") + expectPrintedMangle(t, "a { color: red } div { color: blue } a { color: red }", "div {\n color: #00f;\n}\na {\n color: red;\n}\n") + expectPrintedMangle(t, "a { color: red; color: red } b { color: red }", "a,\nb {\n color: red;\n}\n") + expectPrintedMangle(t, "a { color: red } b { color: red; color: red }", "a,\nb {\n color: red;\n}\n") + expectPrintedMangle(t, "a { color: red } b { color: blue }", "a {\n color: red;\n}\nb {\n color: #00f;\n}\n") +}