fix #3016: minify bug with bad @layer rules

CHANGELOG.md

@@ -6,6 +6,25 @@
 
     Go's WebAssembly implementation returns `EINVAL` instead of `ENOTDIR` when using the `readdir` syscall on a file. This messes up esbuild's implementation of node's module resolution algorithm since encountering `ENOTDIR` causes esbuild to continue its search (since it's a normal condition) while other encountering other errors causes esbuild to fail with an I/O error (since it's an unexpected condition). You can encounter this issue in practice if you use node's legacy `NODE_PATH` feature to tell esbuild to resolve node modules in a custom directory that was not installed by npm. This release works around this problem by converting `EINVAL` into `ENOTDIR` for the `readdir` syscall.
 
+* Fix a minification bug with CSS `@layer` rules that have parsing errors ([#3016](https://github.com/evanw/esbuild/issues/3016))
+
+    CSS at-rules [require either a `{}` block or a semicolon at the end](https://www.w3.org/TR/css-syntax-3/#consume-at-rule). Omitting both of these causes esbuild to treat the rule as an unknown at-rule. Previous releases of esbuild had a bug that incorrectly removed unknown at-rules without any children during minification if the at-rule token matched an at-rule that esbuild can handle. Specifically [cssnano](https://cssnano.co/) can generate `@layer` rules with parsing errors, and empty `@layer` rules cannot be removed because they have side effects (`@layer` didn't exist when esbuild's CSS support was added, so esbuild wasn't written to handle this). This release changes esbuild to no longer discard `@layer` rules with parsing errors when minifying (the rule `@layer c` has a parsing error):
+
+    ```css
+    /* Original input */
+    @layer a {
+      @layer b {
+        @layer c
+      }
+    }
+
+    /* Old output (with --minify) */
+    @layer a.b;
+
+    /* New output (with --minify) */
+    @layer a.b.c;
+    ```
+
 * Unterminated strings in CSS are no longer an error
 
     The CSS specification provides [rules for handling parsing errors](https://www.w3.org/TR/CSS22/syndata.html#parsing-errors). One of those rules is that user agents must close strings upon reaching the end of a line (i.e., before an unescaped line feed, carriage return or form feed character), but then drop the construct (declaration or rule) in which the string was found. For example:

internal/css_parser/css_parser.go

@@ -375,7 +375,7 @@ func (p *parser) mangleRules(rules []css_ast.Rule, isTopLevel bool) []css_ast.Ru
 			}
 
 		case *css_ast.RKnownAt:
-			if len(r.Rules) == 0 {
+			if len(r.Rules) == 0 && atKnownRuleCanBeRemovedIfEmpty[r.AtToken] {
 				continue
 			}
 
@@ -704,6 +704,9 @@ const (
 )
 
 var specialAtRules = map[string]atRuleKind{
+	"media":    atRuleInheritContext,
+	"supports": atRuleInheritContext,
+
 	"font-face": atRuleDeclarations,
 	"page":      atRuleDeclarations,
 
@@ -755,9 +758,8 @@ var specialAtRules = map[string]atRuleKind{
 	//
 	"layer": atRuleQualifiedOrEmpty,
 
-	"media":    atRuleInheritContext,
-	"scope":    atRuleInheritContext,
-	"supports": atRuleInheritContext,
+	// Reference: https://drafts.csswg.org/css-cascade-6/#scoped-styles
+	"scope": atRuleInheritContext,
 
 	// Reference: https://drafts.csswg.org/css-fonts-4/#font-palette-values
 	"font-palette-values": atRuleDeclarations,
@@ -782,6 +784,40 @@ var specialAtRules = map[string]atRuleKind{
 	"container": atRuleInheritContext,
 }
 
+var atKnownRuleCanBeRemovedIfEmpty = map[string]bool{
+	"media":     true,
+	"supports":  true,
+	"font-face": true,
+	"page":      true,
+
+	// https://www.w3.org/TR/css-page-3/#syntax-page-selector
+	"bottom-center":       true,
+	"bottom-left-corner":  true,
+	"bottom-left":         true,
+	"bottom-right-corner": true,
+	"bottom-right":        true,
+	"left-bottom":         true,
+	"left-middle":         true,
+	"left-top":            true,
+	"right-bottom":        true,
+	"right-middle":        true,
+	"right-top":           true,
+	"top-center":          true,
+	"top-left-corner":     true,
+	"top-left":            true,
+	"top-right-corner":    true,
+	"top-right":           true,
+
+	// https://drafts.csswg.org/css-cascade-6/#scoped-styles
+	"scope": true,
+
+	// https://drafts.csswg.org/css-fonts-4/#font-palette-values
+	"font-palette-values": true,
+
+	// https://drafts.csswg.org/css-contain-3/#container-rule
+	"container": true,
+}
+
 type atRuleValidity uint8
 
 const (
@@ -1067,9 +1103,21 @@ abortRuleParser:
 		}
 
 		// Otherwise there's some kind of syntax error
-		if kind := p.current().Kind; kind == css_lexer.TOpenBrace || kind == css_lexer.TCloseBrace || kind == css_lexer.TEndOfFile {
+		switch p.current().Kind {
+		case css_lexer.TEndOfFile:
 			p.expect(css_lexer.TSemicolon)
-		} else {
+			return css_ast.Rule{Loc: atRange.Loc, Data: &css_ast.RAtLayer{Names: names}}
+
+		case css_lexer.TCloseBrace:
+			p.expect(css_lexer.TSemicolon)
+			if !context.isTopLevel {
+				return css_ast.Rule{Loc: atRange.Loc, Data: &css_ast.RAtLayer{Names: names}}
+			}
+
+		case css_lexer.TOpenBrace:
+			p.expect(css_lexer.TSemicolon)
+
+		default:
 			p.unexpected()
 		}
 

internal/css_parser/css_parser_test.go

@@ -55,6 +55,13 @@ func expectParseErrorMinify(t *testing.T, contents string, expected string, expe
 	})
 }
 
+func expectParseErrorMangle(t *testing.T, contents string, expected string, expectedLog string) {
+	t.Helper()
+	expectPrintedCommon(t, contents, contents, expected, &expectedLog, config.Options{
+		MinifySyntax: true,
+	})
+}
+
 func expectPrinted(t *testing.T, contents string, expected string) {
 	t.Helper()
 	expectPrintedCommon(t, contents, contents, expected, nil, config.Options{})
@@ -1126,10 +1133,14 @@ func TestAtLayer(t *testing.T) {
 	expectPrinted(t, "@layer foo { div { color: red } }", "@layer foo {\n  div {\n    color: red;\n  }\n}\n")
 
 	// Check semicolon error recovery
-	expectPrinted(t, "@layer", "@layer;\n")
-	expectPrinted(t, "@layer a", "@layer a;\n")
-	expectPrinted(t, "@layer a { @layer }", "@layer a {\n  @layer;\n}\n")
-	expectPrinted(t, "@layer a { @layer b }", "@layer a {\n  @layer b;\n}\n")
+	expectParseError(t, "@layer", "@layer;\n", "<stdin>: WARNING: Expected \";\" but found end of file\n")
+	expectParseError(t, "@layer a", "@layer a;\n", "<stdin>: WARNING: Expected \";\" but found end of file\n")
+	expectParseError(t, "@layer a { @layer }", "@layer a {\n  @layer;\n}\n", "<stdin>: WARNING: Expected \";\"\n")
+	expectParseError(t, "@layer a { @layer b }", "@layer a {\n  @layer b;\n}\n", "<stdin>: WARNING: Expected \";\"\n")
+	expectParseErrorMangle(t, "@layer", "@layer;\n", "<stdin>: WARNING: Expected \";\" but found end of file\n")
+	expectParseErrorMangle(t, "@layer a", "@layer a;\n", "<stdin>: WARNING: Expected \";\" but found end of file\n")
+	expectParseErrorMangle(t, "@layer a { @layer }", "@layer a {\n  @layer;\n}\n", "<stdin>: WARNING: Expected \";\"\n")
+	expectParseErrorMangle(t, "@layer a { @layer b }", "@layer a.b;\n", "<stdin>: WARNING: Expected \";\"\n")
 
 	// Check mangling
 	expectPrintedMangle(t, "@layer foo { div {} }", "@layer foo;\n")