Skip to content

Commit

Permalink
fix #1372: css minification bug with !important
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Jun 15, 2021
1 parent 7a05678 commit a848ad6
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 53 deletions.
13 changes: 13 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,19 @@

Note that overriding what top-level `this` is will likely break code that uses it correctly. So this new feature is only useful in certain cases.

* Fix CSS minification issue with `!important` and duplicate declarations ([#1372](https://github.com/evanw/esbuild/issues/1372))

Previously CSS with duplicate declarations for the same property where the first one was marked with `!important` was sometimes minified incorrectly. For example:

```css
.selector {
padding: 10px !important;
padding: 0;
}
```

This was incorrectly minified as `.selector{padding:0}`. The bug affected three properties: `padding`, `margin`, and `border-radius`. With this release, this code will now be minified as `.selector{padding:10px!important;padding:0}` instead which means there is no longer a difference between minified and non-minified code in this case.

## 0.12.8

* Plugins can now specify `sideEffects: false` ([#1009](https://github.com/evanw/esbuild/issues/1009))
Expand Down
26 changes: 15 additions & 11 deletions internal/css_parser/css_decls_border_radius.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ type borderRadiusCorner struct {
firstToken css_ast.Token
secondToken css_ast.Token
index uint32
important bool
single bool
}

type borderRadiusTracker struct {
corners [4]borderRadiusCorner
corners [4]borderRadiusCorner
important bool
}

func (borderRadius *borderRadiusTracker) updateCorner(rules []css_ast.R, corner int, new borderRadiusCorner) {
Expand All @@ -33,6 +33,12 @@ func (borderRadius *borderRadiusTracker) updateCorner(rules []css_ast.R, corner
}

func (borderRadius *borderRadiusTracker) mangleCorners(rules []css_ast.R, decl *css_ast.RDeclaration, index int, removeWhitespace bool) {
// Reset if we see a change in the "!important" flag
if borderRadius.important != decl.Important {
borderRadius.corners = [4]borderRadiusCorner{}
borderRadius.important = decl.Important
}

tokens := decl.Value
split := len(tokens)

Expand Down Expand Up @@ -60,7 +66,6 @@ func (borderRadius *borderRadiusTracker) mangleCorners(rules []css_ast.R, decl *
firstToken: t,
secondToken: t,
index: uint32(index),
important: decl.Important,
})
}

Expand All @@ -82,6 +87,12 @@ func (borderRadius *borderRadiusTracker) mangleCorners(rules []css_ast.R, decl *
}

func (borderRadius *borderRadiusTracker) mangleCorner(rules []css_ast.R, decl *css_ast.RDeclaration, index int, removeWhitespace bool, corner int) {
// Reset if we see a change in the "!important" flag
if borderRadius.important != decl.Important {
borderRadius.corners = [4]borderRadiusCorner{}
borderRadius.important = decl.Important
}

if tokens := decl.Value; len(tokens) == 1 || len(tokens) == 2 {
firstToken := tokens[0]
if firstToken.TurnLengthIntoNumberIfZero() {
Expand All @@ -102,7 +113,6 @@ func (borderRadius *borderRadiusTracker) mangleCorner(rules []css_ast.R, decl *c
firstToken: firstToken,
secondToken: secondToken,
index: uint32(index),
important: decl.Important,
single: true,
})
borderRadius.compactRules(rules, decl.KeyRange, removeWhitespace)
Expand All @@ -118,12 +128,6 @@ func (borderRadius *borderRadiusTracker) compactRules(rules []css_ast.R, keyRang
return
}

// All declarations must have the same "!important" state
if i := borderRadius.corners[0].important; i != borderRadius.corners[1].important ||
i != borderRadius.corners[2].important || i != borderRadius.corners[3].important {
return
}

// Generate the most minimal representation
tokens := compactTokenQuad(
borderRadius.corners[0].firstToken,
Expand Down Expand Up @@ -164,6 +168,6 @@ func (borderRadius *borderRadiusTracker) compactRules(rules []css_ast.R, keyRang
KeyText: "border-radius",
Value: tokens,
KeyRange: keyRange,
Important: borderRadius.corners[0].important,
Important: borderRadius.important,
}
}
34 changes: 20 additions & 14 deletions internal/css_parser/css_decls_box.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ const (
)

type boxSide struct {
token css_ast.Token
index uint32
important bool
single bool
token css_ast.Token
index uint32
single bool
}

type boxTracker struct {
sides [4]boxSide
sides [4]boxSide
important bool
}

func (box *boxTracker) updateSide(rules []css_ast.R, side int, new boxSide) {
Expand All @@ -32,11 +32,17 @@ func (box *boxTracker) updateSide(rules []css_ast.R, side int, new boxSide) {
}

func (box *boxTracker) mangleSides(rules []css_ast.R, decl *css_ast.RDeclaration, index int, removeWhitespace bool) {
// Reset if we see a change in the "!important" flag
if box.important != decl.Important {
box.sides = [4]boxSide{}
box.important = decl.Important
}

if n := len(decl.Value); n >= 1 && n <= 4 {
isMargin := decl.Key == css_ast.DMargin
for side, t := range expandTokenQuad(decl.Value) {
t.TurnLengthIntoNumberIfZero()
box.updateSide(rules, side, boxSide{token: t, index: uint32(index), important: decl.Important})
box.updateSide(rules, side, boxSide{token: t, index: uint32(index)})
}
box.compactRules(rules, decl.KeyRange, removeWhitespace, isMargin)
} else {
Expand All @@ -45,6 +51,12 @@ func (box *boxTracker) mangleSides(rules []css_ast.R, decl *css_ast.RDeclaration
}

func (box *boxTracker) mangleSide(rules []css_ast.R, decl *css_ast.RDeclaration, index int, removeWhitespace bool, side int) {
// Reset if we see a change in the "!important" flag
if box.important != decl.Important {
box.sides = [4]boxSide{}
box.important = decl.Important
}

if tokens := decl.Value; len(tokens) == 1 {
isMargin := false
switch decl.Key {
Expand All @@ -55,7 +67,7 @@ func (box *boxTracker) mangleSide(rules []css_ast.R, decl *css_ast.RDeclaration,
if t.TurnLengthIntoNumberIfZero() {
tokens[0] = t
}
box.updateSide(rules, side, boxSide{token: t, index: uint32(index), important: decl.Important, single: true})
box.updateSide(rules, side, boxSide{token: t, index: uint32(index), single: true})
box.compactRules(rules, decl.KeyRange, removeWhitespace, isMargin)
} else {
box.sides = [4]boxSide{}
Expand All @@ -69,12 +81,6 @@ func (box *boxTracker) compactRules(rules []css_ast.R, keyRange logger.Range, re
return
}

// All declarations must have the same "!important" state
if i := box.sides[0].important; i != box.sides[1].important ||
i != box.sides[2].important || i != box.sides[3].important {
return
}

// Generate the most minimal representation
tokens := compactTokenQuad(
box.sides[0].token,
Expand Down Expand Up @@ -105,6 +111,6 @@ func (box *boxTracker) compactRules(rules []css_ast.R, keyRange logger.Range, re
KeyText: keyText,
Value: tokens,
KeyRange: keyRange,
Important: box.sides[0].important,
Important: box.important,
}
}
65 changes: 37 additions & 28 deletions internal/css_parser/css_parser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -943,17 +943,27 @@ func TestMarginAndPadding(t *testing.T) {
expectPrintedMangle(t, "a { "+x+"-bottom: 1; "+x+"-bottom: 2 }", "a {\n "+x+"-bottom: 2;\n}\n")
expectPrintedMangle(t, "a { "+x+"-left: 1; "+x+"-left: 2 }", "a {\n "+x+"-left: 2;\n}\n")

expectPrintedMangle(t, "a { "+x+": 1; "+x+": 2 !important }", "a {\n "+x+": 2 !important;\n}\n")
expectPrintedMangle(t, "a { "+x+"-top: 1; "+x+"-top: 2 !important }", "a {\n "+x+"-top: 2 !important;\n}\n")
expectPrintedMangle(t, "a { "+x+"-right: 1; "+x+"-right: 2 !important }", "a {\n "+x+"-right: 2 !important;\n}\n")
expectPrintedMangle(t, "a { "+x+"-bottom: 1; "+x+"-bottom: 2 !important }", "a {\n "+x+"-bottom: 2 !important;\n}\n")
expectPrintedMangle(t, "a { "+x+"-left: 1; "+x+"-left: 2 !important }", "a {\n "+x+"-left: 2 !important;\n}\n")

expectPrintedMangle(t, "a { "+x+": 1 !important; "+x+": 2 }", "a {\n "+x+": 2;\n}\n")
expectPrintedMangle(t, "a { "+x+"-top: 1 !important; "+x+"-top: 2 }", "a {\n "+x+"-top: 2;\n}\n")
expectPrintedMangle(t, "a { "+x+"-right: 1 !important; "+x+"-right: 2 }", "a {\n "+x+"-right: 2;\n}\n")
expectPrintedMangle(t, "a { "+x+"-bottom: 1 !important; "+x+"-bottom: 2 }", "a {\n "+x+"-bottom: 2;\n}\n")
expectPrintedMangle(t, "a { "+x+"-left: 1 !important; "+x+"-left: 2 }", "a {\n "+x+"-left: 2;\n}\n")
expectPrintedMangle(t, "a { "+x+": 1; "+x+": 2 !important }",
"a {\n "+x+": 1;\n "+x+": 2 !important;\n}\n")
expectPrintedMangle(t, "a { "+x+"-top: 1; "+x+"-top: 2 !important }",
"a {\n "+x+"-top: 1;\n "+x+"-top: 2 !important;\n}\n")
expectPrintedMangle(t, "a { "+x+"-right: 1; "+x+"-right: 2 !important }",
"a {\n "+x+"-right: 1;\n "+x+"-right: 2 !important;\n}\n")
expectPrintedMangle(t, "a { "+x+"-bottom: 1; "+x+"-bottom: 2 !important }",
"a {\n "+x+"-bottom: 1;\n "+x+"-bottom: 2 !important;\n}\n")
expectPrintedMangle(t, "a { "+x+"-left: 1; "+x+"-left: 2 !important }",
"a {\n "+x+"-left: 1;\n "+x+"-left: 2 !important;\n}\n")

expectPrintedMangle(t, "a { "+x+": 1 !important; "+x+": 2 }",
"a {\n "+x+": 1 !important;\n "+x+": 2;\n}\n")
expectPrintedMangle(t, "a { "+x+"-top: 1 !important; "+x+"-top: 2 }",
"a {\n "+x+"-top: 1 !important;\n "+x+"-top: 2;\n}\n")
expectPrintedMangle(t, "a { "+x+"-right: 1 !important; "+x+"-right: 2 }",
"a {\n "+x+"-right: 1 !important;\n "+x+"-right: 2;\n}\n")
expectPrintedMangle(t, "a { "+x+"-bottom: 1 !important; "+x+"-bottom: 2 }",
"a {\n "+x+"-bottom: 1 !important;\n "+x+"-bottom: 2;\n}\n")
expectPrintedMangle(t, "a { "+x+"-left: 1 !important; "+x+"-left: 2 }",
"a {\n "+x+"-left: 1 !important;\n "+x+"-left: 2;\n}\n")

expectPrintedMangle(t, "a { "+x+"-top: 1; "+x+"-top: }", "a {\n "+x+"-top: 1;\n "+x+"-top:;\n}\n")
expectPrintedMangle(t, "a { "+x+"-top: 1; "+x+"-top: 2 3 }", "a {\n "+x+"-top: 1;\n "+x+"-top: 2 3;\n}\n")
Expand Down Expand Up @@ -999,23 +1009,22 @@ func TestBorderRadius(t *testing.T) {
expectPrintedMangle(t, "a { border-radius: 0/1; border-top-left-radius: 3; }", "a {\n border-radius: 3 0 0 / 3 1 1;\n}\n")
expectPrintedMangle(t, "a { border-radius: 0/1 2; border-top-left-radius: 3; }", "a {\n border-radius: 3 0 0 / 3 2 1;\n}\n")

expectPrintedMangle(t, "a { border-radius: 1; border-top-left-radius: 2 !important; }",
"a {\n border-radius: 1;\n border-top-left-radius: 2 !important;\n}\n")
expectPrintedMangle(t, "a { border-radius: 1; border-top-right-radius: 2 !important; }",
"a {\n border-radius: 1;\n border-top-right-radius: 2 !important;\n}\n")
expectPrintedMangle(t, "a { border-radius: 1; border-bottom-left-radius: 2 !important; }",
"a {\n border-radius: 1;\n border-bottom-left-radius: 2 !important;\n}\n")
expectPrintedMangle(t, "a { border-radius: 1; border-bottom-right-radius: 2 !important; }",
"a {\n border-radius: 1;\n border-bottom-right-radius: 2 !important;\n}\n")

expectPrintedMangle(t, "a { border-radius: 1 !important; border-top-left-radius: 2; }",
"a {\n border-radius: 1 !important;\n border-top-left-radius: 2;\n}\n")
expectPrintedMangle(t, "a { border-radius: 1 !important; border-top-right-radius: 2; }",
"a {\n border-radius: 1 !important;\n border-top-right-radius: 2;\n}\n")
expectPrintedMangle(t, "a { border-radius: 1 !important; border-bottom-left-radius: 2; }",
"a {\n border-radius: 1 !important;\n border-bottom-left-radius: 2;\n}\n")
expectPrintedMangle(t, "a { border-radius: 1 !important; border-bottom-right-radius: 2; }",
"a {\n border-radius: 1 !important;\n border-bottom-right-radius: 2;\n}\n")
for _, x := range []string{"", "-top-left", "-top-right", "-bottom-left", "-bottom-right"} {
y := "border" + x + "-radius"
expectPrintedMangle(t, "a { "+y+": 1; "+y+": 2 }",
"a {\n "+y+": 2;\n}\n")
expectPrintedMangle(t, "a { "+y+": 1 !important; "+y+": 2 }",
"a {\n "+y+": 1 !important;\n "+y+": 2;\n}\n")
expectPrintedMangle(t, "a { "+y+": 1; "+y+": 2 !important }",
"a {\n "+y+": 1;\n "+y+": 2 !important;\n}\n")
expectPrintedMangle(t, "a { "+y+": 1 !important; "+y+": 2 !important }",
"a {\n "+y+": 2 !important;\n}\n")

expectPrintedMangle(t, "a { border-radius: 1; "+y+": 2 !important; }",
"a {\n border-radius: 1;\n "+y+": 2 !important;\n}\n")
expectPrintedMangle(t, "a { border-radius: 1 !important; "+y+": 2; }",
"a {\n border-radius: 1 !important;\n "+y+": 2;\n}\n")
}

expectPrintedMangle(t, "a { border-top-left-radius: ; border-radius: 1 }",
"a {\n border-top-left-radius:;\n border-radius: 1;\n}\n")
Expand Down

0 comments on commit a848ad6

Please sign in to comment.