From 9851b5aa6f05e90da2f508f7d74518e18fae928d Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Thu, 20 Jan 2022 15:22:57 +0000 Subject: [PATCH] warn when CSS nesting syntax is used (#1945) --- CHANGELOG.md | 4 +++ internal/bundler/bundler.go | 1 + internal/bundler/bundler_css_test.go | 20 +++++++++++ internal/bundler/snapshots/snapshots_css.txt | 10 ++++++ internal/compat/css_table.go | 4 +++ internal/css_parser/css_parser.go | 1 + internal/css_parser/css_parser_selector.go | 36 +++++++++++++++++--- internal/css_parser/css_parser_test.go | 5 +++ 8 files changed, 77 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e0412c5d14..514f088de11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,10 @@ .bad{@import url(other);}.red{background:red} ``` +* Warn about CSS nesting syntax ([#1945](https://github.com/evanw/esbuild/issues/1945)) + + There's a proposed [CSS syntax for nesting rules](https://drafts.csswg.org/css-nesting/) using the `&` selector, but it's not currently implemented in any browser. Previously esbuild silently passed the syntax through untransformed. With this release, esbuild will now warn when you use nesting syntax with a `--target=` setting that includes a browser. + ## 0.14.11 * Fix a bug with enum inlining ([#1903](https://github.com/evanw/esbuild/issues/1903)) diff --git a/internal/bundler/bundler.go b/internal/bundler/bundler.go index dc6d0ceb752..89a862fab03 100644 --- a/internal/bundler/bundler.go +++ b/internal/bundler/bundler.go @@ -224,6 +224,7 @@ func parseFile(args parseArgs) { MangleSyntax: args.options.MangleSyntax, RemoveWhitespace: args.options.RemoveWhitespace, UnsupportedCSSFeatures: args.options.UnsupportedCSSFeatures, + OriginalTargetEnv: args.options.OriginalTargetEnv, }) result.file.inputFile.Repr = &graph.CSSRepr{AST: ast} result.ok = true diff --git a/internal/bundler/bundler_css_test.go b/internal/bundler/bundler_css_test.go index f87bb248936..fe101d85c3c 100644 --- a/internal/bundler/bundler_css_test.go +++ b/internal/bundler/bundler_css_test.go @@ -3,6 +3,7 @@ package bundler import ( "testing" + "github.com/evanw/esbuild/internal/compat" "github.com/evanw/esbuild/internal/config" ) @@ -686,3 +687,22 @@ func TestCSSExternalQueryAndHashMatchIssue1822(t *testing.T) { }, }) } + +func TestCSSNestingOldBrowser(t *testing.T) { + css_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.css": ` + a { &:hover { color: red; } } + `, + }, + entryPaths: []string{"/entry.css"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputFile: "/out.css", + UnsupportedCSSFeatures: compat.Nesting, + OriginalTargetEnv: "chrome10", + }, + expectedScanLog: `entry.css: WARNING: CSS nesting syntax is not supported in the configured target environment (chrome10) +`, + }) +} diff --git a/internal/bundler/snapshots/snapshots_css.txt b/internal/bundler/snapshots/snapshots_css.txt index 1ca0219ea82..1b83d532614 100644 --- a/internal/bundler/snapshots/snapshots_css.txt +++ b/internal/bundler/snapshots/snapshots_css.txt @@ -178,6 +178,16 @@ console.log(void 0); color: red; } +================================================================================ +TestCSSNestingOldBrowser +---------- /out.css ---------- +/* entry.css */ +a { + &:hover { + color: red; + } +} + ================================================================================ TestDataURLImportURLInCSS ---------- /out/entry.css ---------- diff --git a/internal/compat/css_table.go b/internal/compat/css_table.go index 5c53a9eeff6..e1fc423e7d0 100644 --- a/internal/compat/css_table.go +++ b/internal/compat/css_table.go @@ -15,6 +15,7 @@ const ( Modern_RGB_HSL InsetProperty + Nesting ) func (features CSSFeature) Has(feature CSSFeature) bool { @@ -53,6 +54,9 @@ var cssTable = map[CSSFeature]map[Engine][]versionRange{ IOS: {{start: v{14, 5, 0}}}, Safari: {{start: v{14, 1, 0}}}, }, + + // This isn't supported anywhere right now: https://caniuse.com/css-nesting + Nesting: {}, } // Return all features that are not available in at least one environment diff --git a/internal/css_parser/css_parser.go b/internal/css_parser/css_parser.go index 854f35870bf..75d7ef8c30c 100644 --- a/internal/css_parser/css_parser.go +++ b/internal/css_parser/css_parser.go @@ -30,6 +30,7 @@ type parser struct { } type Options struct { + OriginalTargetEnv string UnsupportedCSSFeatures compat.CSSFeature MangleSyntax bool RemoveWhitespace bool diff --git a/internal/css_parser/css_parser_selector.go b/internal/css_parser/css_parser_selector.go index bce97ce0c28..b63f7ccaef2 100644 --- a/internal/css_parser/css_parser_selector.go +++ b/internal/css_parser/css_parser_selector.go @@ -1,14 +1,18 @@ package css_parser import ( + "fmt" + + "github.com/evanw/esbuild/internal/compat" "github.com/evanw/esbuild/internal/css_ast" "github.com/evanw/esbuild/internal/css_lexer" + "github.com/evanw/esbuild/internal/logger" ) func (p *parser) parseSelectorList() (list []css_ast.ComplexSelector, ok bool) { // Parse the first selector - p.eat(css_lexer.TWhitespace) - sel, good := p.parseComplexSelector() + firstRange := p.current().Range + sel, good, firstHasNestPrefix := p.parseComplexSelector() if !good { return } @@ -21,23 +25,36 @@ func (p *parser) parseSelectorList() (list []css_ast.ComplexSelector, ok bool) { break } p.eat(css_lexer.TWhitespace) - sel, good := p.parseComplexSelector() + loc := p.current().Range.Loc + sel, good, hasNestPrefix := p.parseComplexSelector() if !good { return } list = append(list, sel) + + // Validate nest prefix consistency + if firstHasNestPrefix && !hasNestPrefix { + data := p.tracker.MsgData(logger.Range{Loc: loc}, "Every selector in a nested style rule must start with \"&\"") + data.Location.Suggestion = "&" + p.log.AddMsg(logger.Msg{ + Kind: logger.Warning, + Data: data, + Notes: []logger.MsgData{p.tracker.MsgData(firstRange, "This is a nested style rule because of the \"&\" here:")}, + }) + } } ok = true return } -func (p *parser) parseComplexSelector() (result css_ast.ComplexSelector, ok bool) { +func (p *parser) parseComplexSelector() (result css_ast.ComplexSelector, ok bool, hasNestPrefix bool) { // Parent sel, good := p.parseCompoundSelector() if !good { return } + hasNestPrefix = sel.HasNestPrefix result.Selectors = append(result.Selectors, sel) for { @@ -74,8 +91,19 @@ func (p *parser) nameToken() css_ast.NameToken { func (p *parser) parseCompoundSelector() (sel css_ast.CompoundSelector, ok bool) { // This is an extension: https://drafts.csswg.org/css-nesting-1/ + r := p.current().Range if p.eat(css_lexer.TDelimAmpersand) { sel.HasNestPrefix = true + + // Warn if we're targeting a browser, since it won't work + if p.options.UnsupportedCSSFeatures.Has(compat.Nesting) { + where := "the configured target environment" + if p.options.OriginalTargetEnv != "" { + where = fmt.Sprintf("%s (%s)", where, p.options.OriginalTargetEnv) + } + p.log.Add(logger.Warning, &p.tracker, r, fmt.Sprintf( + "CSS nesting syntax is not supported in %s", where)) + } } // Parse the type selector diff --git a/internal/css_parser/css_parser_test.go b/internal/css_parser/css_parser_test.go index 074474af06e..e74c43cfdab 100644 --- a/internal/css_parser/css_parser_test.go +++ b/internal/css_parser/css_parser_test.go @@ -688,6 +688,11 @@ func TestNestedSelector(t *testing.T) { expectPrinted(t, "a { &*|b {} }", "a {\n &*|b {\n }\n}\n") expectPrinted(t, "a { &a|b {} }", "a {\n &a|b {\n }\n}\n") expectPrinted(t, "a { &[b] {} }", "a {\n &[b] {\n }\n}\n") + + expectParseError(t, "a { & b, c {} }", + ": WARNING: Every selector in a nested style rule must start with \"&\"\n"+ + ": NOTE: This is a nested style rule because of the \"&\" here:\n") + expectParseError(t, "a { & b, & c {} }", "") } func TestBadQualifiedRules(t *testing.T) {