remove duplicate css rules across files (#2688)

CHANGELOG.md

@@ -2,6 +2,23 @@
 
 ## Unreleased
 
+* Remove duplicate CSS rules across files ([#2688](https://github.com/evanw/esbuild/issues/2688))
+
+    When two or more CSS rules are exactly the same (even if they are not adjacent), all but the last one can safely be removed:
+
+    ```css
+    /* Before */
+    a { color: red; }
+    span { font-weight: bold; }
+    a { color: red; }
+
+    /* After */
+    span { font-weight: bold; }
+    a { color: red; }
+    ```
+
+    Previously esbuild only did this transformation within a single source file. But with this release, esbuild will now do this transformation across source files, which may lead to smaller CSS output if the same rules are repeated across multiple CSS source files in the same bundle. This transformation is only enabled when minifying (specifically when syntax minification is enabled).
+
 * Add `deno` as a valid value for `target` ([#2686](https://github.com/evanw/esbuild/issues/2686))
 
     The `target` setting in esbuild allows you to enable or disable JavaScript syntax features for a given version of a set of target JavaScript VMs. Previously [Deno](https://deno.land/) was not one of the JavaScript VMs that esbuild supported with `target`, but it will now be supported starting from this release. For example, versions of Deno older than v1.2 don't support the new `||=` operator, so adding e.g. `--target=deno1.0` to esbuild now lets you tell esbuild to transpile `||=` to older JavaScript.

internal/bundler/linker.go

@@ -15,6 +15,7 @@ import (
 	"github.com/evanw/esbuild/internal/compat"
 	"github.com/evanw/esbuild/internal/config"
 	"github.com/evanw/esbuild/internal/css_ast"
+	"github.com/evanw/esbuild/internal/css_parser"
 	"github.com/evanw/esbuild/internal/css_printer"
 	"github.com/evanw/esbuild/internal/fs"
 	"github.com/evanw/esbuild/internal/graph"
@@ -5262,32 +5263,52 @@ func (c *linkerContext) generateChunkCSS(chunks []chunkInfo, chunkIndex int, chu
 	// never change the "../" count.
 	chunkAbsDir := c.fs.Dir(c.fs.Join(c.options.AbsOutputDir, config.TemplateToString(chunk.finalTemplate)))
 
+	// Remove duplicate rules across files. This must be done in serial, not
+	// in parallel, and must be done from the last rule to the first rule.
+	timer.Begin("Prepare CSS ASTs")
+	asts := make([]css_ast.AST, len(chunkRepr.filesInChunkInOrder))
+	var remover css_parser.DuplicateRuleRemover
+	if c.options.MinifySyntax {
+		remover = css_parser.MakeDuplicateRuleMangler()
+	}
+	for i := len(chunkRepr.filesInChunkInOrder) - 1; i >= 0; i-- {
+		sourceIndex := chunkRepr.filesInChunkInOrder[i]
+		file := &c.graph.Files[sourceIndex]
+		ast := file.InputFile.Repr.(*graph.CSSRepr).AST
+
+		// Filter out "@charset" and "@import" rules
+		rules := make([]css_ast.Rule, 0, len(ast.Rules))
+		for _, rule := range ast.Rules {
+			switch rule.Data.(type) {
+			case *css_ast.RAtCharset:
+				compileResults[i].hasCharset = true
+				continue
+			case *css_ast.RAtImport:
+				continue
+			}
+			rules = append(rules, rule)
+		}
+
+		// Remove top-level duplicate rules across files
+		if c.options.MinifySyntax {
+			rules = remover.RemoveDuplicateRulesInPlace(rules)
+		}
+
+		ast.Rules = rules
+		asts[i] = ast
+	}
+	timer.End("Prepare CSS ASTs")
+
 	// Generate CSS for each file in parallel
 	timer.Begin("Print CSS files")
 	waitGroup := sync.WaitGroup{}
 	for i, sourceIndex := range chunkRepr.filesInChunkInOrder {
 		// Create a goroutine for this file
 		waitGroup.Add(1)
-		go func(sourceIndex uint32, compileResult *compileResultCSS) {
+		go func(i int, sourceIndex uint32, compileResult *compileResultCSS) {
 			defer c.recoverInternalError(&waitGroup, sourceIndex)
 
 			file := &c.graph.Files[sourceIndex]
-			ast := file.InputFile.Repr.(*graph.CSSRepr).AST
-
-			// Filter out "@charset" and "@import" rules
-			rules := make([]css_ast.Rule, 0, len(ast.Rules))
-			hasCharset := false
-			for _, rule := range ast.Rules {
-				switch rule.Data.(type) {
-				case *css_ast.RAtCharset:
-					hasCharset = true
-					continue
-				case *css_ast.RAtImport:
-					continue
-				}
-				rules = append(rules, rule)
-			}
-			ast.Rules = rules
 
 			// Only generate a source map if needed
 			var addSourceMappings bool
@@ -5307,13 +5328,10 @@ func (c *linkerContext) generateChunkCSS(chunks []chunkInfo, chunkIndex int, chu
 				InputSourceMap:    inputSourceMap,
 				LineOffsetTables:  lineOffsetTables,
 			}
-			*compileResult = compileResultCSS{
-				PrintResult: css_printer.Print(ast, cssOptions),
-				sourceIndex: sourceIndex,
-				hasCharset:  hasCharset,
-			}
+			compileResult.PrintResult = css_printer.Print(asts[i], cssOptions)
+			compileResult.sourceIndex = sourceIndex
 			waitGroup.Done()
-		}(sourceIndex, &compileResults[i])
+		}(i, sourceIndex, &compileResults[i])
 	}
 
 	waitGroup.Wait()

internal/bundler/snapshots/snapshots_css.txt

@@ -315,10 +315,6 @@ a {
 
 ---------- /out/across-files.css ----------
 /* across-files-0.css */
-a {
-  color: red;
-}
-
 /* across-files-1.css */
 a {
   color: green;

internal/css_parser/css_parser.go

@@ -289,7 +289,7 @@ loop:
 	}
 
 	if p.options.MinifySyntax {
-		rules = mangleRules(rules)
+		rules = mangleRules(rules, context.isTopLevel)
 	}
 	return rules
 }
@@ -304,7 +304,7 @@ func (p *parser) parseListOfDeclarations() (list []css_ast.Rule) {
 		case css_lexer.TEndOfFile, css_lexer.TCloseBrace:
 			list = p.processDeclarations(list)
 			if p.options.MinifySyntax {
-				list = mangleRules(list)
+				list = mangleRules(list, false /* isTopLevel */)
 			}
 			return
 
@@ -324,7 +324,7 @@ func (p *parser) parseListOfDeclarations() (list []css_ast.Rule) {
 	}
 }
 
-func mangleRules(rules []css_ast.Rule) []css_ast.Rule {
+func mangleRules(rules []css_ast.Rule, isTopLevel bool) []css_ast.Rule {
 	type hashEntry struct {
 		indices []uint32
 	}
@@ -407,23 +407,50 @@ func mangleRules(rules []css_ast.Rule) []css_ast.Rule {
 	}
 	rules = rules[:n]
 
-	// Remove duplicate rules, scanning from the back so we keep the last duplicate
+	// 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 {
+		rules = MakeDuplicateRuleMangler().RemoveDuplicateRulesInPlace(rules)
+	}
+
+	return rules
+}
+
+type hashEntry struct {
+	rules []css_ast.R
+}
+
+type DuplicateRuleRemover struct {
+	entries map[uint32]hashEntry
+}
+
+func MakeDuplicateRuleMangler() DuplicateRuleRemover {
+	return DuplicateRuleRemover{entries: make(map[uint32]hashEntry)}
+}
+
+func (remover DuplicateRuleRemover) RemoveDuplicateRulesInPlace(rules []css_ast.Rule) []css_ast.Rule {
+	// Remove duplicate rules, scanning from the back so we keep the last
+	// duplicate. Note that the linker calls this, so we do not want to do
+	// anything that modifies the rules themselves. One reason is that ASTs
+	// are immutable at the linking stage. Another reason is that merging
+	// CSS ASTs from separate files will mess up source maps because a single
+	// AST cannot simultaneously represent offsets from multiple files.
+	n := len(rules)
 	start := n
-	entries := make(map[uint32]hashEntry)
 skipRule:
 	for i := n - 1; i >= 0; i-- {
 		rule := rules[i]
 
 		// For duplicate rules, omit all but the last copy
 		if hash, ok := rule.Data.Hash(); ok {
-			entry := entries[hash]
-			for _, index := range entry.indices {
-				if rule.Data.Equal(rules[index].Data) {
+			entry := remover.entries[hash]
+			for _, data := range entry.rules {
+				if rule.Data.Equal(data) {
 					continue skipRule
 				}
 			}
-			entry.indices = append(entry.indices, uint32(i))
-			entries[hash] = entry
+			entry.rules = append(entry.rules, rule.Data)
+			remover.entries[hash] = entry
 		}
 
 		start--

internal/css_parser/css_parser_test.go

@@ -1750,7 +1750,7 @@ func TestMangleDuplicateSelectorRules(t *testing.T) {
 	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 } div { color: blue } a { color: red }", "a {\n  color: red;\n}\ndiv {\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")