diff --git a/CHANGELOG.md b/CHANGELOG.md index df34109055a..ae4ba6f0d8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,37 @@ # Changelog +## Unreleased + +* Fix invalid CSS minification of `border-radius` ([#1702](https://github.com/evanw/esbuild/issues/1702)) + + CSS minification does collapsing of `border-radius` related properties. For example: + + ```css + /* Original CSS */ + div { + border-radius: 1px; + border-top-left-radius: 5px; + } + + /* Minified CSS */ + div{border-radius:5px 1px 1px} + ``` + + However, this only works for numeric tokens, not identifiers. For example: + + ```css + /* Original CSS */ + div { + border-radius: 1px; + border-top-left-radius: inherit; + } + + /* Minified CSS */ + div{border-radius:1px;border-top-left-radius:inherit} + ``` + + Transforming this to `div{border-radius:inherit 1px 1px}`, as was done in previous releases of esbuild, is an invalid transformation and results in incorrect CSS. This release of esbuild fixes this CSS transformation bug. + ## 0.13.8 * Fix `super` inside arrow function inside lowered `async` function ([#1425](https://github.com/evanw/esbuild/issues/1425)) diff --git a/internal/css_lexer/css_lexer.go b/internal/css_lexer/css_lexer.go index 93c0fb84f11..3e8ae6ffa0b 100644 --- a/internal/css_lexer/css_lexer.go +++ b/internal/css_lexer/css_lexer.go @@ -104,10 +104,6 @@ func (t T) IsNumeric() bool { return t == TNumber || t == TPercentage || t == TDimension } -func (t T) IsNumericOrIdent() bool { - return t.IsNumeric() || t == TIdent -} - // This token struct is designed to be memory-efficient. It just references a // range in the input file instead of directly containing the substring of text // since a range takes up less memory than a string. diff --git a/internal/css_parser/css_decls.go b/internal/css_parser/css_decls.go index cf34bae70cf..ebb4369dbb0 100644 --- a/internal/css_parser/css_decls.go +++ b/internal/css_parser/css_decls.go @@ -16,7 +16,7 @@ func (p *parser) commaToken() css_ast.Token { return t } -func expandTokenQuad(tokens []css_ast.Token) (result [4]css_ast.Token, ok bool) { +func expandTokenQuad(tokens []css_ast.Token, allowedIdent string) (result [4]css_ast.Token, ok bool) { n := len(tokens) if n < 1 || n > 4 { return @@ -24,7 +24,7 @@ func expandTokenQuad(tokens []css_ast.Token) (result [4]css_ast.Token, ok bool) // Don't do this if we encounter any unexpected tokens such as "var()" for i := 0; i < n; i++ { - if !tokens[i].Kind.IsNumericOrIdent() { + if t := tokens[i]; !t.Kind.IsNumeric() && (t.Kind != css_lexer.TIdent || allowedIdent == "" || t.Text != allowedIdent) { return } } diff --git a/internal/css_parser/css_decls_border_radius.go b/internal/css_parser/css_decls_border_radius.go index 8f2b6f9dd82..0eb7a40e3e2 100644 --- a/internal/css_parser/css_decls_border_radius.go +++ b/internal/css_parser/css_decls_border_radius.go @@ -57,8 +57,8 @@ func (borderRadius *borderRadiusTracker) mangleCorners(rules []css_ast.Rule, dec } } - firstRadii, firstRadiiOk := expandTokenQuad(tokens[:beforeSplit]) - lastRadii, lastRadiiOk := expandTokenQuad(tokens[afterSplit:]) + firstRadii, firstRadiiOk := expandTokenQuad(tokens[:beforeSplit], "") + lastRadii, lastRadiiOk := expandTokenQuad(tokens[afterSplit:], "") // Stop now if the pattern wasn't matched if !firstRadiiOk || (beforeSplit < afterSplit && !lastRadiiOk) { @@ -95,8 +95,8 @@ func (borderRadius *borderRadiusTracker) mangleCorner(rules []css_ast.Rule, decl borderRadius.important = decl.Important } - if tokens := decl.Value; (len(tokens) == 1 && tokens[0].Kind.IsNumericOrIdent()) || - (len(tokens) == 2 && tokens[0].Kind.IsNumericOrIdent() && tokens[1].Kind.IsNumericOrIdent()) { + if tokens := decl.Value; (len(tokens) == 1 && tokens[0].Kind.IsNumeric()) || + (len(tokens) == 2 && tokens[0].Kind.IsNumeric() && tokens[1].Kind.IsNumeric()) { firstToken := tokens[0] if firstToken.TurnLengthIntoNumberIfZero() { tokens[0] = firstToken diff --git a/internal/css_parser/css_decls_box.go b/internal/css_parser/css_decls_box.go index d43b90cbfa6..5637f175e11 100644 --- a/internal/css_parser/css_decls_box.go +++ b/internal/css_parser/css_decls_box.go @@ -38,11 +38,12 @@ func (box *boxTracker) mangleSides(rules []css_ast.Rule, decl *css_ast.RDeclarat box.important = decl.Important } + allowedIdent := "" isMargin := decl.Key == css_ast.DMargin - if quad, ok := expandTokenQuad(decl.Value); ok && - isNumericOrMarginAuto(&quad[0], isMargin) && isNumericOrMarginAuto(&quad[1], isMargin) && - isNumericOrMarginAuto(&quad[2], isMargin) && isNumericOrMarginAuto(&quad[3], isMargin) { - isMargin := decl.Key == css_ast.DMargin + if isMargin { + allowedIdent = "auto" + } + if quad, ok := expandTokenQuad(decl.Value, allowedIdent); ok { for side, t := range quad { t.TurnLengthIntoNumberIfZero() box.updateSide(rules, side, boxSide{token: t, index: uint32(index)}) @@ -65,16 +66,19 @@ func (box *boxTracker) mangleSide(rules []css_ast.Rule, decl *css_ast.RDeclarati case css_ast.DMarginTop, css_ast.DMarginRight, css_ast.DMarginBottom, css_ast.DMarginLeft: isMargin = true } - if tokens := decl.Value; len(tokens) == 1 && isNumericOrMarginAuto(&tokens[0], isMargin) { - t := tokens[0] - if t.TurnLengthIntoNumberIfZero() { - tokens[0] = t + + if tokens := decl.Value; len(tokens) == 1 { + if t := tokens[0]; t.Kind.IsNumeric() || (t.Kind == css_lexer.TIdent && isMargin && t.Text == "auto") { + if t.TurnLengthIntoNumberIfZero() { + tokens[0] = t + } + box.updateSide(rules, side, boxSide{token: t, index: uint32(index), single: true}) + box.compactRules(rules, decl.KeyRange, removeWhitespace, isMargin) + return } - box.updateSide(rules, side, boxSide{token: t, index: uint32(index), single: true}) - box.compactRules(rules, decl.KeyRange, removeWhitespace, isMargin) - } else { - box.sides = [4]boxSide{} } + + box.sides = [4]boxSide{} } func (box *boxTracker) compactRules(rules []css_ast.Rule, keyRange logger.Range, removeWhitespace bool, isMargin bool) { @@ -117,13 +121,3 @@ func (box *boxTracker) compactRules(rules []css_ast.Rule, keyRange logger.Range, Important: box.important, } } - -func isNumericOrMarginAuto(t *css_ast.Token, isMargin bool) bool { - if t.Kind.IsNumeric() { - return true - } - if isMargin && t.Kind == css_lexer.TIdent && t.Text == "auto" { - return true - } - return false -} diff --git a/internal/css_parser/css_parser_test.go b/internal/css_parser/css_parser_test.go index efaa3603bc3..9f786b17a5a 100644 --- a/internal/css_parser/css_parser_test.go +++ b/internal/css_parser/css_parser_test.go @@ -1016,7 +1016,6 @@ func TestBorderRadius(t *testing.T) { expectPrintedMangle(t, "a { border-top-left-radius: 0 1 }", "a {\n border-top-left-radius: 0 1;\n}\n") expectPrintedMangle(t, "a { border-top-left-radius: 0; border-radius: 1 }", "a {\n border-radius: 1;\n}\n") - expectPrintedMangle(t, "a { border-top-left-radius: 0; border-radius: inherit }", "a {\n border-radius: inherit;\n}\n") expectPrintedMangle(t, "a { border-radius: 1 2 3 4 }", "a {\n border-radius: 1 2 3 4;\n}\n") expectPrintedMangle(t, "a { border-radius: 1 2 1 3 }", "a {\n border-radius: 1 2 1 3;\n}\n") @@ -1063,6 +1062,13 @@ func TestBorderRadius(t *testing.T) { // These should not be changed because "--x" and "--z" could be empty expectPrintedMangle(t, "a { border-radius: var(--x) var(--y) var(--z) var(--y) }", "a {\n border-radius: var(--x) var(--y) var(--z) var(--y);\n}\n") expectPrintedMangle(t, "a { border-radius: 0 / var(--x) var(--y) var(--z) var(--y) }", "a {\n border-radius: 0 / var(--x) var(--y) var(--z) var(--y);\n}\n") + + // "inherit" should not be merged + expectPrintedMangle(t, "a { border-radius: 1px; border-top-left-radius: 0 }", "a {\n border-radius: 0 1px 1px;\n}\n") + expectPrintedMangle(t, "a { border-radius: inherit; border-top-left-radius: 0 }", "a {\n border-radius: inherit;\n border-top-left-radius: 0;\n}\n") + expectPrintedMangle(t, "a { border-radius: 0; border-top-left-radius: inherit }", "a {\n border-radius: 0;\n border-top-left-radius: inherit;\n}\n") + expectPrintedMangle(t, "a { border-top-left-radius: 0; border-radius: inherit }", "a {\n border-top-left-radius: 0;\n border-radius: inherit;\n}\n") + expectPrintedMangle(t, "a { border-top-left-radius: inherit; border-radius: 0 }", "a {\n border-top-left-radius: inherit;\n border-radius: 0;\n}\n") } func TestBoxShadow(t *testing.T) {