From e7ad5fbef7737674fe706a44d29876e87dc12654 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Sun, 20 Nov 2022 23:29:56 -0500 Subject: [PATCH] remove duplicate css rules across files (#2688) --- CHANGELOG.md | 17 ++++++ internal/bundler/linker.go | 64 +++++++++++++------- internal/bundler/snapshots/snapshots_css.txt | 4 -- internal/css_parser/css_parser.go | 47 +++++++++++--- internal/css_parser/css_parser_test.go | 2 +- 5 files changed, 96 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 677898ba23e..b0ed7ff4395 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/internal/bundler/linker.go b/internal/bundler/linker.go index 1118eb1a5cc..e7e1469a45d 100644 --- a/internal/bundler/linker.go +++ b/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() diff --git a/internal/bundler/snapshots/snapshots_css.txt b/internal/bundler/snapshots/snapshots_css.txt index 8ba4e73d67f..fc89af1fd6a 100644 --- a/internal/bundler/snapshots/snapshots_css.txt +++ b/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; diff --git a/internal/css_parser/css_parser.go b/internal/css_parser/css_parser.go index 00936b0be9e..6e99576e963 100644 --- a/internal/css_parser/css_parser.go +++ b/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-- diff --git a/internal/css_parser/css_parser_test.go b/internal/css_parser/css_parser_test.go index 61ff272deb4..717f79302cf 100644 --- a/internal/css_parser/css_parser_test.go +++ b/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")