fix #1755: merge adjacent selectors with same body

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))

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 ""

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
 		}

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")
+}