Skip to content

Commit

Permalink
fix #2555: parse @keyframes with string namems
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Sep 18, 2022
1 parent 93e068d commit c9b24d8
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 11 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Expand Up @@ -93,6 +93,12 @@

However, the compilation had a subtle bug where the automatically-generated function-level symbols for multible hoisted block-level function declarations in the same block a sloppy-mode context were generated in a random order if the output was in strict mode, which could be the case if TypeScript's `alwaysStrict` setting was set to true. This lead to non-determinism in the output as the minifier would randomly exchange the generated names for these symbols on different runs. This bug has been fixed by sorting the keys of the unordered map before iterating over them.

* Fix parsing of `@keyframes` with string identifiers ([#2555](https://github.com/evanw/esbuild/issues/2555))

Firefox supports `@keyframes` with string identifier names. Previously this was treated as a syntax error by esbuild as it doesn't work in any other browser. The specification allows for this however, so it's technically not a syntax error (even though it would be unwise to use this feature at the moment). There was also a bug where esbuild would remove the identifier name in this case as the syntax wasn't recognized.

This release changes esbuild's parsing of `@keyframes` to now consider this case to be an unrecognized CSS rule. That means it will be passed through unmodified (so you can now use esbuild to bundle this Firefox-specific CSS) but the CSS will not be pretty-printed or minified. I don't think it makes sense for esbuild to have special code to handle this Firefox-specific syntax at this time. This decision can be revisited in the future if other browsers add support for this feature.

## 0.15.7

* Add `--watch=forever` to allow esbuild to never terminate ([#1511](https://github.com/evanw/esbuild/issues/1511), [#1885](https://github.com/evanw/esbuild/issues/1885))
Expand Down
11 changes: 7 additions & 4 deletions internal/css_parser/css_parser.go
Expand Up @@ -832,10 +832,13 @@ abortRuleParser:
if p.peek(css_lexer.TIdent) {
name = p.decoded()
p.advance()
} else if !p.expect(css_lexer.TIdent) && !p.eat(css_lexer.TString) && !p.peek(css_lexer.TOpenBrace) {
// Consider string names a syntax error even though they are allowed by
// the specification and they work in Firefox because they do not work in
// Chrome or Safari.
} 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.expect(css_lexer.TIdent) {
break
}

Expand Down
5 changes: 3 additions & 2 deletions internal/css_parser/css_parser_test.go
Expand Up @@ -1011,7 +1011,8 @@ func TestLegalComment(t *testing.T) {
}

func TestAtKeyframes(t *testing.T) {
expectPrinted(t, "@keyframes {}", "@keyframes \"\" {\n}\n")
expectPrinted(t, "@keyframes {}", "@keyframes {}\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}}",
Expand All @@ -1034,7 +1035,7 @@ func TestAtKeyframes(t *testing.T) {
expectPrinted(t, "@-o-keyframes name {}", "@-o-keyframes name {\n}\n")

expectParseError(t, "@keyframes {}", "<stdin>: WARNING: Expected identifier but found \"{\"\n")
expectParseError(t, "@keyframes 'name' {}", "<stdin>: WARNING: Expected identifier but found \"'name'\"\n")
expectParseError(t, "@keyframes 'name' {}", "") // This is allowed as it's technically possible to use in Firefox (but in no other browser)
expectParseError(t, "@keyframes name { 0% 100% {} }", "<stdin>: WARNING: Expected \",\" but found \"100%\"\n")
expectParseError(t, "@keyframes name { {} 0% {} }", "<stdin>: WARNING: Expected percentage but found \"{\"\n")
expectParseError(t, "@keyframes name { 100 {} }", "<stdin>: WARNING: Expected percentage but found \"100\"\n")
Expand Down
6 changes: 1 addition & 5 deletions internal/css_printer/css_printer.go
Expand Up @@ -108,11 +108,7 @@ func (p *printer) printRule(rule css_ast.Rule, indent int32, omitTrailingSemicol
p.print("@")
p.printIdent(r.AtToken, identNormal, mayNeedWhitespaceAfter)
p.print(" ")
if r.Name == "" {
p.print("\"\"")
} else {
p.printIdent(r.Name, identNormal, canDiscardWhitespaceAfter)
}
p.printIdent(r.Name, identNormal, canDiscardWhitespaceAfter)
if !p.options.MinifyWhitespace {
p.print(" ")
}
Expand Down

0 comments on commit c9b24d8

Please sign in to comment.