release notes for css nesting bugfix

CHANGELOG.md

@@ -11,6 +11,27 @@
     import Alias = Base.SomeType;
     ```
 
+* Fix CSS minification bug involving `&` ([#4497](https://github.com/evanw/esbuild/issues/4497))
+
+    This release fixes a bug where esbuild's CSS minifier incorrectly removed a `&` when it was unsafe to do so. Here is an example:
+
+    ```css
+    /* Original code */
+    .a .b {
+      & .b:not(& .c) {
+        color: red;
+      }
+    }
+
+    /* Old output (with --minify) */
+    .a .b{.b:not(& .c){color:red}}
+
+    /* New output (with --minify) */
+    .a .b{& .b:not(& .c){color:red}}
+    ```
+
+    This should match `<span class="a"><span class="b"><span class="b">yes</span></span></span>` but not `<span class="a"><span class="b">no</span></span>`. The old output incorrectly matched both.
+
 * Avoid overwriting input files without `--allow-overwrite` ([#4484](https://github.com/evanw/esbuild/issues/4484))
 
     For example: `esbuild input.js --outfile=input.js` tells esbuild to overwrite `input.js` with the output of running esbuild on it. This was supposed to already be prevented by default, but it accidentally regressed in version 0.17.0 and apparently didn't have any test coverage. The error message was being printed but the input file was still being overwritten. Oops.

internal/css_parser/css_parser_selector.go

@@ -257,6 +257,12 @@ func analyzeLeadingAmpersand(sel css_ast.ComplexSelector, isDeclarationContext b
 			if second.Combinator.Byte == 0 && rest.ContainsNestingCombinator() {
 				// ".foo { & &.bar {} }" => ".foo { & &.bar {} }"
 				// ".foo { & .bar:not(& .baz) {} }" => ".foo { & .bar:not(& .baz) {} }"
+
+				// The specification says: "If a selector in the <relative-selector-list>
+				// does not start with a combinator but does contain the nesting selector,
+				// it is interpreted as a non-relative selector." So we don't want to
+				// remove the leading "&" here as that would change the meaning to a
+				// non-relative selector. See: https://www.w3.org/TR/css-nesting-1/
 			} else if second.Combinator.Byte != 0 || second.TypeSelector == nil || !isDeclarationContext {
 				// "& + div {}" => "+ div {}"
 				// "& div {}" => "div {}"
@@ -267,6 +273,30 @@ func analyzeLeadingAmpersand(sel css_ast.ComplexSelector, isDeclarationContext b
 			} else {
 				// ".foo { & div {} }"
 				// ".foo { .bar, & div {} }" => ".foo { .bar, div {} }"
+
+				// The first iteration of CSS nesting didn't support an implicit "&" if
+				// the selector started with an identifier. This was done to avoid
+				// confusion with declarations in the parser, as the parser would
+				// otherwise need unbounded lookahead to distinguish between the two.
+				// Consider the following:
+				//
+				//   span {
+				//     color:blue;
+				//     div:hover span {}
+				//   }
+				//
+				// The "color:blue" is a declaration while "div:hover" is actually a
+				// relative selector in a nested style rule. The parser would need to
+				// scan to the ";" or "{" to distinguish between the two cases.
+				// Generally you want to avoid parsers with unbounded lookahead.
+				//
+				// However, this edge case is commonly encountered and it's confusing
+				// that it doesn't work. So the CSS working group later added unbounded
+				// lookahead to fix it: https://github.com/w3c/csswg-drafts/issues/7961.
+				//
+				// We deliberately don't take advantage of that newest syntax addition
+				// here so that our output still works in browsers before that newest
+				// syntax addition. We still keep a leading "&" before identifiers.
 				return canRemoveLeadingAmpersandIfNotFirst
 			}
 		}

internal/css_parser/css_parser_test.go

@@ -1140,6 +1140,7 @@ func TestNestedSelector(t *testing.T) {
 	expectPrintedMangle(t, "a { & .b:not(& .c) { color: red } }", "a {\n  & .b:not(& .c) {\n    color: red;\n  }\n}\n", "")
 	expectPrintedMangle(t, "a { & .b:is(.c &) { color: red } }", "a {\n  & .b:is(.c &) {\n    color: red;\n  }\n}\n", "")
 	expectPrintedMangle(t, "a { & + .b:not(& .c) { color: red } }", "a {\n  + .b:not(& .c) {\n    color: red;\n  }\n}\n", "")
+	expectPrintedMangle(t, "a { & + .b:is(& .c) { color: red } }", "a {\n  + .b:is(& .c) {\n    color: red;\n  }\n}\n", "")
 
 	// Reorder selectors to enable removing "&"
 	expectPrintedMangle(t, "reorder { & first, .second { color: red } }", "reorder {\n  .second,\n  first {\n    color: red;\n  }\n}\n", "")

scripts/browser/index.html

@@ -302,6 +302,34 @@
         `,
       })
     },
+
+    // See: https://github.com/evanw/esbuild/issues/4497
+    async cssNestingIssue4497() {
+      await assertSameColorsWithNestingTransform(esbuild, {
+        css: `
+          .a {
+            color: red;
+
+            .b:not(.a .b .b) {
+              color: green;
+
+              & .b:not(& .b .b) {
+                color: blue;
+              }
+            }
+          }
+        `,
+        html: `
+          <div class="a">
+            <span>1</span>
+            <div class="b">
+              <span>2</span>
+              <div class="b"><span>3</span></div>
+            </div>
+          </div>
+        `,
+      })
+    },
   })
 
   async function runTest(test, fn) {