diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ecdc8615ac..0e2c6c8a726 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,43 @@ ## Unreleased +* Fix CSS transform bugs with nested selectors that start with a combinator ([#3096](https://github.com/evanw/esbuild/issues/3096)) + + This release fixes several bugs regarding transforming nested CSS into non-nested CSS for older browsers. The bugs were due to lack of test coverage for nested selectors with more than one compound selector where they all start with the same combinator. Here's what some problematic cases look like before and after these fixes: + + ```css + /* Original code */ + .foo { + > &a, + > &b { + color: red; + } + } + .bar { + > &a, + + &b { + color: green; + } + } + + /* Old output (with --target=chrome90) */ + .foo :is(> .fooa, > .foob) { + color: red; + } + .bar :is(> .bara, + .barb) { + color: green; + } + + /* New output (with --target=chrome90) */ + .foo > :is(a.foo, b.foo) { + color: red; + } + .bar > a.bar, + .bar + b.bar { + color: green; + } + ``` + * Avoid removing unrecognized directives from the directive prologue when minifying ([#3115](https://github.com/evanw/esbuild/issues/3115)) The [directive prologue](https://262.ecma-international.org/6.0/#sec-directive-prologues-and-the-use-strict-directive) in JavaScript is a sequence of top-level string expressions that come before your code. The only directives that JavaScript engines currently recognize are `use strict` and sometimes `use asm`. However, the people behind React have made up their own directive for their own custom dialect of JavaScript. Previously esbuild only preserved the `use strict` directive when minifying, although you could still write React JavaScript with esbuild using something like `--banner:js="'your directive here';"`. With this release, you can now put arbitrary directives in the entry point and esbuild will preserve them in its minified output: diff --git a/internal/css_ast/css_ast.go b/internal/css_ast/css_ast.go index 8d73a5124f3..5c255613f29 100644 --- a/internal/css_ast/css_ast.go +++ b/internal/css_ast/css_ast.go @@ -598,11 +598,14 @@ type ComplexSelector struct { Selectors []CompoundSelector } -func (s ComplexSelector) AppendToTokens(tokens []Token) []Token { +func (s ComplexSelector) AppendToTokensWithoutLeadingCombinator(tokens []Token) []Token { for i, sel := range s.Selectors { if n := len(tokens); i > 0 && n > 0 { tokens[n-1].Whitespace |= WhitespaceAfter } + if i == 0 { + sel.Combinator = 0 + } tokens = sel.AppendToTokens(tokens) } return tokens @@ -714,14 +717,16 @@ func (sel CompoundSelector) AppendToTokens(tokens []Token) []Token { } } - if sel.HasNestingSelector { - tokens = append(tokens, Token{Kind: css_lexer.TDelimAmpersand, Text: "&"}) - } - if sel.TypeSelector != nil { tokens = sel.TypeSelector.AppendToTokens(tokens) } + // Put this after the type selector in case it's substituted for ":is()" + // ".foo { > &a, > &b {} }" => ".foo > :is(b.foo, c.foo) {}" (we don't want to get ".foo > :is(.foob, .fooc) {}" instead) + if sel.HasNestingSelector { + tokens = append(tokens, Token{Kind: css_lexer.TDelimAmpersand, Text: "&"}) + } + for _, ss := range sel.SubclassSelectors { tokens = ss.AppendToTokens(tokens) } diff --git a/internal/css_parser/css_nesting.go b/internal/css_parser/css_nesting.go index f767050d6ee..fc4572798b3 100644 --- a/internal/css_parser/css_nesting.go +++ b/internal/css_parser/css_nesting.go @@ -108,6 +108,7 @@ func lowerNestingInRuleWithContext(rule css_ast.Rule, context *lowerNestingConte // Pass 1: Canonicalize and analyze our selectors canUseGroupDescendantCombinator := true // Can we do "parent «space» :is(...selectors)"? canUseGroupSubSelector := true // Can we do "parent«nospace»:is(...selectors)"? + var commonLeadingCombinator uint8 for i := range r.Selectors { sel := &r.Selectors[i] @@ -119,6 +120,17 @@ func lowerNestingInRuleWithContext(rule css_ast.Rule, context *lowerNestingConte // Are all children of the form "& «something»"? if len(sel.Selectors) < 2 || !sel.Selectors[0].IsSingleAmpersand() { canUseGroupDescendantCombinator = false + } else { + // If all children are of the form "& «COMBINATOR» «something»", is «COMBINATOR» the same in all cases? + var combinator uint8 + if len(sel.Selectors) >= 2 { + combinator = sel.Selectors[1].Combinator + } + if i == 0 { + commonLeadingCombinator = combinator + } else if commonLeadingCombinator != combinator { + canUseGroupDescendantCombinator = false + } } // Are all children of the form "&«something»"? @@ -130,6 +142,7 @@ func lowerNestingInRuleWithContext(rule css_ast.Rule, context *lowerNestingConte // Try to apply simplifications for shorter output if canUseGroupDescendantCombinator { // "& a, & b {}" => "& :is(a, b) {}" + // "& > a, & > b {}" => "& > :is(a, b) {}" for i := range r.Selectors { sel := &r.Selectors[i] sel.Selectors = sel.Selectors[1:] @@ -139,6 +152,7 @@ func lowerNestingInRuleWithContext(rule css_ast.Rule, context *lowerNestingConte r.Selectors = []css_ast.ComplexSelector{merged} } else if canUseGroupSubSelector { // "&a, &b {}" => "&:is(a, b) {}" + // "> &a, > &b {}" => "> &:is(a, b) {}" for i := range r.Selectors { sel := &r.Selectors[i] sel.Selectors[0].HasNestingSelector = false @@ -212,27 +226,29 @@ func substituteAmpersandsInCompoundSelector(sel css_ast.CompoundSelector, replac // Convert the replacement to a single compound selector var single css_ast.CompoundSelector - if len(replacement.Selectors) == 1 || len(results) == 0 { + if sel.Combinator == 0 && (len(replacement.Selectors) == 1 || len(results) == 0) { // ".foo { :hover & {} }" => ":hover .foo {}" // ".foo .bar { &:hover {} }" => ".foo .bar:hover {}" last := len(replacement.Selectors) - 1 results = append(results, replacement.Selectors[:last]...) single = replacement.Selectors[last] + sel.Combinator = single.Combinator + } else if len(replacement.Selectors) == 1 { + // ".foo { > &:hover {} }" => ".foo > .foo:hover {}" + single = replacement.Selectors[0] } else { // ".foo .bar { :hover & {} }" => ":hover :is(.foo .bar) {}" + // ".foo .bar { > &:hover {} }" => ".foo .bar > :is(.foo .bar):hover {}" single = css_ast.CompoundSelector{ SubclassSelectors: []css_ast.SS{&css_ast.SSPseudoClass{ Name: "is", - Args: replacement.AppendToTokens(nil), + Args: replacement.AppendToTokensWithoutLeadingCombinator(nil), }}, } } var subclassSelectorPrefix []css_ast.SS - // Copy over the combinator, if any - sel.Combinator = single.Combinator - // Insert the type selector if single.TypeSelector != nil { if sel.TypeSelector != nil { @@ -281,7 +297,7 @@ func substituteAmpersandsInTokens(tokens []css_ast.Token, replacement css_ast.Co } var results []css_ast.Token - replacementTokens := replacement.AppendToTokens(nil) + replacementTokens := replacement.AppendToTokensWithoutLeadingCombinator(nil) for _, t := range tokens { if t.Kind != css_lexer.TDelimAmpersand { results = append(results, t) @@ -332,16 +348,21 @@ func multipleComplexSelectorsToSingleComplexSelector(selectors []css_ast.Complex // This must be non-nil so we print ":is()" instead of ":is" tokens := []css_ast.Token{} + var leadingCombinator uint8 for i, sel := range selectors { if i > 0 { tokens = append(tokens, css_ast.Token{Kind: css_lexer.TComma, Text: ",", Whitespace: css_ast.WhitespaceAfter}) } - tokens = sel.AppendToTokens(tokens) + + // "> a, > b" => "> :is(a, b)" (the caller should have already checked that all leading combinators are the same) + leadingCombinator = sel.Selectors[0].Combinator + tokens = sel.AppendToTokensWithoutLeadingCombinator(tokens) } return css_ast.ComplexSelector{ Selectors: []css_ast.CompoundSelector{{ + Combinator: leadingCombinator, SubclassSelectors: []css_ast.SS{&css_ast.SSPseudoClass{ Name: "is", Args: tokens, diff --git a/internal/css_parser/css_parser_test.go b/internal/css_parser/css_parser_test.go index 71a9e09b247..8dc404091dd 100644 --- a/internal/css_parser/css_parser_test.go +++ b/internal/css_parser/css_parser_test.go @@ -907,6 +907,22 @@ func TestNestedSelector(t *testing.T) { expectPrintedLower(t, "&, a { color: red }", ":scope,\na {\n color: red;\n}\n") expectPrintedLower(t, "&, a { .b { color: red } }", ":is(:scope, a) .b {\n color: red;\n}\n") expectPrintedLower(t, "&, a { .b { .c { color: red } } }", ":is(:scope, a) .b .c {\n color: red;\n}\n") + expectPrintedLower(t, "a { > b, > c { color: red } }", "a > :is(b, c) {\n color: red;\n}\n") + expectPrintedLower(t, "a { > b, + c { color: red } }", "a > b,\na + c {\n color: red;\n}\n") + expectPrintedLower(t, "a { & > b, & > c { color: red } }", "a > :is(b, c) {\n color: red;\n}\n") + expectPrintedLower(t, "a { & > b, & + c { color: red } }", "a > b,\na + c {\n color: red;\n}\n") + expectPrintedLower(t, "a { > &b, > &c { color: red } }", "a > :is(b:is(a), c:is(a)) {\n color: red;\n}\n") + expectPrintedLower(t, "a { > &b, + &c { color: red } }", "a > a:is(b),\na + a:is(c) {\n color: red;\n}\n") + expectPrintedLower(t, "a { > &.b, > &.c { color: red } }", "a > :is(a.b, a.c) {\n color: red;\n}\n") + expectPrintedLower(t, "a { > &.b, + &.c { color: red } }", "a > a.b,\na + a.c {\n color: red;\n}\n") + expectPrintedLower(t, ".a { > &b, > &c { color: red } }", ".a > :is(b.a, c.a) {\n color: red;\n}\n") + expectPrintedLower(t, ".a { > &b, + &c { color: red } }", ".a > b.a,\n.a + c.a {\n color: red;\n}\n") + expectPrintedLower(t, ".a { > &.b, > &.c { color: red } }", ".a > :is(.a.b, .a.c) {\n color: red;\n}\n") + expectPrintedLower(t, ".a { > &.b, + &.c { color: red } }", ".a > .a.b,\n.a + .a.c {\n color: red;\n}\n") + expectPrintedLower(t, "~ .a { > &.b, > &.c { color: red } }", "~ .a > :is(.a.b, .a.c) {\n color: red;\n}\n") + expectPrintedLower(t, "~ .a { > &.b, + &.c { color: red } }", "~ .a > .a.b,\n~ .a + .a.c {\n color: red;\n}\n") + expectPrintedLower(t, ".foo .bar { > &.a, > &.b { color: red } }", ".foo .bar > :is(.foo .bar.a, .foo .bar.b) {\n color: red;\n}\n") + expectPrintedLower(t, ".foo .bar { > &.a, + &.b { color: red } }", ".foo .bar > :is(.foo .bar).a,\n.foo .bar + :is(.foo .bar).b {\n color: red;\n}\n") expectPrintedLower(t, ".foo { @media screen {} }", "") expectPrintedLower(t, ".foo { @media screen { color: red } }", "@media screen {\n .foo {\n color: red;\n }\n}\n") expectPrintedLower(t, ".foo { @media screen { &:hover { color: red } } }", "@media screen {\n .foo:hover {\n color: red;\n }\n}\n")