Skip to content

Commit

Permalink
fix #3620, fix #4037: remove wrong :is() nesting
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Feb 4, 2025
1 parent ca47c09 commit 31e7b4d
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 80 deletions.
29 changes: 29 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,35 @@

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))

This release fixes the following problems:

* Naive expansion of CSS nesting can result in an exponential blow-up of generated CSS if each nesting level has multiple selectors. Previously esbuild sometimes collapsed individual nesting levels using `:is()` to limit expansion. However, this collapsing wasn't correct in some cases, so it has been removed to fix correctness issues.

```css
/* Original code */
.parent {
> .a,
> .b1 > .b2 {
color: red;
}
}

/* Old output (with --supported:nesting=false) */
.parent > :is(.a, .b1 > .b2) {
color: red;
}

/* New output (with --supported:nesting=false) */
.parent > .a,
.parent > .b1 > .b2 {
color: red;
}
```

Thanks to [@tim-we](https://github.com/tim-we) 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
63 changes: 0 additions & 63 deletions internal/css_parser/css_nesting.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,76 +118,13 @@ func (p *parser) lowerNestingInRuleWithContext(rule css_ast.Rule, context *lower
// "a, b { &.c, & d, e & {} }" => ":is(a, b).c, :is(a, b) d, e :is(a, b) {}"

// 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 css_ast.Combinator
for i := range r.Selectors {
sel := &r.Selectors[i]

// 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...)
}

// Pseudo-elements aren't supported by ":is" (i.e. ":is(div, div::before)"
// is the same as ":is(div)") so we need to avoid generating ":is" if a
// pseudo-element is present.
if sel.UsesPseudoElement() {
canUseGroupDescendantCombinator = false
canUseGroupSubSelector = false
}

// 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 css_ast.Combinator
if len(sel.Selectors) >= 2 {
combinator = sel.Selectors[1].Combinator
}
if i == 0 {
commonLeadingCombinator = combinator
} else if commonLeadingCombinator.Byte != combinator.Byte {
canUseGroupDescendantCombinator = false
}
}

// Are all children of the form "&«something»"?
if first := sel.Selectors[0]; !first.HasNestingSelector() || first.IsSingleAmpersand() {
canUseGroupSubSelector = false
}
}

// Avoid generating ":is" if it's not supported
if p.options.unsupportedCSSFeatures.Has(compat.IsPseudoClass) && len(r.Selectors) > 1 {
canUseGroupDescendantCombinator = false
canUseGroupSubSelector = false
}

// Try to apply simplifications for shorter output
if canUseGroupDescendantCombinator {
// "& a, & b {}" => "& :is(a, b) {}"
// "& > a, & > b {}" => "& > :is(a, b) {}"
nestingSelectorLoc := r.Selectors[0].Selectors[0].NestingSelectorLoc
for i := range r.Selectors {
sel := &r.Selectors[i]
sel.Selectors = sel.Selectors[1:]
}
merged := p.multipleComplexSelectorsToSingleComplexSelector(r.Selectors)(rule.Loc)
merged.Selectors = append([]css_ast.CompoundSelector{{NestingSelectorLoc: nestingSelectorLoc}}, merged.Selectors...)
r.Selectors = []css_ast.ComplexSelector{merged}
} else if canUseGroupSubSelector {
// "&a, &b {}" => "&:is(a, b) {}"
// "> &a, > &b {}" => "> &:is(a, b) {}"
nestingSelectorLoc := r.Selectors[0].Selectors[0].NestingSelectorLoc
for i := range r.Selectors {
sel := &r.Selectors[i]
sel.Selectors[0].NestingSelectorLoc = ast.Index32{}
}
merged := p.multipleComplexSelectorsToSingleComplexSelector(r.Selectors)(rule.Loc)
merged.Selectors[0].NestingSelectorLoc = nestingSelectorLoc
r.Selectors = []css_ast.ComplexSelector{merged}
}

