Skip to content

Commit

Permalink
fix #3997, fix #4038: && should add specificity
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Feb 4, 2025
1 parent 31e7b4d commit adac05d
Show file tree
Hide file tree
Showing 8 changed files with 79 additions and 40 deletions.
30 changes: 29 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

It has been requested for esbuild to delete files when a build fails in watch mode. Previously esbuild left the old files in place, which could cause people to not immediately realize that the most recent build failed. With this release, esbuild will now delete all output files if a rebuild fails. Fixing the build error and triggering another rebuild will restore all output files again.

* Fix correctness issues with the CSS nesting transform ([#3620](https://github.com/evanw/esbuild/issues/3620), [#4037](https://github.com/evanw/esbuild/pull/4037))
* Fix correctness issues with the CSS nesting transform ([#3620](https://github.com/evanw/esbuild/issues/3620), [#3997](https://github.com/evanw/esbuild/issues/3997), [#4037](https://github.com/evanw/esbuild/pull/4037), [#4038](https://github.com/evanw/esbuild/pull/4038))

This release fixes the following problems:

Expand Down Expand Up @@ -35,6 +35,34 @@

Thanks to [@tim-we](https://github.com/tim-we) for working on a fix.

* The `&` CSS nesting selector can be repeated multiple times to increase CSS specificity. Previously esbuild ignored this possibility and incorrectly considered `&&` to have the same specificity as `&`. With this release, this should now work correctly:

```css
/* Original code (color should be red) */
div {
&& { color: red }
& { color: blue }
}

/* Old output (with --supported:nesting=false) */
div {
color: red;
}
div {
color: blue;
}

/* New output (with --supported:nesting=false) */
div:is(div) {
color: red;
}
div {
color: blue;
}
```

Thanks to [@CPunisher](https://github.com/CPunisher) for working on a fix.

* Fix incorrect package for `@esbuild/netbsd-arm64` ([#4018](https://github.com/evanw/esbuild/issues/4018))

Due to a copy+paste typo, the binary published to `@esbuild/netbsd-arm64` was not actually for `arm64`, and didn't run in that environment. This release should fix running esbuild in that environment (NetBSD on 64-bit ARM). Sorry about the mistake.
Expand Down
24 changes: 10 additions & 14 deletions internal/css_ast/css_ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -802,7 +802,7 @@ func (s ComplexSelector) CloneWithoutLeadingCombinator() ComplexSelector {
func (sel ComplexSelector) IsRelative() bool {
if sel.Selectors[0].Combinator.Byte == 0 {
for _, inner := range sel.Selectors {
if inner.HasNestingSelector() {
if len(inner.NestingSelectorLocs) > 0 {
return false
}
for _, ss := range inner.SubclassSelectors {
Expand Down Expand Up @@ -861,7 +861,7 @@ func (a ComplexSelector) Equal(b ComplexSelector, check *CrossFileEqualityCheck)

for i, ai := range a.Selectors {
bi := b.Selectors[i]
if ai.HasNestingSelector() != bi.HasNestingSelector() || ai.Combinator.Byte != bi.Combinator.Byte {
if len(ai.NestingSelectorLocs) != len(bi.NestingSelectorLocs) || ai.Combinator.Byte != bi.Combinator.Byte {
return false
}

Expand Down Expand Up @@ -890,25 +890,21 @@ type Combinator struct {
}

type CompoundSelector struct {
TypeSelector *NamespacedName
SubclassSelectors []SubclassSelector
NestingSelectorLoc ast.Index32 // "&"
Combinator Combinator // Optional, may be 0
TypeSelector *NamespacedName
SubclassSelectors []SubclassSelector
NestingSelectorLocs []logger.Loc // "&" vs. "&&" is different specificity
Combinator Combinator // Optional, may be 0

// If this is true, this is a "&" that was generated by a bare ":local" or ":global"
WasEmptyFromLocalOrGlobal bool
}

func (sel *CompoundSelector) HasNestingSelector() bool {
return sel.NestingSelectorLoc.IsValid()
}

func (sel CompoundSelector) IsSingleAmpersand() bool {
return sel.HasNestingSelector() && sel.Combinator.Byte == 0 && sel.TypeSelector == nil && len(sel.SubclassSelectors) == 0
return len(sel.NestingSelectorLocs) == 1 && sel.Combinator.Byte == 0 && sel.TypeSelector == nil && len(sel.SubclassSelectors) == 0
}

func (sel CompoundSelector) IsInvalidBecauseEmpty() bool {
return !sel.HasNestingSelector() && sel.TypeSelector == nil && len(sel.SubclassSelectors) == 0
return len(sel.NestingSelectorLocs) == 0 && sel.TypeSelector == nil && len(sel.SubclassSelectors) == 0
}

func (sel CompoundSelector) Range() (r logger.Range) {
Expand All @@ -918,8 +914,8 @@ func (sel CompoundSelector) Range() (r logger.Range) {
if sel.TypeSelector != nil {
r.ExpandBy(sel.TypeSelector.Range())
}
if sel.HasNestingSelector() {
r.ExpandBy(logger.Range{Loc: logger.Loc{Start: int32(sel.NestingSelectorLoc.GetIndex())}, Len: 1})
for _, loc := range sel.NestingSelectorLocs {
r.ExpandBy(logger.Range{Loc: loc, Len: 1})
}
if len(sel.SubclassSelectors) > 0 {
for _, ss := range sel.SubclassSelectors {
Expand Down
8 changes: 3 additions & 5 deletions internal/css_parser/css_nesting.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package css_parser
import (
"fmt"

"github.com/evanw/esbuild/internal/ast"
"github.com/evanw/esbuild/internal/compat"
"github.com/evanw/esbuild/internal/css_ast"
"github.com/evanw/esbuild/internal/logger"
Expand Down Expand Up @@ -123,7 +122,7 @@ func (p *parser) lowerNestingInRuleWithContext(rule css_ast.Rule, context *lower

// Inject the implicit "&" now for simplicity later on
if sel.IsRelative() {
sel.Selectors = append([]css_ast.CompoundSelector{{NestingSelectorLoc: ast.MakeIndex32(uint32(rule.Loc.Start))}}, sel.Selectors...)
sel.Selectors = append([]css_ast.CompoundSelector{{NestingSelectorLocs: []logger.Loc{rule.Loc}}}, sel.Selectors...)
}
}

Expand Down Expand Up @@ -288,9 +287,7 @@ func (p *parser) substituteAmpersandsInCompoundSelector(
results []css_ast.CompoundSelector,
strip leadingCombinatorStrip,
) []css_ast.CompoundSelector {
if sel.HasNestingSelector() {
nestingSelectorLoc := logger.Loc{Start: int32(sel.NestingSelectorLoc.GetIndex())}
sel.NestingSelectorLoc = ast.Index32{}
for _, nestingSelectorLoc := range sel.NestingSelectorLocs {
replacement := replacementFn(nestingSelectorLoc)

// Convert the replacement to a single compound selector
Expand Down Expand Up @@ -351,6 +348,7 @@ func (p *parser) substituteAmpersandsInCompoundSelector(
sel.SubclassSelectors = append(subclassSelectorPrefix, sel.SubclassSelectors...)
}
}
sel.NestingSelectorLocs = nil

// "div { :is(&.foo) {} }" => ":is(div.foo) {}"
for _, ss := range sel.SubclassSelectors {
Expand Down
6 changes: 3 additions & 3 deletions internal/css_parser/css_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -898,7 +898,7 @@ var nonDeprecatedElementsSupportedByIE7 = map[string]bool{
func isSafeSelectors(complexSelectors []css_ast.ComplexSelector) bool {
for _, complex := range complexSelectors {
for _, compound := range complex.Selectors {
if compound.HasNestingSelector() {
if len(compound.NestingSelectorLocs) > 0 {
// Bail because this is an extension: https://drafts.csswg.org/css-nesting-1/
return false
}
Expand Down Expand Up @@ -2088,8 +2088,8 @@ func (p *parser) parseSelectorRule(isTopLevel bool, opts parseSelectorOpts) css_
composesContext.problemRange = logger.Range{Loc: first.Combinator.Loc, Len: 1}
} else if first.TypeSelector != nil {
composesContext.problemRange = first.TypeSelector.Range()
} else if first.HasNestingSelector() {
composesContext.problemRange = logger.Range{Loc: logger.Loc{Start: int32(first.NestingSelectorLoc.GetIndex())}, Len: 1}
} else if len(first.NestingSelectorLocs) > 0 {
composesContext.problemRange = logger.Range{Loc: first.NestingSelectorLocs[0], Len: 1}
} else {
for i, ss := range first.SubclassSelectors {
class, ok := ss.Data.(*css_ast.SSClass)
Expand Down
15 changes: 7 additions & 8 deletions internal/css_parser/css_parser_selector.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"fmt"
"strings"

"github.com/evanw/esbuild/internal/ast"
"github.com/evanw/esbuild/internal/css_ast"
"github.com/evanw/esbuild/internal/css_lexer"
"github.com/evanw/esbuild/internal/logger"
Expand Down Expand Up @@ -82,7 +81,7 @@ func (p *parser) parseSelectorList(opts parseSelectorOpts) (list []css_ast.Compl

case canRemoveLeadingAmpersandIfNotFirst:
for i := 1; i < len(list); i++ {
if sel := list[i].Selectors[0]; !sel.HasNestingSelector() && (sel.Combinator.Byte != 0 || sel.TypeSelector == nil) {
if sel := list[i].Selectors[0]; len(sel.NestingSelectorLocs) == 0 && (sel.Combinator.Byte != 0 || sel.TypeSelector == nil) {
list[0].Selectors = list[0].Selectors[1:]
list[0], list[i] = list[i], list[0]
break
Expand All @@ -97,8 +96,8 @@ func (p *parser) parseSelectorList(opts parseSelectorOpts) (list []css_ast.Compl

func mergeCompoundSelectors(target *css_ast.CompoundSelector, source css_ast.CompoundSelector) {
// ".foo:local(&)" => "&.foo"
if source.HasNestingSelector() && !target.HasNestingSelector() {
target.NestingSelectorLoc = source.NestingSelectorLoc
if len(source.NestingSelectorLocs) > 0 && len(target.NestingSelectorLocs) == 0 {
target.NestingSelectorLocs = source.NestingSelectorLocs
}

if source.TypeSelector != nil {
Expand Down Expand Up @@ -210,7 +209,7 @@ func (p *parser) flattenLocalAndGlobalSelectors(list []css_ast.ComplexSelector,
if len(selectors) == 0 {
// Treat a bare ":global" or ":local" as a bare "&" nesting selector
selectors = append(selectors, css_ast.CompoundSelector{
NestingSelectorLoc: ast.MakeIndex32(uint32(sel.Selectors[0].Range().Loc.Start)),
NestingSelectorLocs: []logger.Loc{sel.Selectors[0].Range().Loc},
WasEmptyFromLocalOrGlobal: true,
})

Expand All @@ -235,7 +234,7 @@ const (
func analyzeLeadingAmpersand(sel css_ast.ComplexSelector, isDeclarationContext bool) leadingAmpersand {
if len(sel.Selectors) > 1 {
if first := sel.Selectors[0]; first.IsSingleAmpersand() {
if second := sel.Selectors[1]; second.Combinator.Byte == 0 && second.HasNestingSelector() {
if second := sel.Selectors[1]; second.Combinator.Byte == 0 && len(second.NestingSelectorLocs) > 0 {
// ".foo { & &.bar {} }" => ".foo { & &.bar {} }"
} else if second.Combinator.Byte != 0 || second.TypeSelector == nil || !isDeclarationContext {
// "& + div {}" => "+ div {}"
Expand Down Expand Up @@ -330,7 +329,7 @@ func (p *parser) parseCompoundSelector(opts parseComplexSelectorOpts) (sel css_a
hasLeadingNestingSelector := p.peek(css_lexer.TDelimAmpersand)
if hasLeadingNestingSelector {
p.nestingIsPresent = true
sel.NestingSelectorLoc = ast.MakeIndex32(uint32(startLoc.Start))
sel.NestingSelectorLocs = append(sel.NestingSelectorLocs, startLoc)
p.advance()
}

Expand Down Expand Up @@ -445,7 +444,7 @@ subclassSelectors:
case css_lexer.TDelimAmpersand:
// This is an extension: https://drafts.csswg.org/css-nesting-1/
p.nestingIsPresent = true
sel.NestingSelectorLoc = ast.MakeIndex32(uint32(subclassToken.Range.Loc.Start))
sel.NestingSelectorLocs = append(sel.NestingSelectorLocs, subclassToken.Range.Loc)
p.advance()

default:
Expand Down
31 changes: 25 additions & 6 deletions internal/css_parser/css_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1049,7 +1049,7 @@ func TestNestedSelector(t *testing.T) {
expectPrinted(t, "a { &a|b {} }", "a {\n &a|b {\n }\n}\n", "<stdin>: WARNING: Cannot use type selector \"a|b\" directly after nesting selector \"&\"\n"+sassWarningWrap)
expectPrinted(t, "a { &[b] {} }", "a {\n &[b] {\n }\n}\n", "")

expectPrinted(t, "a { && {} }", "a {\n & {\n }\n}\n", "")
expectPrinted(t, "a { && {} }", "a {\n && {\n }\n}\n", "")
expectPrinted(t, "a { & + & {} }", "a {\n & + & {\n }\n}\n", "")
expectPrinted(t, "a { & > & {} }", "a {\n & > & {\n }\n}\n", "")
expectPrinted(t, "a { & ~ & {} }", "a {\n & ~ & {\n }\n}\n", "")
Expand Down Expand Up @@ -1141,12 +1141,13 @@ func TestNestedSelector(t *testing.T) {

// Inline no-op nesting
expectPrintedMangle(t, "div { & { color: red } }", "div {\n color: red;\n}\n", "")
expectPrintedMangle(t, "div { && { color: red } }", "div {\n color: red;\n}\n", "")
expectPrintedMangle(t, "div { && { color: red } }", "div {\n && {\n color: red;\n }\n}\n", "")
expectPrintedMangle(t, "div { zoom: 2; & { color: red } }", "div {\n zoom: 2;\n color: red;\n}\n", "")
expectPrintedMangle(t, "div { zoom: 2; && { color: red } }", "div {\n zoom: 2;\n color: red;\n}\n", "")
expectPrintedMangle(t, "div { &, && { color: red } zoom: 2 }", "div {\n zoom: 2;\n color: red;\n}\n", "")
expectPrintedMangle(t, "div { &&, & { color: red } zoom: 2 }", "div {\n zoom: 2;\n color: red;\n}\n", "")
expectPrintedMangle(t, "div { a: 1; & { b: 4 } b: 2; && { c: 5 } c: 3 }", "div {\n a: 1;\n b: 2;\n c: 3;\n b: 4;\n c: 5;\n}\n", "")
expectPrintedMangle(t, "div { zoom: 2; && { color: red } }", "div {\n zoom: 2;\n && {\n color: red;\n }\n}\n", "")
expectPrintedMangle(t, "div { &, & { color: red } zoom: 2 }", "div {\n zoom: 2;\n color: red;\n}\n", "")
expectPrintedMangle(t, "div { &, && { color: red } zoom: 2 }", "div {\n &,\n && {\n color: red;\n }\n zoom: 2;\n}\n", "")
expectPrintedMangle(t, "div { &&, & { color: red } zoom: 2 }", "div {\n &&,\n & {\n color: red;\n }\n zoom: 2;\n}\n", "")
expectPrintedMangle(t, "div { a: 1; & { b: 4 } b: 2; && { c: 5 } c: 3 }", "div {\n a: 1;\n b: 2;\n && {\n c: 5;\n }\n c: 3;\n b: 4;\n}\n", "")
expectPrintedMangle(t, "div { .b { x: 1 } & { x: 2 } }", "div {\n .b {\n x: 1;\n }\n x: 2;\n}\n", "")
expectPrintedMangle(t, "div { & { & { & { color: red } } & { & { zoom: 2 } } } }", "div {\n color: red;\n zoom: 2;\n}\n", "")

Expand Down Expand Up @@ -1262,6 +1263,24 @@ func TestNestedSelector(t *testing.T) {
expectPrintedLowerUnsupported(t, nesting, ".demo { .lg { .triangle, .circle { color: red } } }", ".demo .lg .triangle,\n.demo .lg .circle {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".card { .featured & & & { color: red } }", ".featured .card .card .card {\n color: red;\n}\n", "")

// Duplicate "&" may be used to increase specificity
expectPrintedLowerUnsupported(t, nesting, ".foo { &&&.bar { color: red } }", ".foo.foo.foo.bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".foo { &&& .bar { color: red } }", ".foo.foo.foo .bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".foo { .bar&&& { color: red } }", ".foo.foo.foo.bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".foo { .bar &&& { color: red } }", ".bar .foo.foo.foo {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".foo { &.bar&.baz& { color: red } }", ".foo.foo.foo.bar.baz {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "a { &&&.bar { color: red } }", "a:is(a):is(a).bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "a { &&& .bar { color: red } }", "a:is(a):is(a) .bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "a { .bar&&& { color: red } }", "a:is(a):is(a).bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "a { .bar &&& { color: red } }", ".bar a:is(a):is(a) {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "a { &.bar&.baz& { color: red } }", "a:is(a):is(a).bar.baz {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "a, b { &&&.bar { color: red } }", ":is(a, b):is(a, b):is(a, b).bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "a, b { &&& .bar { color: red } }", ":is(a, b):is(a, b):is(a, b) .bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "a, b { .bar&&& { color: red } }", ":is(a, b):is(a, b):is(a, b).bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "a, b { .bar &&& { color: red } }", ".bar :is(a, b):is(a, b):is(a, b) {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "a, b { &.bar&.baz& { color: red } }", ":is(a, b):is(a, b):is(a, b).bar.baz {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".foo { &, &&.bar, &&& .baz { color: red } }", ".foo,\n.foo.foo.bar,\n.foo.foo.foo .baz {\n color: red;\n}\n", "")

// These are invalid SASS-style nested suffixes
expectPrintedLower(t, ".card { &--header { color: red } }", ".card {\n &--header {\n color: red;\n }\n}\n",
"<stdin>: WARNING: Cannot use type selector \"--header\" directly after nesting selector \"&\"\n"+sassWarningWrap)
Expand Down
4 changes: 2 additions & 2 deletions internal/css_printer/css_printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -459,9 +459,9 @@ func (p *printer) printCompoundSelector(sel css_ast.CompoundSelector, isFirst bo
p.printNamespacedName(*sel.TypeSelector, whitespace)
}

if sel.HasNestingSelector() {
for _, loc := range sel.NestingSelectorLocs {
if p.options.AddSourceMappings {
p.builder.AddSourceMapping(logger.Loc{Start: int32(sel.NestingSelectorLoc.GetIndex())}, "", p.css)
p.builder.AddSourceMapping(loc, "", p.css)
}

p.print("&")
Expand Down
1 change: 0 additions & 1 deletion scripts/browser/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,6 @@

// See: https://github.com/evanw/esbuild/issues/3997
async cssNestingIssue3997() {
return // TODO: Remove this
await assertSameColorsWithNestingTransform(esbuild, {
css: `
.foo {
Expand Down

0 comments on commit adac05d

Please sign in to comment.