fix #1538: minify bug for "var()" and "box-shadow"

CHANGELOG.md

@@ -8,6 +8,23 @@
 
     Previously this was implemented inefficiently for files that aren't part of the build, but that are read from the underlying fallback directory. In that case the entire file was being read even though only part of the file was needed. In this release, only the part of the file that is needed is read so using HTTP range requests with esbuild in this case will now use less memory.
 
+* Fix CSS minification bug with `box-shadow` and `var()` ([#1538](https://github.com/evanw/esbuild/issues/1538))
+
+    The `box-shadow` property can be specified using 2, 3, or 4 numbers. The 3rd and 4th numbers are the blur radius and spread radius, and can be omitted if zero. When minifying, esbuild has an optimization that removes trailing zeros from runs of numbers within the `box-shadow` property. However, that optimization is not correct in the presence of tokens that are neither a number, a color, nor the token `insert`. These edge cases include `var()` or `calc()` tokens. With this release, esbuild will now do stronger validation and will only remove trailing zeros if the contents of the `box-shadow` property matches the underlying CSS grammar exactly.
+
+    ```css
+    /* Original code */
+    button {
+      box-shadow: 0 0 0 var(--spread) red;
+    }
+
+    /* Old minified output */
+    button{box-shadow:0 0 var(--spread) red}
+
+    /* New minified output */
+    button{box-shadow:0 0 0 var(--spread) red}
+    ```
+
 ## 0.12.21
 
 * Add support for native esbuild on Windows 64-bit ARM ([#995](https://github.com/evanw/esbuild/issues/995))

internal/css_parser/css_decls.go

@@ -114,7 +114,10 @@ func (p *parser) processDeclarations(rules []css_ast.Rule) []css_ast.Rule {
 				decl.Value[0] = p.lowerColor(decl.Value[0])
 
 				if p.options.MangleSyntax {
-					decl.Value[0] = p.mangleColor(decl.Value[0])
+					t := decl.Value[0]
+					if hex, ok := parseColor(t); ok {
+						decl.Value[0] = p.mangleColor(t, hex)
+					}
 				}
 			}
 

internal/css_parser/css_decls_box_shadow.go

@@ -6,62 +6,68 @@ import (
 )
 
 func (p *parser) mangleBoxShadow(tokens []css_ast.Token) []css_ast.Token {
-	n := len(tokens)
-	end := 0
-	i := 0
+	insetCount := 0
+	colorCount := 0
+	numbersBegin := 0
+	numbersCount := 0
+	numbersDone := false
+	foundUnexpectedToken := false
 
-	for i < n {
-		t := tokens[i]
-
-		// Parse a run of numbers
+	for i, t := range tokens {
 		if t.Kind == css_lexer.TNumber || t.Kind == css_lexer.TDimension {
-			runStart := i
-			for i < n {
-				t := tokens[i]
-				if t.Kind != css_lexer.TNumber && t.Kind != css_lexer.TDimension {
-					break
-				}
-				if t.TurnLengthIntoNumberIfZero() {
-					tokens[i] = t
-				}
-				i++
+			if numbersDone {
+				// Track if we found a non-number in between two numbers
+				foundUnexpectedToken = true
 			}
-
-			// Trim trailing zeros. There are three valid configurations:
-			//
-			//   offset-x | offset-y
-			//   offset-x | offset-y | blur-radius
-			//   offset-x | offset-y | blur-radius | spread-radius
-			//
-			// If omitted, blur-radius and spread-radius are implied to be zero.
-			runEnd := i
-			for runEnd > runStart+2 {
-				t := tokens[runEnd-1]
-				if t.Kind != css_lexer.TNumber || t.Text != "0" {
-					break
-				}
-				runEnd--
+			if t.TurnLengthIntoNumberIfZero() {
+				// "0px" => "0"
+				tokens[i] = t
+			}
+			if numbersCount == 0 {
+				// Track the index of the first number
+				numbersBegin = i
+			}
+			numbersCount++
+		} else {
+			if numbersCount != 0 {
+				// Track when we find a non-number after a number
+				numbersDone = true
+			}
+			if hex, ok := parseColor(t); ok {
+				colorCount++
+				tokens[i] = p.mangleColor(t, hex)
+			} else if t.Kind == css_lexer.TIdent && t.Text == "inset" {
+				insetCount++
+			} else {
+				// Track if we found a token other than a number, a color, or "inset"
+				foundUnexpectedToken = true
 			}
-
-			// Copy over the remaining tokens
-			end += copy(tokens[end:], tokens[runStart:runEnd])
-			continue
 		}
+	}
 
-		t = p.mangleColor(t)
-		tokens[end] = t
-		end++
-		i++
+	// If everything looks like a valid rule, trim trailing zeros off the numbers.
+	// There are three valid configurations of numbers:
+	//
+	//   offset-x | offset-y
+	//   offset-x | offset-y | blur-radius
+	//   offset-x | offset-y | blur-radius | spread-radius
+	//
+	// If omitted, blur-radius and spread-radius are implied to be zero.
+	if insetCount <= 1 && colorCount <= 1 && numbersCount > 2 && numbersCount <= 4 && !foundUnexpectedToken {
+		numbersEnd := numbersBegin + numbersCount
+		for numbersCount > 2 && tokens[numbersBegin+numbersCount-1].IsZero() {
+			numbersCount--
+		}
+		tokens = append(tokens[:numbersBegin+numbersCount], tokens[numbersEnd:]...)
 	}
 
 	// Set the whitespace flags
-	tokens = tokens[:end]
 	for i := range tokens {
 		var whitespace css_ast.WhitespaceFlags
 		if i > 0 || !p.options.RemoveWhitespace {
 			whitespace |= css_ast.WhitespaceBefore
 		}
-		if i+1 < end {
+		if i+1 < len(tokens) {
 			whitespace |= css_ast.WhitespaceAfter
 		}
 		tokens[i].Whitespace = whitespace

internal/css_parser/css_decls_color.go

@@ -600,53 +600,51 @@ func parseColorByte(token css_ast.Token, scale float64) (uint32, bool) {
 	return uint32(i), ok
 }
 
-func (p *parser) mangleColor(token css_ast.Token) css_ast.Token {
+func (p *parser) mangleColor(token css_ast.Token, hex uint32) css_ast.Token {
 	// Note: Do NOT remove color information from fully transparent colors.
 	// Safari behaves differently than other browsers for color interpolation:
 	// https://css-tricks.com/thing-know-gradients-transparent-black/
 
-	if hex, ok := parseColor(token); ok {
-		if hexA(hex) == 255 {
-			token.Children = nil
-			if name, ok := shortColorName[hex]; ok {
-				token.Kind = css_lexer.TIdent
-				token.Text = name
-			} else {
-				token.Kind = css_lexer.THash
-				hex >>= 8
-				compact := compactHex(hex)
-				if hex == expandHex(compact) {
-					token.Text = fmt.Sprintf("%03x", compact)
-				} else {
-					token.Text = fmt.Sprintf("%06x", hex)
-				}
-			}
-		} else if !p.options.UnsupportedCSSFeatures.Has(compat.HexRGBA) {
-			token.Children = nil
+	if hexA(hex) == 255 {
+		token.Children = nil
+		if name, ok := shortColorName[hex]; ok {
+			token.Kind = css_lexer.TIdent
+			token.Text = name
+		} else {
 			token.Kind = css_lexer.THash
+			hex >>= 8
 			compact := compactHex(hex)
 			if hex == expandHex(compact) {
-				token.Text = fmt.Sprintf("%04x", compact)
+				token.Text = fmt.Sprintf("%03x", compact)
 			} else {
-				token.Text = fmt.Sprintf("%08x", hex)
+				token.Text = fmt.Sprintf("%06x", hex)
 			}
+		}
+	} else if !p.options.UnsupportedCSSFeatures.Has(compat.HexRGBA) {
+		token.Children = nil
+		token.Kind = css_lexer.THash
+		compact := compactHex(hex)
+		if hex == expandHex(compact) {
+			token.Text = fmt.Sprintf("%04x", compact)
 		} else {
-			token.Kind = css_lexer.TFunction
-			token.Text = "rgba"
-			commaToken := p.commaToken()
-			alpha := floatToString(float64(hexA(hex)) / 255)
-			if p.options.MangleSyntax {
-				if text, ok := mangleNumber(alpha); ok {
-					alpha = text
-				}
-			}
-			token.Children = &[]css_ast.Token{
-				{Kind: css_lexer.TNumber, Text: strconv.Itoa(hexR(hex))}, commaToken,
-				{Kind: css_lexer.TNumber, Text: strconv.Itoa(hexG(hex))}, commaToken,
-				{Kind: css_lexer.TNumber, Text: strconv.Itoa(hexB(hex))}, commaToken,
-				{Kind: css_lexer.TNumber, Text: alpha},
+			token.Text = fmt.Sprintf("%08x", hex)
+		}
+	} else {
+		token.Kind = css_lexer.TFunction
+		token.Text = "rgba"
+		commaToken := p.commaToken()
+		alpha := floatToString(float64(hexA(hex)) / 255)
+		if p.options.MangleSyntax {
+			if text, ok := mangleNumber(alpha); ok {
+				alpha = text
 			}
 		}
+		token.Children = &[]css_ast.Token{
+			{Kind: css_lexer.TNumber, Text: strconv.Itoa(hexR(hex))}, commaToken,
+			{Kind: css_lexer.TNumber, Text: strconv.Itoa(hexG(hex))}, commaToken,
+			{Kind: css_lexer.TNumber, Text: strconv.Itoa(hexB(hex))}, commaToken,
+			{Kind: css_lexer.TNumber, Text: alpha},
+		}
 	}
 
 	return token

internal/css_parser/css_parser_test.go

@@ -1068,6 +1068,10 @@ func TestBoxShadow(t *testing.T) {
 	expectPrintedMangle(t, "a { box-shadow: yellow 1px 0px 0px 1px inset }", "a {\n  box-shadow: #ff0 1px 0 0 1px inset;\n}\n")
 	expectPrintedMangle(t, "a { box-shadow: yellow 1px 0px 1px 0px inset }", "a {\n  box-shadow: #ff0 1px 0 1px inset;\n}\n")
 	expectPrintedMangle(t, "a { box-shadow: rebeccapurple, yellow, black }", "a {\n  box-shadow:\n    #639,\n    #ff0,\n    #000;\n}\n")
+	expectPrintedMangle(t, "a { box-shadow: 0px 0px 0px var(--foo) black }", "a {\n  box-shadow: 0 0 0 var(--foo) #000;\n}\n")
+	expectPrintedMangle(t, "a { box-shadow: 0px 0px 0px 0px var(--foo) black }", "a {\n  box-shadow: 0 0 0 0 var(--foo) #000;\n}\n")
+	expectPrintedMangle(t, "a { box-shadow: calc(1px + var(--foo)) 0px 0px 0px black }", "a {\n  box-shadow: calc(1px + var(--foo)) 0 0 0 #000;\n}\n")
+	expectPrintedMangle(t, "a { box-shadow: inset 0px 0px 0px 0px 0px magenta; }", "a {\n  box-shadow: inset 0 0 0 0 0 #f0f;\n}\n")
 	expectPrintedMangleMinify(t, "a { box-shadow: rebeccapurple , yellow , black }", "a{box-shadow:#639,#ff0,#000}")
 	expectPrintedMangleMinify(t, "a { box-shadow: rgb(255, 0, 17) 0 0 1 inset }", "a{box-shadow:#f01 0 0 1 inset}")
 }