// Pass 2: Substitute "&" for the parent selector
Expand Down
32 changes: 16 additions & 16 deletions internal/css_parser/css_parser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1186,22 +1186,22 @@ func TestNestedSelector(t *testing.T) {
expectPrintedLowerUnsupported(t, everything, ".foo, [bar~='abc'] { .baz { color: red } }", ".foo .baz,\n[bar~=abc] .baz {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".foo, [bar~='a b c'] { .baz { color: red } }", ":is(.foo, [bar~=\"a b c\"]) .baz {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, everything, ".foo, [bar~='a b c'] { .baz { color: red } }", ".foo .baz,\n[bar~=\"a b c\"] .baz {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".baz { .foo, .bar { color: red } }", ".baz :is(.foo, .bar) {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".baz { .foo, .bar { color: red } }", ".baz .foo,\n.baz .bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, everything, ".baz { .foo, .bar { color: red } }", ".baz .foo,\n.baz .bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".baz { .foo, & .bar { color: red } }", ".baz :is(.foo, .bar) {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".baz { .foo, & .bar { color: red } }", ".baz .foo,\n.baz .bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, everything, ".baz { .foo, & .bar { color: red } }", ".baz .foo,\n.baz .bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".baz { & .foo, .bar { color: red } }", ".baz :is(.foo, .bar) {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".baz { & .foo, .bar { color: red } }", ".baz .foo,\n.baz .bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, everything, ".baz { & .foo, .bar { color: red } }", ".baz .foo,\n.baz .bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".baz { & .foo, & .bar { color: red } }", ".baz :is(.foo, .bar) {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".baz { & .foo, & .bar { color: red } }", ".baz .foo,\n.baz .bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, everything, ".baz { & .foo, & .bar { color: red } }", ".baz .foo,\n.baz .bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".baz { .foo, &.bar { color: red } }", ".baz .foo,\n.baz.bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, everything, ".baz { &.foo, .bar { color: red } }", ".baz.foo,\n.baz .bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".baz { &.foo, &.bar { color: red } }", ".baz:is(.foo, .bar) {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".baz { &.foo, &.bar { color: red } }", ".baz.foo,\n.baz.bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, everything, ".baz { &.foo, &.bar { color: red } }", ".baz.foo,\n.baz.bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".foo { color: blue; & .bar { color: red } }", ".foo {\n color: blue;\n}\n.foo .bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".foo { & .bar { color: red } color: blue }", ".foo {\n color: blue;\n}\n.foo .bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".foo { color: blue; & .bar { color: red } zoom: 2 }", ".foo {\n color: blue;\n zoom: 2;\n}\n.foo .bar {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".a, .b { .c, .d { color: red } }", ":is(.a, .b) :is(.c, .d) {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".a, .b { .c, .d { color: red } }", ":is(.a, .b) .c,\n:is(.a, .b) .d {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, everything, ".a, .b { .c, .d { color: red } }", ".a .c,\n.a .d,\n.b .c,\n.b .d {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".a, .b { & > & { color: red } }", ":is(.a, .b) > :is(.a, .b) {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, everything, ".a, .b { & > & { color: red } }", ".a > .a,\n.a > .b,\n.b > .a,\n.b > .b {\n color: red;\n}\n", "")
Expand Down Expand Up @@ -1234,32 +1234,32 @@ func TestNestedSelector(t *testing.T) {
expectPrintedLowerUnsupported(t, everything, "&, a { .b { color: red } }", ":scope .b,\na .b {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "&, a { .b { .c { color: red } } }", ":is(:scope, a) .b .c {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, everything, "&, a { .b { .c { color: red } } }", ":scope .b .c,\na .b .c {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "a { > b, > c { color: red } }", "a > :is(b, c) {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "a { > b, > c { color: red } }", "a > b,\na > c {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, everything, "a { > b, > c { color: red } }", "a > b,\na > c {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "a { > b, + c { color: red } }", "a > b,\na + c {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "a { & > b, & > c { color: red } }", "a > :is(b, c) {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "a { & > b, & > c { color: red } }", "a > b,\na > c {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, everything, "a { & > b, & > c { color: red } }", "a > b,\na > c {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "a { & > b, & + c { color: red } }", "a > b,\na + c {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "a { > b&, > c& { color: red } }", "a > :is(a:is(b), a:is(c)) {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "a { > b&, > c& { color: red } }", "a > a:is(b),\na > a:is(c) {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, everything, "a { > b&, > c& { color: red } }", "a > a:is(b),\na > a:is(c) {\n color: red;\n}\n", nestingWarningIs+nestingWarningIs)
expectPrintedLowerUnsupported(t, nesting, "a { > b&, + c& { color: red } }", "a > a:is(b),\na + a:is(c) {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "a { > &.b, > &.c { color: red } }", "a > :is(a.b, a.c) {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "a { > &.b, > &.c { color: red } }", "a > a.b,\na > a.c {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, everything, "a { > &.b, > &.c { color: red } }", "a > a.b,\na > a.c {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "a { > &.b, + &.c { color: red } }", "a > a.b,\na + a.c {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".a { > b&, > c& { color: red } }", ".a > :is(b.a, c.a) {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".a { > b&, > c& { color: red } }", ".a > b.a,\n.a > c.a {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, everything, ".a { > b&, > c& { color: red } }", ".a > b.a,\n.a > c.a {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".a { > b&, + c& { color: red } }", ".a > b.a,\n.a + c.a {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".a { > &.b, > &.c { color: red } }", ".a > :is(.a.b, .a.c) {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".a { > &.b, > &.c { color: red } }", ".a > .a.b,\n.a > .a.c {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, everything, ".a { > &.b, > &.c { color: red } }", ".a > .a.b,\n.a > .a.c {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".a { > &.b, + &.c { color: red } }", ".a > .a.b,\n.a + .a.c {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "~ .a { > &.b, > &.c { color: red } }", "~ .a > :is(.a.b, .a.c) {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "~ .a { > &.b, > &.c { color: red } }", "~ .a > .a.b,\n~ .a > .a.c {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, everything, "~ .a { > &.b, > &.c { color: red } }", "~ .a > .a.b,\n~ .a > .a.c {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, "~ .a { > &.b, + &.c { color: red } }", "~ .a > .a.b,\n~ .a + .a.c {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".foo .bar { > &.a, > &.b { color: red } }", ".foo .bar > :is(.foo .bar.a, .foo .bar.b) {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".foo .bar { > &.a, > &.b { color: red } }", ".foo .bar > :is(.foo .bar).a,\n.foo .bar > :is(.foo .bar).b {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, everything, ".foo .bar { > &.a, > &.b { color: red } }", ".foo .bar > :is(.foo .bar).a,\n.foo .bar > :is(.foo .bar).b {\n color: red;\n}\n", nestingWarningIs+nestingWarningIs)
expectPrintedLowerUnsupported(t, nesting, ".foo .bar { > &.a, + &.b { color: red } }", ".foo .bar > :is(.foo .bar).a,\n.foo .bar + :is(.foo .bar).b {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".demo { .lg { &.triangle, &.circle { color: red } } }", ".demo .lg:is(.triangle, .circle) {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".demo { .lg { .triangle, .circle { color: red } } }", ".demo .lg :is(.triangle, .circle) {\n color: red;\n}\n", "")
expectPrintedLowerUnsupported(t, nesting, ".demo { .lg { &.triangle, &.circle { color: red } } }", ".demo .lg.triangle,\n.demo .lg.circle {\n color: red;\n}\n", "")
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", "")

// These are invalid SASS-style nested suffixes
Expand Down
1 change: 0 additions & 1 deletion scripts/browser/index.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,6 @@

// See: https://github.com/evanw/esbuild/pull/4037
async cssNestingIssue4037() {
return // TODO: Remove this
await assertSameColorsWithNestingTransform(esbuild, {
css: `
.parent {
Expand Down

0 comments on commit 31e7b4d

Please # to comment.