Skip to content

Commit

Permalink
css: nesting transform now avoids :is (#1945)
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Jul 19, 2023
1 parent 7fcbdb8 commit 47d4f89
Show file tree
Hide file tree
Showing 7 changed files with 315 additions and 123 deletions.
67 changes: 67 additions & 0 deletions CHANGELOG.md
View file
Expand Up @@ -2,6 +2,73 @@

## Unreleased

* Implement CSS nesting without `:is()` when possible ([#1945](https://github.com/evanw/esbuild/issues/1945))

Previously esbuild would always produce a warning when transforming nested CSS for a browser that doesn't support the `:is()` pseudo-class. This was because the nesting transform needs to generate an `:is()` in some complex cases which means the transformed CSS would then not work in that browser. However, the CSS nesting transform can often be done without generating an `:is()`. So with this release, esbuild will no longer warn when targeting browsers that don't support `:is()` in the cases where an `:is()` isn't needed to represent the nested CSS.

In addition, esbuild's nested CSS transform has been updated to avoid generating an `:is()` in cases where an `:is()` is preferable but there's a longer alternative that is also equivalent. This update means esbuild can now generate a combinatorial explosion of CSS for complex CSS nesting syntax when targeting browsers that don't support `:is()`. This combinatorial explosion is necessary to accurately represent the original semantics. For example:

```css
/* Original code */
.first,
.second,
.third {
& > & {
color: red;
}
}

/* Old output (with --target=chrome80) */
:is(.first, .second, .third) > :is(.first, .second, .third) {
color: red;
}

/* New output (with --target=chrome80) */
.first > .first,
.first > .second,
.first > .third,
.second > .first,
.second > .second,
.second > .third,
.third > .first,
.third > .second,
.third > .third {
color: red;
}
```

This change means you can now use CSS nesting with esbuild when targeting an older browser that doesn't support `:is()`. You'll now only get a warning from esbuild if you use complex CSS nesting syntax that esbuild can't represent in that older browser without using `:is()`. There are two such cases:

```css
/* Case 1 */
a b {
.foo & {
color: red;
}
}

/* Case 2 */
a {
> b& {
color: red;
}
}
```

These two cases still need to use `:is()`, both for different reasons, and cannot be used when targeting an older browser that doesn't support `:is()`:

```css
/* Case 1 */
.foo :is(a b) {
color: red;
}

/* Case 2 */
a > a:is(b) {
color: red;
}
```

* Automatically lower `inset` in CSS for older browsers

With this release, esbuild will now automatically expand the `inset` property to the `top`, `right`, `bottom`, and `left` properties when esbuild's `target` is set to a browser that doesn't support `inset`:
Expand Down
36 changes: 11 additions & 25 deletions internal/bundler_tests/bundler_css_test.go
View file
Expand Up @@ -925,6 +925,10 @@ func TestCSSExternalQueryAndHashMatchIssue1822(t *testing.T) {
func TestCSSNestingOldBrowser(t *testing.T) {
css_suite.expectBundled(t, bundled{
files: map[string]string{
// These are now the only two cases that warn about ":is" not being supported
"/two-type-selectors.css": `a { .c b& { color: red; } }`,
"/two-parent-selectors.css": `a b { .c & { color: red; } }`,

"/nested-@layer.css": `a { @layer base { color: red; } }`,
"/nested-@media.css": `a { @media screen { color: red; } }`,
"/nested-ampersand-twice.css": `a { &, & { color: red; } }`,
Expand Down Expand Up @@ -963,6 +967,9 @@ func TestCSSNestingOldBrowser(t *testing.T) {
"/page-no-warning.css": `@page { @top-left { background: red } }`,
},
entryPaths: []string{
"/two-type-selectors.css",
"/two-parent-selectors.css",

"/nested-@layer.css",
"/nested-@media.css",
"/nested-ampersand-twice.css",
Expand Down Expand Up @@ -1005,31 +1012,10 @@ func TestCSSNestingOldBrowser(t *testing.T) {
UnsupportedCSSFeatures: compat.Nesting | compat.IsPseudoClass,
OriginalTargetEnv: "chrome10",
},
expectedScanLog: `media-ampersand-first.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
media-ampersand-second.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
media-ampersand-twice.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
media-ampersand-twice.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
media-greaterthan.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
media-plus.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
media-tilde.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
nested-@layer.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
nested-@media.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
nested-ampersand-first.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
nested-ampersand-twice.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
nested-attribute.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
nested-colon.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
nested-dot.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
nested-greaterthan.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
nested-hash.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
nested-plus.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
nested-tilde.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
toplevel-ampersand-first.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
toplevel-ampersand-second.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
toplevel-ampersand-twice.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
toplevel-ampersand-twice.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
toplevel-greaterthan.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
toplevel-plus.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
toplevel-tilde.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10)
expectedScanLog: `two-parent-selectors.css: WARNING: Transforming this CSS nesting syntax is not supported in the configured target environment (chrome10)
NOTE: The nesting transform for this case must generate an ":is(...)" but the configured target environment does not support the ":is" pseudo-class.
two-type-selectors.css: WARNING: Transforming this CSS nesting syntax is not supported in the configured target environment (chrome10)
NOTE: The nesting transform for this case must generate an ":is(...)" but the configured target environment does not support the ":is" pseudo-class.
`,
})
}
Expand Down
12 changes: 12 additions & 0 deletions internal/bundler_tests/snapshots/snapshots_css.txt
View file
Expand Up @@ -180,6 +180,18 @@ console.log(void 0);

================================================================================
TestCSSNestingOldBrowser
---------- /out/two-type-selectors.css ----------
/* two-type-selectors.css */
.c a:is(b) {
color: red;
}

---------- /out/two-parent-selectors.css ----------
/* two-parent-selectors.css */
.c :is(a b) {
color: red;
}

---------- /out/nested-@layer.css ----------
/* nested-@layer.css */
@layer base {
Expand Down
126 changes: 119 additions & 7 deletions internal/css_parser/css_nesting.go
View file
@@ -1,7 +1,10 @@
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 @@ -153,6 +156,12 @@ func (p *parser) lowerNestingInRuleWithContext(rule css_ast.Rule, context *lower
}
}

// 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) {}"
Expand All @@ -179,14 +188,104 @@ func (p *parser) lowerNestingInRuleWithContext(rule css_ast.Rule, context *lower
}

// Pass 2: Substitue "&" for the parent selector
for i := range r.Selectors {
complex := &r.Selectors[i]
results := make([]css_ast.CompoundSelector, 0, len(complex.Selectors))
parent := p.multipleComplexSelectorsToSingleComplexSelector(context.parentSelectors)
for _, compound := range complex.Selectors {
results = p.substituteAmpersandsInCompoundSelector(compound, parent, results, keepLeadingCombinator)
if !p.options.unsupportedCSSFeatures.Has(compat.IsPseudoClass) || len(context.parentSelectors) <= 1 {
// If we can use ":is", or we don't have to because there's only one
// parent selector, or we are using ":is()" to match zero parent selectors
// (even if ":is" is unsupported), then substituting "&" for the parent
// selector is easy.
for i := range r.Selectors {
complex := &r.Selectors[i]
results := make([]css_ast.CompoundSelector, 0, len(complex.Selectors))
parent := p.multipleComplexSelectorsToSingleComplexSelector(context.parentSelectors)
for _, compound := range complex.Selectors {
results = p.substituteAmpersandsInCompoundSelector(compound, parent, results, keepLeadingCombinator)
}
complex.Selectors = results
}
} else {
// Otherwise if we can't use ":is", the transform is more complicated.
// Avoiding ":is" can lead to a combinatorial explosion of cases so we
// want to avoid this if possible. For example:
//
// .first, .second, .third {
// & > & {
// color: red;
// }
// }
//
// If we can use ":is" (the easy case above) then we can do this:
//
// :is(.first, .second, .third) > :is(.first, .second, .third) {
// color: red;
// }
//
// But if we can't use ":is" then we have to do this instead:
//
// .first > .first,
// .first > .second,
// .first > .third,
// .second > .first,
// .second > .second,
// .second > .third,
// .third > .first,
// .third > .second,
// .third > .third {
// color: red;
// }
//
// That combinatorial explosion is what the loop below implements. Note
// that PostCSS's implementation of nesting gets this wrong. It generates
// this instead:
//
// .first > .first,
// .second > .second,
// .third > .third {
// color: red;
// }
//
// That's not equivalent, so that's an incorrect transformation.
var selectors []css_ast.ComplexSelector
var indices []int
for {
// Every time we encounter another "&", add another dimension
offset := 0
parent := func(loc logger.Loc) css_ast.ComplexSelector {
if offset == len(indices) {
indices = append(indices, 0)
}
index := indices[offset]
offset++
return context.parentSelectors[index]
}

// Do the substitution for this particular combination
for i := range r.Selectors {
complex := r.Selectors[i]
results := make([]css_ast.CompoundSelector, 0, len(complex.Selectors))
for _, compound := range complex.Selectors {
results = p.substituteAmpersandsInCompoundSelector(compound, parent, results, keepLeadingCombinator)
}
complex.Selectors = results
selectors = append(selectors, complex)
offset = 0
}

// Do addition with carry on the indices across dimensions
carry := len(indices)
for carry > 0 {
index := &indices[carry-1]
if *index+1 < len(context.parentSelectors) {
*index++
break
}
*index = 0
carry--
}
if carry == 0 {
break
}
}
complex.Selectors = results
r.Selectors = selectors
}

// Lower all child rules using our newly substituted selector
Expand Down Expand Up @@ -275,6 +374,7 @@ func (p *parser) substituteAmpersandsInCompoundSelector(
} else {
// ".foo .bar { :hover & {} }" => ":hover :is(.foo .bar) {}"
// ".foo .bar { > &:hover {} }" => ".foo .bar > :is(.foo .bar):hover {}"
p.reportNestingWithGeneratedPseudoClassIs(logger.Range{Loc: nestingSelectorLoc, Len: 1})
single = css_ast.CompoundSelector{
SubclassSelectors: []css_ast.SubclassSelector{{
Loc: nestingSelectorLoc,
Expand All @@ -291,6 +391,7 @@ func (p *parser) substituteAmpersandsInCompoundSelector(
// Insert the type selector
if single.TypeSelector != nil {
if sel.TypeSelector != nil {
p.reportNestingWithGeneratedPseudoClassIs(logger.Range{Loc: nestingSelectorLoc, Len: 1})
subclassSelectorPrefix = append(subclassSelectorPrefix, css_ast.SubclassSelector{
Loc: sel.TypeSelector.FirstLoc(),
Data: &css_ast.SSPseudoClassWithSelectorList{
Expand Down Expand Up @@ -363,3 +464,14 @@ func (p *parser) multipleComplexSelectorsToSingleComplexSelector(selectors []css
}
}
}

func (p *parser) reportNestingWithGeneratedPseudoClassIs(r logger.Range) {
if p.options.unsupportedCSSFeatures.Has(compat.IsPseudoClass) {
text := "Transforming this CSS nesting syntax is not supported in the configured target environment"
if p.options.originalTargetEnv != "" {
text = fmt.Sprintf("%s (%s)", text, p.options.originalTargetEnv)
}
p.log.AddIDWithNotes(logger.MsgID_CSS_UnsupportedCSSNesting, logger.Warning, &p.tracker, r, text, []logger.MsgData{{
Text: "The nesting transform for this case must generate an \":is(...)\" but the configured target environment does not support the \":is\" pseudo-class."}})
}
}
12 changes: 0 additions & 12 deletions internal/css_parser/css_parser.go
View file
Expand Up @@ -506,7 +506,6 @@ func (p *parser) parseListOfDeclarations(opts listOfDeclarationsOpts) (list []cs
case css_lexer.TAtKeyword:
if p.inSelectorSubtree > 0 {
p.shouldLowerNesting = true
p.reportUseOfNesting(p.current().Range, false)
}
list = append(list, p.parseAtRule(atRuleContext{
isDeclarationList: true,
Expand All @@ -525,7 +524,6 @@ func (p *parser) parseListOfDeclarations(opts listOfDeclarationsOpts) (list []cs
css_lexer.TDelimGreaterThan,
css_lexer.TDelimTilde:
p.shouldLowerNesting = true
p.reportUseOfNesting(p.current().Range, false)
list = append(list, p.parseSelectorRuleFrom(p.index, false, parseSelectorOpts{isDeclarationContext: true}))
foundNesting = true

Expand Down Expand Up @@ -1475,16 +1473,6 @@ func (p *parser) expectValidLayerNameIdent() (string, bool) {
return text, true
}

func (p *parser) reportUseOfNesting(r logger.Range, didWarnAlready bool) {
if p.options.unsupportedCSSFeatures.Has(compat.Nesting) && p.options.unsupportedCSSFeatures.Has(compat.IsPseudoClass) && !didWarnAlready {
text := "CSS nesting syntax is not supported in the configured target environment"
if p.options.originalTargetEnv != "" {
text = fmt.Sprintf("%s (%s)", text, p.options.originalTargetEnv)
}
p.log.AddID(logger.MsgID_CSS_UnsupportedCSSNesting, logger.Warning, &p.tracker, r, text)
}
}

func (p *parser) convertTokens(tokens []css_lexer.Token) []css_ast.Token {
result, _ := p.convertTokensHelper(tokens, css_lexer.TEndOfFile, convertTokensOpts{})
return result
Expand Down
6 changes: 1 addition & 5 deletions internal/css_parser/css_parser_selector.go
View file
Expand Up @@ -258,11 +258,9 @@ type parseComplexSelectorOpts struct {

func (p *parser) parseComplexSelector(opts parseComplexSelectorOpts) (result css_ast.ComplexSelector, ok bool) {
// This is an extension: https://drafts.csswg.org/css-nesting-1/
r := p.current().Range
combinator := p.parseCombinator()
if combinator.Byte != 0 {
p.shouldLowerNesting = true
p.reportUseOfNesting(r, opts.isDeclarationContext)
p.eat(css_lexer.TWhitespace)
}

Expand Down Expand Up @@ -324,7 +322,6 @@ func (p *parser) parseCompoundSelector(opts parseComplexSelectorOpts) (sel css_a
hasLeadingNestingSelector := p.peek(css_lexer.TDelimAmpersand)
if hasLeadingNestingSelector {
p.shouldLowerNesting = true
p.reportUseOfNesting(p.current().Range, opts.isDeclarationContext)
sel.NestingSelectorLoc = ast.MakeIndex32(uint32(startLoc.Start))
p.advance()
}
Expand Down Expand Up @@ -439,7 +436,6 @@ subclassSelectors:
case css_lexer.TDelimAmpersand:
// This is an extension: https://drafts.csswg.org/css-nesting-1/
p.shouldLowerNesting = true
p.reportUseOfNesting(subclassToken.Range, sel.HasNestingSelector())
sel.NestingSelectorLoc = ast.MakeIndex32(uint32(subclassToken.Range.Loc.Start))
p.advance()

Expand Down Expand Up @@ -468,7 +464,7 @@ subclassSelectors:
suggestion := p.source.TextForRange(r)
if opts.isFirst {
suggestion = fmt.Sprintf(":is(%s)", suggestion)
howToFix = "You can wrap this selector in \":is()\" as a workaround. "
howToFix = "You can wrap this selector in \":is(...)\" as a workaround. "
} else {
r = logger.Range{Loc: startLoc, Len: r.End() - startLoc.Start}
suggestion += "&"
Expand Down

0 comments on commit 47d4f89

Please sign in to comment.