fix #3648: copy selectors before checking children

Author: evanw

CHANGELOG.md

@@ -2,6 +2,32 @@
 
 ## Unreleased
 
+* Fix a bug with the CSS nesting transform ([#3648](https://github.com/evanw/esbuild/issues/3648))
+
+    This release fixes a bug with the CSS nesting transform for older browsers where the generated CSS could be incorrect if a selector list contained a pseudo element followed by another selector. The bug was caused by incorrectly mutating the parent rule's selector list when filtering out pseudo elements for the child rules:
+
+    ```css
+    /* Original code */
+    .foo {
+      &:after,
+      & .bar {
+        color: red;
+      }
+    }
+
+    /* Old output (with --supported:nesting=false) */
+    .foo .bar,
+    .foo .bar {
+      color: red;
+    }
+
+    /* New output (with --supported:nesting=false) */
+    .foo:after,
+    .foo .bar {
+      color: red;
+    }
+    ```
+
 * Fix a crash when resolving a path from a directory that doesn't exist ([#3634](https://github.com/evanw/esbuild/issues/3634))
 
     This release fixes a regression where esbuild could crash when resolving an absolute path if the source directory for the path resolution operation doesn't exist. While this situation doesn't normally come up, it could come up when running esbuild concurrently with another operation that mutates the file system as esbuild is doing a build (such as using `git` to switch branches). The underlying problem was a regression that was introduced in version 0.18.0.

internal/css_parser/css_nesting.go

@@ -23,40 +23,43 @@ func (p *parser) lowerNestingInRule(rule css_ast.Rule, results []css_ast.Rule) [
 			}
 		}
 
-		// Filter out pseudo elements because they are ignored by nested style
-		// rules. This is because pseudo-elements are not valid within :is():
-		// https://www.w3.org/TR/selectors-4/#matches-pseudo. This restriction
-		// may be relaxed in the future, but this restriction hash shipped so
-		// we're stuck with it: https://github.com/w3c/csswg-drafts/issues/7433.
-		selectors := r.Selectors
-		n := 0
-		for _, sel := range selectors {
+		parentSelectors := make([]css_ast.ComplexSelector, 0, len(r.Selectors))
+		for i, sel := range r.Selectors {
+			// Top-level "&" should be replaced with ":scope" to avoid recursion.
+			// From https://www.w3.org/TR/css-nesting-1/#nest-selector:
+			//
+			//   "When used in the selector of a nested style rule, the nesting
+			//   selector represents the elements matched by the parent rule. When
+			//   used in any other context, it represents the same elements as
+			//   :scope in that context (unless otherwise defined)."
+			//
+			substituted := make([]css_ast.CompoundSelector, 0, len(sel.Selectors))
+			for _, x := range sel.Selectors {
+				substituted = p.substituteAmpersandsInCompoundSelector(x, scope, substituted, keepLeadingCombinator)
+			}
+			r.Selectors[i] = css_ast.ComplexSelector{Selectors: substituted}
+
+			// Filter out pseudo elements because they are ignored by nested style
+			// rules. This is because pseudo-elements are not valid within :is():
+			// https://www.w3.org/TR/selectors-4/#matches-pseudo. This restriction
+			// may be relaxed in the future, but this restriction hash shipped so
+			// we're stuck with it: https://github.com/w3c/csswg-drafts/issues/7433.
+			//
+			// Note: This is only for the parent selector list that is used to
+			// substitute "&" within child rules. Do not filter out the pseudo
+			// element from the top-level selector list.
 			if !sel.UsesPseudoElement() {
-				// Top-level "&" should be replaced with ":scope" to avoid recursion.
-				// From https://www.w3.org/TR/css-nesting-1/#nest-selector:
-				//
-				//   "When used in the selector of a nested style rule, the nesting
-				//   selector represents the elements matched by the parent rule. When
-				//   used in any other context, it represents the same elements as
-				//   :scope in that context (unless otherwise defined)."
-				//
-				substituted := make([]css_ast.CompoundSelector, 0, len(sel.Selectors))
-				for _, x := range sel.Selectors {
-					substituted = p.substituteAmpersandsInCompoundSelector(x, scope, substituted, keepLeadingCombinator)
-				}
-				selectors[n] = css_ast.ComplexSelector{Selectors: substituted}
-				n++
+				parentSelectors = append(parentSelectors, css_ast.ComplexSelector{Selectors: substituted})
 			}
 		}
-		selectors = selectors[:n]
 
 		// Emit this selector before its nested children
 		start := len(results)
 		results = append(results, rule)
 
 		// Lower all children and filter out ones that become empty
 		context := lowerNestingContext{
-			parentSelectors: selectors,
+			parentSelectors: parentSelectors,
 			loweredRules:    results,
 		}
 		r.Rules = p.lowerNestingInRulesAndReturnRemaining(r.Rules, &context)

internal/css_parser/css_parser_test.go

@@ -1213,6 +1213,8 @@ func TestNestedSelector(t *testing.T) {
 	expectPrintedLowerUnsupported(t, nesting, ".foo, .bar:before { :hover & { color: red } }", ":hover .foo {\n  color: red;\n}\n", "")
 	expectPrintedLowerUnsupported(t, nesting, ".bar:before { &:hover { color: red } }", ":is():hover {\n  color: red;\n}\n", "")
 	expectPrintedLowerUnsupported(t, nesting, ".bar:before { :hover & { color: red } }", ":hover :is() {\n  color: red;\n}\n", "")
+	expectPrintedLowerUnsupported(t, nesting, ".foo { &:after, & .bar { color: red } }", ".foo:after,\n.foo .bar {\n  color: red;\n}\n", "")
+	expectPrintedLowerUnsupported(t, nesting, ".foo { & .bar, &:after { color: red } }", ".foo .bar,\n.foo:after {\n  color: red;\n}\n", "")
 	expectPrintedLowerUnsupported(t, nesting, ".xy { :where(&.foo) { color: red } }", ":where(.xy.foo) {\n  color: red;\n}\n", "")
 	expectPrintedLowerUnsupported(t, nesting, "div { :where(&.foo) { color: red } }", ":where(div.foo) {\n  color: red;\n}\n", "")
 	expectPrintedLowerUnsupported(t, nesting, ".xy { :where(.foo&) { color: red } }", ":where(.xy.foo) {\n  color: red;\n}\n", "")