From 491dd5077053b9c323664d620f034a60f2c5593e Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Fri, 12 Mar 2021 00:50:30 -0800 Subject: [PATCH] css empty rule removal should happen in parser --- internal/bundler/linker.go | 1 - internal/css_parser/css_parser.go | 49 +++++++++++++++++++++--- internal/css_parser/css_parser_test.go | 39 ++++++++++++++++++- internal/css_printer/css_printer.go | 21 ---------- internal/css_printer/css_printer_test.go | 27 ------------- 5 files changed, 82 insertions(+), 55 deletions(-) diff --git a/internal/bundler/linker.go b/internal/bundler/linker.go index be7fe83886b..985a45edc79 100644 --- a/internal/bundler/linker.go +++ b/internal/bundler/linker.go @@ -4181,7 +4181,6 @@ func (repr *chunkReprCSS) generate(c *linkerContext, chunk *chunkInfo) func(gene ast.Rules = rules compileResult.printedCSS = css_printer.Print(ast, css_printer.Options{ - MangleSyntax: c.options.MangleSyntax, RemoveWhitespace: c.options.RemoveWhitespace, ASCIIOnly: c.options.ASCIIOnly, }) diff --git a/internal/css_parser/css_parser.go b/internal/css_parser/css_parser.go index a0a7f726dab..8efa15879d3 100644 --- a/internal/css_parser/css_parser.go +++ b/internal/css_parser/css_parser.go @@ -158,10 +158,11 @@ func (p *parser) parseListOfRules(context ruleContext) []css_ast.R { rules := []css_ast.R{} locs := []logger.Loc{} +loop: for { switch p.current().Kind { case css_lexer.TEndOfFile, css_lexer.TCloseBrace: - return rules + break loop case css_lexer.TWhitespace: p.advance() @@ -222,6 +223,11 @@ func (p *parser) parseListOfRules(context ruleContext) []css_ast.R { rules = append(rules, p.parseQualifiedRuleFrom(p.index, false /* isAlreadyInvalid */)) } } + + if p.options.MangleSyntax { + rules = removeEmptyRules(rules) + } + return rules } func (p *parser) parseListOfDeclarations() (list []css_ast.R) { @@ -232,6 +238,9 @@ func (p *parser) parseListOfDeclarations() (list []css_ast.R) { case css_lexer.TEndOfFile, css_lexer.TCloseBrace: p.processDeclarations(list) + if p.options.MangleSyntax { + list = removeEmptyRules(list) + } return case css_lexer.TAtKeyword: @@ -249,6 +258,32 @@ func (p *parser) parseListOfDeclarations() (list []css_ast.R) { } } +func removeEmptyRules(rules []css_ast.R) []css_ast.R { + end := 0 + for _, rule := range rules { + switch r := rule.(type) { + case *css_ast.RAtKeyframes: + if len(r.Blocks) == 0 { + continue + } + + case *css_ast.RKnownAt: + if len(r.Rules) == 0 { + continue + } + + case *css_ast.RSelector: + if len(r.Rules) == 0 { + continue + } + } + + rules[end] = rule + end++ + } + return rules[:end] +} + func (p *parser) parseURLOrString() (string, logger.Range, bool) { t := p.current() switch t.Kind { @@ -460,10 +495,14 @@ func (p *parser) parseAtRule(context atRuleContext) css_ast.R { if p.expect(css_lexer.TOpenBrace) { rules := p.parseListOfDeclarations() p.expect(css_lexer.TCloseBrace) - blocks = append(blocks, css_ast.KeyframeBlock{ - Selectors: selectors, - Rules: rules, - }) + + // "@keyframes { from {} to { color: red } }" => "@keyframes { to { color: red } }" + if !p.options.MangleSyntax || len(rules) > 0 { + blocks = append(blocks, css_ast.KeyframeBlock{ + Selectors: selectors, + Rules: rules, + }) + } } } } diff --git a/internal/css_parser/css_parser_test.go b/internal/css_parser/css_parser_test.go index c6c767cd330..94da3831dd7 100644 --- a/internal/css_parser/css_parser_test.go +++ b/internal/css_parser/css_parser_test.go @@ -50,7 +50,9 @@ func expectPrintedCommon(t *testing.T, name string, contents string, expected st } } assertEqual(t, text, "") - css := css_printer.Print(tree, css_printer.Options{}) + css := css_printer.Print(tree, css_printer.Options{ + RemoveWhitespace: options.RemoveWhitespace, + }) assertEqual(t, string(css), expected) }) } @@ -82,6 +84,14 @@ func expectPrintedLowerMangle(t *testing.T, contents string, expected string) { }) } +func expectPrintedMangleMinify(t *testing.T, contents string, expected string) { + t.Helper() + expectPrintedCommon(t, contents+" [mangle, minify]", contents, expected, config.Options{ + MangleSyntax: true, + RemoveWhitespace: true, + }) +} + func TestEscapes(t *testing.T) { // TIdent expectPrinted(t, "a { value: id\\65nt }", "a {\n value: ident;\n}\n") @@ -839,3 +849,30 @@ func TestAtRuleValidation(t *testing.T) { ": warning: All \"@import\" rules must come first\n"+ ": note: This rule cannot come before an \"@import\" rule\n") } + +func TestEmptyRule(t *testing.T) { + expectPrinted(t, "div {}", "div {\n}\n") + expectPrinted(t, "@media screen {}", "@media screen {\n}\n") + expectPrinted(t, "@page { @top-left {} }", "@page {\n @top-left {\n }\n}\n") + expectPrinted(t, "@keyframes test { from {} to {} }", "@keyframes test {\n from {\n }\n to {\n }\n}\n") + + expectPrintedMangle(t, "div {}", "") + expectPrintedMangle(t, "@media screen {}", "") + expectPrintedMangle(t, "@page { @top-left {} }", "") + expectPrintedMangle(t, "@keyframes test { from {} to {} }", "") + + expectPrinted(t, "$invalid {}", "$invalid {\n}\n") + expectPrinted(t, "@page { color: red; @top-left {} }", "@page {\n color: red;\n @top-left {\n }\n}\n") + expectPrinted(t, "@keyframes test { from {} to { color: red } }", "@keyframes test {\n from {\n }\n to {\n color: red;\n }\n}\n") + expectPrinted(t, "@keyframes test { from { color: red } to {} }", "@keyframes test {\n from {\n color: red;\n }\n to {\n }\n}\n") + + expectPrintedMangle(t, "$invalid {}", "$invalid {\n}\n") + expectPrintedMangle(t, "@page { color: red; @top-left {} }", "@page {\n color: red;\n}\n") + expectPrintedMangle(t, "@keyframes test { from {} to { color: red } }", "@keyframes test {\n to {\n color: red;\n }\n}\n") + expectPrintedMangle(t, "@keyframes test { from { color: red } to {} }", "@keyframes test {\n 0% {\n color: red;\n }\n}\n") + + expectPrintedMangleMinify(t, "$invalid {}", "$invalid{}") + expectPrintedMangleMinify(t, "@page { color: red; @top-left {} }", "@page{color:red}") + expectPrintedMangleMinify(t, "@keyframes test { from {} to { color: red } }", "@keyframes test{to{color:red}}") + expectPrintedMangleMinify(t, "@keyframes test { from { color: red } to {} }", "@keyframes test{0%{color:red}}") +} diff --git a/internal/css_printer/css_printer.go b/internal/css_printer/css_printer.go index 8ac2ecd316a..ccd81f387c3 100644 --- a/internal/css_printer/css_printer.go +++ b/internal/css_printer/css_printer.go @@ -19,7 +19,6 @@ type printer struct { } type Options struct { - MangleSyntax bool RemoveWhitespace bool ASCIIOnly bool } @@ -78,11 +77,6 @@ func (p *printer) printRule(rule css_ast.R, indent int32, omitTrailingSemicolon } indent++ for _, block := range r.Blocks { - // "@keyframes { from {} to { color: red } }" => "@keyframes { to { color: red } }" - if p.options.MangleSyntax && len(block.Rules) == 0 { - continue - } - if !p.options.RemoveWhitespace { p.printIndent(indent) } @@ -111,16 +105,6 @@ func (p *printer) printRule(rule css_ast.R, indent int32, omitTrailingSemicolon p.print("}") case *css_ast.RKnownAt: - // "@font-face {}" => "" - // "@page {}" => "" - // "@document {}" => "" - // "@media {}" => "" - // "@scope {}" => "" - // "@supports {}" => "" - if p.options.MangleSyntax && len(r.Rules) == 0 { - return - } - p.print("@") whitespace := mayNeedWhitespaceAfter if len(r.Prelude) == 0 { @@ -157,11 +141,6 @@ func (p *printer) printRule(rule css_ast.R, indent int32, omitTrailingSemicolon } case *css_ast.RSelector: - // "a {}" => "" - if p.options.MangleSyntax && len(r.Rules) == 0 { - return - } - p.printComplexSelectors(r.Selectors, indent) if !p.options.RemoveWhitespace { p.print(" ") diff --git a/internal/css_printer/css_printer_test.go b/internal/css_printer/css_printer_test.go index e0cdabde30b..276d52bde23 100644 --- a/internal/css_printer/css_printer_test.go +++ b/internal/css_printer/css_printer_test.go @@ -48,13 +48,6 @@ func expectPrintedMinify(t *testing.T, contents string, expected string) { }) } -func expectPrintedMangle(t *testing.T, contents string, expected string) { - t.Helper() - expectPrintedCommon(t, contents+" [mangled]", contents, expected, Options{ - MangleSyntax: true, - }) -} - func expectPrintedASCII(t *testing.T, contents string, expected string) { t.Helper() expectPrintedCommon(t, contents+" [ascii]", contents, expected, Options{ @@ -399,23 +392,3 @@ func TestASCII(t *testing.T) { // This character should always be escaped expectPrinted(t, ".\\FEFF:after { content: '\uFEFF' }", ".\\feff:after {\n content: \"\\feff\";\n}\n") } - -func TestEmptyRule(t *testing.T) { - expectPrinted(t, "div {}", "div {\n}\n") - expectPrinted(t, "$invalid {}", "$invalid {\n}\n") - expectPrinted(t, "@media screen {}", "@media screen {\n}\n") - expectPrinted(t, "@keyframes test { from {} to { color: red } }", "@keyframes test {\n from {\n }\n to {\n color: red;\n }\n}\n") - expectPrinted(t, "@keyframes test { from { color: red } to {} }", "@keyframes test {\n from {\n color: red;\n }\n to {\n }\n}\n") - - expectPrintedMinify(t, "div {}", "div{}") - expectPrintedMinify(t, "$invalid {}", "$invalid{}") - expectPrintedMinify(t, "@media screen {}", "@media screen{}") - expectPrintedMinify(t, "@keyframes test { from {} to { color: red } }", "@keyframes test{from{}to{color:red}}") - expectPrintedMinify(t, "@keyframes test { from { color: red } to {} }", "@keyframes test{from{color:red}to{}}") - - expectPrintedMangle(t, "div {}", "") - expectPrintedMangle(t, "$invalid {}", "$invalid {\n}\n") - expectPrintedMangle(t, "@media screen {}", "") - expectPrintedMangle(t, "@keyframes test { from {} to { color: red } }", "@keyframes test {\n to {\n color: red;\n }\n}\n") - expectPrintedMangle(t, "@keyframes test { from { color: red } to {} }", "@keyframes test {\n from {\n color: red;\n }\n}\n") -}