fix #2555: keyframe animation names can be strings

CHANGELOG.md

@@ -1,5 +1,41 @@
 # Changelog
 
+## Unreleased
+
+* Support strings as keyframe animation names in CSS ([#2555](https://github.com/evanw/esbuild/issues/2555))
+
+    With this release, esbuild will now parse animation names that are specified as strings and will convert them to identifiers. The CSS specification allows animation names to be specified using either identifiers or strings but Chrome only understands identifiers, so esbuild will now always convert string names to identifier names for Chrome compatibility:
+
+    ```css
+    /* Original code */
+    @keyframes "hide menu" {
+      from { opacity: 1 }
+      to { opacity: 0 }
+    }
+    menu.hide {
+      animation: 0.5s ease-in-out "hide menu";
+    }
+
+    /* Old output */
+    @keyframes "hide menu" { from { opacity: 1 } to { opacity: 0 } }
+    menu.hide {
+      animation: 0.5s ease-in-out "hide menu";
+    }
+
+    /* New output */
+    @keyframes hide\ menu {
+      from {
+        opacity: 1;
+      }
+      to {
+        opacity: 0;
+      }
+    }
+    menu.hide {
+      animation: 0.5s ease-in-out hide\ menu;
+    }
+    ```
+
 ## 0.18.17
 
 * Support `An+B` syntax and `:nth-*()` pseudo-classes in CSS

internal/css_parser/css_decls.go

@@ -133,6 +133,14 @@ func (p *parser) processDeclarations(rules []css_ast.Rule) (rewrittenRules []css
 				}
 			}
 
+		case css_ast.DAnimation:
+			p.processAnimationShorthand(decl.Value)
+
+		case css_ast.DAnimationName:
+			if len(decl.Value) == 1 {
+				p.processAnimationName(&decl.Value[0])
+			}
+
 		case css_ast.DFont:
 			if p.options.minifySyntax {
 				decl.Value = p.mangleFont(decl.Value)

internal/css_parser/css_decls_animation.go

@@ -0,0 +1,97 @@
+package css_parser
+
+import (
+	"strings"
+
+	"github.com/evanw/esbuild/internal/css_ast"
+	"github.com/evanw/esbuild/internal/css_lexer"
+)
+
+// Scan for animation names in the "animation" shorthand property
+func (p *parser) processAnimationShorthand(tokens []css_ast.Token) {
+	type foundFlags struct {
+		timingFunction bool
+		iterationCount bool
+		direction      bool
+		fillMode       bool
+		playState      bool
+		name           bool
+	}
+
+	found := foundFlags{}
+
+	for i, t := range tokens {
+		switch t.Kind {
+		case css_lexer.TComma:
+			// Reset the flags when we encounter a comma
+			found = foundFlags{}
+
+		case css_lexer.TNumber:
+			if !found.iterationCount {
+				found.iterationCount = true
+				continue
+			}
+
+		case css_lexer.TIdent:
+			if !found.timingFunction {
+				switch strings.ToLower(t.Text) {
+				case "linear", "ease", "ease-in", "ease-out", "ease-in-out", "step-start", "step-end":
+					found.timingFunction = true
+					continue
+				}
+			}
+
+			if !found.iterationCount && strings.ToLower(t.Text) == "infinite" {
+				found.iterationCount = true
+				continue
+			}
+
+			if !found.direction {
+				switch strings.ToLower(t.Text) {
+				case "normal", "reverse", "alternate", "alternate-reverse":
+					found.direction = true
+					continue
+				}
+			}
+
+			if !found.fillMode {
+				switch strings.ToLower(t.Text) {
+				case "none", "forwards", "backwards", "both":
+					found.fillMode = true
+					continue
+				}
+			}
+
+			if !found.playState {
+				switch strings.ToLower(t.Text) {
+				case "running", "paused":
+					found.playState = true
+					continue
+				}
+			}
+
+			if !found.name {
+				p.processAnimationName(&tokens[i])
+				found.name = true
+				continue
+			}
+
+		case css_lexer.TString:
+			if !found.name {
+				p.processAnimationName(&tokens[i])
+				found.name = true
+				continue
+			}
+		}
+	}
+}
+
+func (p *parser) processAnimationName(token *css_ast.Token) {
+	// Note: Strings as names is allowed in the CSS specification and works in
+	// Firefox and Safari but Chrome has strangely decided to deliberately not
+	// support this. We always turn all string names into identifiers to avoid
+	// them silently breaking in Chrome.
+	if token.Kind == css_lexer.TString {
+		token.Kind = css_lexer.TIdent
+	}
+}

internal/css_parser/css_parser.go

@@ -1144,12 +1144,13 @@ abortRuleParser:
 		if p.peek(css_lexer.TIdent) {
 			name = p.decoded()
 			p.advance()
-		} else if p.eat(css_lexer.TString) {
-			// Consider string names to be an unknown rule even though they are allowed
-			// by the specification and they work in Firefox because they do not work in
-			// Chrome or Safari. We don't take the effort to support this Firefox-only
-			// feature natively. Instead, we just pass the syntax through unmodified.
-			break
+		} else if p.peek(css_lexer.TString) {
+			// Note: Strings as names is allowed in the CSS specification and works in
+			// Firefox and Safari but Chrome has strangely decided to deliberately not
+			// support this. We always turn all string names into identifiers to avoid
+			// them silently breaking in Chrome.
+			name = p.decoded()
+			p.advance()
 		} else if !p.expect(css_lexer.TIdent) {
 			break
 		}

internal/css_parser/css_parser_test.go

@@ -1320,7 +1320,6 @@ func TestLegalComment(t *testing.T) {
 
 func TestAtKeyframes(t *testing.T) {
 	expectPrinted(t, "@keyframes {}", "@keyframes {}\n", "<stdin>: WARNING: Expected identifier but found \"{\"\n")
-	expectPrinted(t, "@keyframes 'name' {}", "@keyframes \"name\" {}\n", "")
 	expectPrinted(t, "@keyframes name{}", "@keyframes name {\n}\n", "")
 	expectPrinted(t, "@keyframes name {}", "@keyframes name {\n}\n", "")
 	expectPrinted(t, "@keyframes name{0%,50%{color:red}25%,75%{color:blue}}",
@@ -1332,6 +1331,13 @@ func TestAtKeyframes(t *testing.T) {
 	expectPrinted(t, "@keyframes name { from { color: red } to { color: blue } }",
 		"@keyframes name {\n  from {\n    color: red;\n  }\n  to {\n    color: blue;\n  }\n}\n", "")
 
+	// Note: Strings as names is allowed in the CSS specification and works in
+	// Firefox and Safari but Chrome has strangely decided to deliberately not
+	// support this. We always turn all string names into identifiers to avoid
+	// them silently breaking in Chrome.
+	expectPrinted(t, "@keyframes 'name' {}", "@keyframes name {\n}\n", "")
+	expectPrinted(t, "@keyframes 'name 2' {}", "@keyframes name\\ 2 {\n}\n", "")
+
 	expectPrinted(t, "@keyframes name { from { color: red } }", "@keyframes name {\n  from {\n    color: red;\n  }\n}\n", "")
 	expectPrinted(t, "@keyframes name { 100% { color: red } }", "@keyframes name {\n  100% {\n    color: red;\n  }\n}\n", "")
 	expectPrintedMangle(t, "@keyframes name { from { color: red } }", "@keyframes name {\n  0% {\n    color: red;\n  }\n}\n", "")
@@ -1343,7 +1349,6 @@ func TestAtKeyframes(t *testing.T) {
 	expectPrinted(t, "@-o-keyframes name {}", "@-o-keyframes name {\n}\n", "")
 
 	expectPrinted(t, "@keyframes {}", "@keyframes {}\n", "<stdin>: WARNING: Expected identifier but found \"{\"\n")
-	expectPrinted(t, "@keyframes 'name' {}", "@keyframes \"name\" {}\n", "") // This is allowed as it's technically possible to use in Firefox (but in no other browser)
 	expectPrinted(t, "@keyframes name { 0% 100% {} }", "@keyframes name { 0% 100% {} }\n", "<stdin>: WARNING: Expected \",\" but found \"100%\"\n")
 	expectPrinted(t, "@keyframes name { {} 0% {} }", "@keyframes name { {} 0% {} }\n", "<stdin>: WARNING: Expected percentage but found \"{\"\n")
 	expectPrinted(t, "@keyframes name { 100 {} }", "@keyframes name { 100 {} }\n", "<stdin>: WARNING: Expected percentage but found \"100\"\n")
@@ -1371,6 +1376,16 @@ func TestAtKeyframes(t *testing.T) {
 	expectPrinted(t, "@keyframes x {", "@keyframes x {}\n", "<stdin>: WARNING: Expected \"}\" to go with \"{\"\n<stdin>: NOTE: The unbalanced \"{\" is here:\n")
 }
 
+func TestAnimationName(t *testing.T) {
+	// Note: Strings as names is allowed in the CSS specification and works in
+	// Firefox and Safari but Chrome has strangely decided to deliberately not
+	// support this. We always turn all string names into identifiers to avoid
+	// them silently breaking in Chrome.
+	expectPrinted(t, "div { animation-name: 'name' }", "div {\n  animation-name: name;\n}\n", "")
+	expectPrinted(t, "div { animation-name: 'name 2' }", "div {\n  animation-name: name\\ 2;\n}\n", "")
+	expectPrinted(t, "div { animation: 2s linear 'name 2', 3s infinite 'name 3' }", "div {\n  animation: 2s linear name\\ 2, 3s infinite name\\ 3;\n}\n", "")
+}
+
 func TestAtRuleValidation(t *testing.T) {
 	expectPrinted(t, "a {} b {} c {} @charset \"UTF-8\";", "a {\n}\nb {\n}\nc {\n}\n@charset \"UTF-8\";\n",
 		"<stdin>: WARNING: \"@charset\" must be the first rule in the file\n"+