Skip to content

Commit

Permalink
css empty rule removal should happen in parser
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Mar 12, 2021
1 parent b51bc60 commit 491dd50
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 55 deletions.
1 change: 0 additions & 1 deletion internal/bundler/linker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
Expand Down
49 changes: 44 additions & 5 deletions internal/css_parser/css_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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) {
Expand All @@ -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:
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
})
}
}
}
}
Expand Down
39 changes: 38 additions & 1 deletion internal/css_parser/css_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -839,3 +849,30 @@ func TestAtRuleValidation(t *testing.T) {
"<stdin>: warning: All \"@import\" rules must come first\n"+
"<stdin>: 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}}")
}
21 changes: 0 additions & 21 deletions internal/css_printer/css_printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ type printer struct {
}

type Options struct {
MangleSyntax bool
RemoveWhitespace bool
ASCIIOnly bool
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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(" ")
Expand Down
27 changes: 0 additions & 27 deletions internal/css_printer/css_printer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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")
}

0 comments on commit 491dd50

Please sign in to comment.