From e76780c041b15fbf5d273861af2a95707f1621f1 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Sun, 13 Aug 2023 01:53:32 -0400 Subject: [PATCH] css: further changes to css nesting syntax --- CHANGELOG.md | 16 +++ internal/css_parser/css_parser.go | 167 +++++++++++++---------- internal/css_parser/css_parser_test.go | 38 +++--- internal/css_printer/css_printer_test.go | 26 ++-- 4 files changed, 143 insertions(+), 104 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b3367aa9b7..273c93a458d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,22 @@ ## Unreleased +* Update how CSS nesting is parsed again + + CSS nesting syntax has been changed again, and esbuild has been updated to match. Type selectors may now be used with CSS nesting: + + ```css + .foo { + div { + color: red; + } + } + ``` + + Previously this was disallowed in the CSS specification because it's ambiguous whether an identifier is a declaration or a nested rule starting with a type selector without requiring unbounded lookahead in the parser. It has now been allowed because the CSS working group has decided that requiring unbounded lookahead is acceptible after all. + + Note that this change means esbuild no longer considers any existing browser to support CSS nesting since none of the existing browsers support this new syntax. CSS nesting will now always be transformed when targeting a browser. This situation will change in the future as browsers add support for this new syntax. + * Make renamed CSS names unique across entry points ([#3295](https://github.com/evanw/esbuild/issues/3295)) Previously esbuild's generated names for local names in CSS were only unique within a given entry point (or across all entry points when code splitting was enabled). That meant that building multiple entry points with esbuild could result in local names being renamed to the same identifier even when those entry points were built simultaneously within a single esbuild API call. This problem was especially likely to happen with minification enabled. With this release, esbuild will now avoid renaming local names from two separate entry points to the same name if those entry points were built with a single esbuild API call, even when code splitting is disabled. diff --git a/internal/css_parser/css_parser.go b/internal/css_parser/css_parser.go index 7cfbaa47518..ebbd88726cb 100644 --- a/internal/css_parser/css_parser.go +++ b/internal/css_parser/css_parser.go @@ -36,7 +36,6 @@ type parser struct { enclosingLayer []string anonLayerCount int index int - end int legalCommentIndex int inSelectorSubtree int prevError logger.Loc @@ -138,7 +137,6 @@ func Parse(log logger.Log, source logger.Source, options Options) css_ast.AST { globalScope: make(map[string]ast.LocRef), makeLocalSymbols: options.symbolMode == symbolModeLocal, } - p.end = len(p.tokens) rules := p.parseListOfRules(ruleContext{ isTopLevel: true, parseSelectors: true, @@ -196,21 +194,15 @@ func (p *parser) computeCharacterFrequency() *ast.CharFreq { } func (p *parser) advance() { - if p.index < p.end { + if p.index < len(p.tokens) { p.index++ } } func (p *parser) at(index int) css_lexer.Token { - if index < p.end { + if index < len(p.tokens) { return p.tokens[index] } - if p.end < len(p.tokens) { - return css_lexer.Token{ - Kind: css_lexer.TEndOfFile, - Range: logger.Range{Loc: p.tokens[p.end].Range.Loc}, - } - } return css_lexer.Token{ Kind: css_lexer.TEndOfFile, Range: logger.Range{Loc: logger.Loc{Start: int32(len(p.source.Contents))}}, @@ -475,6 +467,18 @@ loop: atRuleContext.importValidity = atRuleInvalidAfter } + // Note: CSS recently changed to parse and discard declarations + // here instead of treating them as the start of a qualified rule. + // See also: https://github.com/w3c/csswg-drafts/issues/8834 + if !context.isTopLevel { + if scan, index := p.scanForEndOfRule(); scan == endOfRuleSemicolon { + tokens := p.convertTokens(p.tokens[p.index:index]) + rules = append(rules, css_ast.Rule{Loc: p.current().Range.Loc, Data: &css_ast.RBadDeclaration{Tokens: tokens}}) + p.index = index + 1 + continue + } + } + var rule css_ast.Rule if context.parseSelectors { rule = p.parseSelectorRule(context.isTopLevel, parseSelectorOpts{}) @@ -550,41 +554,33 @@ func (p *parser) parseListOfDeclarations(opts listOfDeclarationsOpts) (list []cs })) // Reference: https://drafts.csswg.org/css-nesting-1/ - case css_lexer.TDelimAmpersand, - css_lexer.TDelimDot, - css_lexer.THash, - css_lexer.TColon, - css_lexer.TOpenBracket, - css_lexer.TDelimAsterisk, - css_lexer.TDelimBar, - css_lexer.TDelimPlus, - css_lexer.TDelimGreaterThan, - css_lexer.TDelimTilde: - p.nestingIsPresent = true - foundNesting = true - rule := p.parseSelectorRule(false, parseSelectorOpts{ - isDeclarationContext: true, - composesContext: opts.composesContext, - }) + default: + if scan, _ := p.scanForEndOfRule(); scan == endOfRuleOpenBrace { + p.nestingIsPresent = true + foundNesting = true + rule := p.parseSelectorRule(false, parseSelectorOpts{ + isDeclarationContext: true, + composesContext: opts.composesContext, + }) - // If this rule was a single ":global" or ":local", inline it here. This - // is handled differently than a bare "&" with normal CSS nesting because - // that would be inlined at the end of the parent rule's body instead, - // which is probably unexpected (e.g. it would trip people up when trying - // to write rules in a specific order). - if sel, ok := rule.Data.(*css_ast.RSelector); ok && len(sel.Selectors) == 1 { - if first := sel.Selectors[0]; len(first.Selectors) == 1 { - if first := first.Selectors[0]; first.WasEmptyFromLocalOrGlobal && first.IsSingleAmpersand() { - list = append(list, sel.Rules...) - continue + // If this rule was a single ":global" or ":local", inline it here. This + // is handled differently than a bare "&" with normal CSS nesting because + // that would be inlined at the end of the parent rule's body instead, + // which is probably unexpected (e.g. it would trip people up when trying + // to write rules in a specific order). + if sel, ok := rule.Data.(*css_ast.RSelector); ok && len(sel.Selectors) == 1 { + if first := sel.Selectors[0]; len(first.Selectors) == 1 { + if first := first.Selectors[0]; first.WasEmptyFromLocalOrGlobal && first.IsSingleAmpersand() { + list = append(list, sel.Rules...) + continue + } } } - } - - list = append(list, rule) - default: - list = append(list, p.parseDeclaration()) + list = append(list, rule) + } else { + list = append(list, p.parseDeclaration()) + } } } } @@ -2168,6 +2164,56 @@ loop: return css_ast.Rule{Loc: preludeLoc, Data: &qualified} } +type endOfRuleScan uint8 + +const ( + endOfRuleUnknown endOfRuleScan = iota + endOfRuleSemicolon + endOfRuleOpenBrace +) + +// Note: This was a late change to the CSS nesting syntax. +// See also: https://github.com/w3c/csswg-drafts/issues/7961 +func (p *parser) scanForEndOfRule() (endOfRuleScan, int) { + var initialStack [4]css_lexer.T + stack := initialStack[:0] + + for i, t := range p.tokens[p.index:] { + switch t.Kind { + case css_lexer.TSemicolon: + if len(stack) == 0 { + return endOfRuleSemicolon, p.index + i + } + + case css_lexer.TFunction, css_lexer.TOpenParen: + stack = append(stack, css_lexer.TCloseParen) + + case css_lexer.TOpenBracket: + stack = append(stack, css_lexer.TCloseBracket) + + case css_lexer.TOpenBrace: + if len(stack) == 0 { + return endOfRuleOpenBrace, p.index + i + } + stack = append(stack, css_lexer.TCloseBrace) + + case css_lexer.TCloseParen, css_lexer.TCloseBracket: + if n := len(stack); n > 0 && t.Kind == stack[n-1] { + stack = stack[:n-1] + } + + case css_lexer.TCloseBrace: + if n := len(stack); n > 0 && t.Kind == stack[n-1] { + stack = stack[:n-1] + } else { + return endOfRuleUnknown, -1 + } + } + } + + return endOfRuleUnknown, -1 +} + func (p *parser) parseDeclaration() css_ast.Rule { // Parse the key keyStart := p.index @@ -2181,17 +2227,12 @@ func (p *parser) parseDeclaration() css_ast.Rule { // Parse the value valueStart := p.index - foundOpenBrace := false stop: for { switch p.current().Kind { case css_lexer.TEndOfFile, css_lexer.TSemicolon, css_lexer.TCloseBrace: break stop - case css_lexer.TOpenBrace: - foundOpenBrace = true - p.parseComponentValue() - default: p.parseComponentValue() } @@ -2200,32 +2241,14 @@ stop: // Stop now if this is not a valid declaration if !ok { if keyIsIdent { - if foundOpenBrace { - // If we encountered a "{", assume this is someone trying to make a nested style rule - if keyRange.Loc.Start > p.prevError.Start { - p.prevError.Start = keyRange.Loc.Start - key := p.tokens[keyStart].DecodedText(p.source.Contents) - data := p.tracker.MsgData(keyRange, fmt.Sprintf("A nested style rule cannot start with %q because it looks like the start of a declaration", key)) - data.Location.Suggestion = fmt.Sprintf(":is(%s)", p.source.TextForRange(keyRange)) - p.log.AddMsgID(logger.MsgID_CSS_CSSSyntaxError, logger.Msg{ - Kind: logger.Warning, - Data: data, - Notes: []logger.MsgData{{ - Text: "To start a nested style rule with an identifier, you need to wrap the " + - "identifier in \":is(...)\" to prevent the rule from being parsed as a declaration."}}, - }) - } - } else { - // Otherwise, show a generic error about a missing ":" - if end := keyRange.End(); end > p.prevError.Start { - p.prevError.Start = end - data := p.tracker.MsgData(logger.Range{Loc: logger.Loc{Start: end}}, "Expected \":\"") - data.Location.Suggestion = ":" - p.log.AddMsgID(logger.MsgID_CSS_CSSSyntaxError, logger.Msg{ - Kind: logger.Warning, - Data: data, - }) - } + if end := keyRange.End(); end > p.prevError.Start { + p.prevError.Start = end + data := p.tracker.MsgData(logger.Range{Loc: logger.Loc{Start: end}}, "Expected \":\"") + data.Location.Suggestion = ":" + p.log.AddMsgID(logger.MsgID_CSS_CSSSyntaxError, logger.Msg{ + Kind: logger.Warning, + Data: data, + }) } } diff --git a/internal/css_parser/css_parser_test.go b/internal/css_parser/css_parser_test.go index 9571d6f9b87..91315e32a72 100644 --- a/internal/css_parser/css_parser_test.go +++ b/internal/css_parser/css_parser_test.go @@ -600,16 +600,12 @@ func TestDeclaration(t *testing.T) { expectPrinted(t, ".decl { a: b; }", ".decl {\n a: b;\n}\n", "") expectPrinted(t, ".decl { a: b; c: d }", ".decl {\n a: b;\n c: d;\n}\n", "") expectPrinted(t, ".decl { a: b; c: d; }", ".decl {\n a: b;\n c: d;\n}\n", "") - expectPrinted(t, ".decl { a { b: c; } }", ".decl {\n a { b: c; };\n}\n", - ": WARNING: A nested style rule cannot start with \"a\" because it looks like the start of a declaration\n"+ - "NOTE: To start a nested style rule with an identifier, you need to wrap the identifier in \":is(...)\" to prevent the rule from being parsed as a declaration.\n") + expectPrinted(t, ".decl { a { b: c; } }", ".decl {\n a {\n b: c;\n }\n}\n", "") expectPrinted(t, ".decl { & a { b: c; } }", ".decl {\n & a {\n b: c;\n }\n}\n", "") // See http://browserhacks.com/ - expectPrinted(t, ".selector { (;property: value;); }", ".selector {\n (;property: value;);\n}\n", - ": WARNING: Expected identifier but found \"(\"\n") - expectPrinted(t, ".selector { [;property: value;]; }", ".selector {\n [;property: value;];\n}\n", - ": WARNING: Expected identifier but found \";\"\n") // Note: This now overlaps with CSS nesting syntax + expectPrinted(t, ".selector { (;property: value;); }", ".selector {\n (;property: value;);\n}\n", ": WARNING: Expected identifier but found \"(\"\n") + expectPrinted(t, ".selector { [;property: value;]; }", ".selector {\n [;property: value;];\n}\n", ": WARNING: Expected identifier but found \"[\"\n") expectPrinted(t, ".selector, {}", ".selector, {\n}\n", ": WARNING: Unexpected \"{\"\n") expectPrinted(t, ".selector\\ {}", ".selector\\ {\n}\n", "") expectPrinted(t, ".selector { property: value\\9; }", ".selector {\n property: value\\\t;\n}\n", "") @@ -791,10 +787,8 @@ func TestNestedSelector(t *testing.T) { expectPrinted(t, "a { >b {} }", "a {\n > b {\n }\n}\n", "") expectPrinted(t, "a { +b {} }", "a {\n + b {\n }\n}\n", "") expectPrinted(t, "a { ~b {} }", "a {\n ~ b {\n }\n}\n", "") - expectPrinted(t, "a { b {} }", "a {\n b {};\n}\n", - ": WARNING: A nested style rule cannot start with \"b\" because it looks like the start of a declaration\n"+ - "NOTE: To start a nested style rule with an identifier, you need to wrap the identifier in \":is(...)\" to prevent the rule from being parsed as a declaration.\n") - expectPrinted(t, "a { b() {} }", "a {\n b() {};\n}\n", ": WARNING: Expected identifier but found \"b(\"\n") + expectPrinted(t, "a { b {} }", "a {\n b {\n }\n}\n", "") + expectPrinted(t, "a { b() {} }", "a {\n b() {\n }\n}\n", ": WARNING: Unexpected \"b(\"\n") // Note: CSS nesting no longer requires each complex selector to contain "&" expectPrinted(t, "a { & b, c {} }", "a {\n & b,\n c {\n }\n}\n", "") @@ -1033,20 +1027,23 @@ func TestNestedSelector(t *testing.T) { "@supports (selector(&)) {\n .card:hover {\n color: red;\n }\n}\n", "") expectPrintedLower(t, "html { @layer base { color: blue; @layer support { & body { color: red } } } }", "@layer base {\n html {\n color: blue;\n }\n @layer support {\n html body {\n color: red;\n }\n }\n}\n", "") + + // https://github.com/w3c/csswg-drafts/issues/7961#issuecomment-1549874958 + expectPrinted(t, "@media screen { a { x: y } x: y; b { x: y } }", "@media screen {\n a {\n x: y;\n }\n x: y;\n b {\n x: y;\n }\n}\n", "") + expectPrinted(t, ":root { @media screen { a { x: y } x: y; b { x: y } } }", ":root {\n @media screen {\n a {\n x: y;\n }\n x: y;\n b {\n x: y;\n }\n }\n}\n", "") } func TestBadQualifiedRules(t *testing.T) { expectPrinted(t, "$bad: rule;", "$bad: rule; {\n}\n", ": WARNING: Unexpected \"$\"\n") expectPrinted(t, "$bad: rule; div { color: red }", "$bad: rule; div {\n color: red;\n}\n", ": WARNING: Unexpected \"$\"\n") expectPrinted(t, "$bad { color: red }", "$bad {\n color: red;\n}\n", ": WARNING: Unexpected \"$\"\n") - expectPrinted(t, "a { div.major { color: blue } color: red }", "a {\n div.major { color: blue } color: red;\n}\n", - ": WARNING: A nested style rule cannot start with \"div\" because it looks like the start of a declaration\n"+ - "NOTE: To start a nested style rule with an identifier, you need to wrap the identifier in \":is(...)\" to prevent the rule from being parsed as a declaration.\n") - expectPrinted(t, "a { div:hover { color: blue } color: red }", "a {\n div: hover { color: blue } color: red;\n}\n", "") - expectPrinted(t, "a { div:hover { color: blue }; color: red }", "a {\n div: hover { color: blue };\n color: red;\n}\n", "") - expectPrinted(t, "a { div:hover { color: blue } ; color: red }", "a {\n div: hover { color: blue };\n color: red;\n}\n", "") - expectPrinted(t, "! { x: {} }", "! {\n x: {};\n}\n", ": WARNING: Unexpected \"!\"\n") - expectPrinted(t, "a { *width: 100%; height: 1px }", "a {\n *width: 100%;\n height: 1px;\n}\n", ": WARNING: Unexpected \"width\"\n") + expectPrinted(t, "a { div.major { color: blue } color: red }", "a {\n div.major {\n color: blue;\n }\n color: red;\n}\n", "") + expectPrinted(t, "a { div:hover { color: blue } color: red }", "a {\n div:hover {\n color: blue;\n }\n color: red;\n}\n", "") + expectPrinted(t, "a { div:hover { color: blue }; color: red }", "a {\n div:hover {\n color: blue;\n }\n color: red;\n}\n", "") + expectPrinted(t, "a { div:hover { color: blue } ; color: red }", "a {\n div:hover {\n color: blue;\n }\n color: red;\n}\n", "") + expectPrinted(t, "! { x: y; }", "! {\n x: y;\n}\n", ": WARNING: Unexpected \"!\"\n") + expectPrinted(t, "! { x: {} }", "! {\n x: {\n }\n}\n", ": WARNING: Unexpected \"!\"\n: WARNING: Expected identifier but found whitespace\n") + expectPrinted(t, "a { *width: 100%; height: 1px }", "a {\n *width: 100%;\n height: 1px;\n}\n", ": WARNING: Expected identifier but found \"*\"\n") expectPrinted(t, "a { garbage; height: 1px }", "a {\n garbage;\n height: 1px;\n}\n", ": WARNING: Expected \":\"\n") expectPrinted(t, "a { !; height: 1px }", "a {\n !;\n height: 1px;\n}\n", ": WARNING: Expected identifier but found \"!\"\n") } @@ -2251,7 +2248,8 @@ func TestParseErrorRecovery(t *testing.T) { expectPrinted(t, "x { y: z", "x {\n y: z;\n}\n", ": WARNING: Expected \"}\" to go with \"{\"\n: NOTE: The unbalanced \"{\" is here:\n") expectPrinted(t, "x { y: (", "x {\n y: ();\n}\n", ": WARNING: Expected \")\" to go with \"(\"\n: NOTE: The unbalanced \"(\" is here:\n") expectPrinted(t, "x { y: [", "x {\n y: [];\n}\n", ": WARNING: Expected \"]\" to go with \"[\"\n: NOTE: The unbalanced \"[\" is here:\n") - expectPrinted(t, "x { y: {", "x {\n y: {};\n}\n", ": WARNING: Expected \"}\" to go with \"{\"\n: NOTE: The unbalanced \"{\" is here:\n") + expectPrinted(t, "x { y: {", "x {\n y: {\n }\n}\n", + ": WARNING: Expected identifier but found whitespace\n: WARNING: Expected \"}\" to go with \"{\"\n: NOTE: The unbalanced \"{\" is here:\n") expectPrinted(t, "x { y: z(", "x {\n y: z();\n}\n", ": WARNING: Expected \")\" to go with \"(\"\n: NOTE: The unbalanced \"(\" is here:\n") expectPrinted(t, "x { y: z(abc", "x {\n y: z(abc);\n}\n", ": WARNING: Expected \")\" to go with \"(\"\n: NOTE: The unbalanced \"(\" is here:\n") expectPrinted(t, "x { y: url(", "x {\n y: url();\n}\n", diff --git a/internal/css_printer/css_printer_test.go b/internal/css_printer/css_printer_test.go index 76b3695503d..6d3e3b21682 100644 --- a/internal/css_printer/css_printer_test.go +++ b/internal/css_printer/css_printer_test.go @@ -155,9 +155,9 @@ func TestBadQualifiedRules(t *testing.T) { expectPrinted(t, ";", "; {\n}\n") expectPrinted(t, "$bad: rule;", "$bad: rule; {\n}\n") expectPrinted(t, "a {}; b {};", "a {\n}\n; b {\n}\n; {\n}\n") - expectPrinted(t, "a { div.major { color: blue } color: red }", "a {\n div.major { color: blue } color: red;\n}\n") - expectPrinted(t, "a { div:hover { color: blue } color: red }", "a {\n div: hover { color: blue } color: red;\n}\n") - expectPrinted(t, "a { div:hover { color: blue }; color: red }", "a {\n div: hover { color: blue };\n color: red;\n}\n") + expectPrinted(t, "a { div.major { color: blue } color: red }", "a {\n div.major {\n color: blue;\n }\n color: red;\n}\n") + expectPrinted(t, "a { div:hover { color: blue } color: red }", "a {\n div:hover {\n color: blue;\n }\n color: red;\n}\n") + expectPrinted(t, "a { div:hover { color: blue }; color: red }", "a {\n div:hover {\n color: blue;\n }\n color: red;\n}\n") expectPrinted(t, "$bad{ color: red }", "$bad {\n color: red;\n}\n") expectPrinted(t, "$bad { color: red }", "$bad {\n color: red;\n}\n") @@ -276,17 +276,19 @@ func TestVerbatimWhitespace(t *testing.T) { expectPrintedMinify(t, "* { --x:[y ]; }", "*{--x:[y ]}") expectPrintedMinify(t, "* { --x:[ y]; }", "*{--x:[ y]}") - expectPrinted(t, "* { --x:{y}; }", "* {\n --x:{y};\n}\n") - expectPrinted(t, "* { --x:{y} ; }", "* {\n --x:{y} ;\n}\n") - expectPrinted(t, "* { --x: {y}; }", "* {\n --x: {y};\n}\n") - expectPrinted(t, "* { --x:{y }; }", "* {\n --x:{y };\n}\n") - expectPrinted(t, "* { --x:{ y}; }", "* {\n --x:{ y};\n}\n") + // Note: These cases now behave like qualified rules + expectPrinted(t, "* { --x:{y}; }", "* {\n --x: {\n y;\n }\n}\n") + expectPrinted(t, "* { --x:{y} ; }", "* {\n --x: {\n y;\n }\n}\n") + expectPrinted(t, "* { --x: {y}; }", "* {\n --x: {\n y;\n }\n}\n") + expectPrinted(t, "* { --x:{y }; }", "* {\n --x: {\n y;\n }\n}\n") + expectPrinted(t, "* { --x:{ y}; }", "* {\n --x: {\n y;\n }\n}\n") + // Note: These cases now behave like qualified rules expectPrintedMinify(t, "* { --x:{y}; }", "*{--x:{y}}") - expectPrintedMinify(t, "* { --x:{y} ; }", "*{--x:{y} }") - expectPrintedMinify(t, "* { --x: {y}; }", "*{--x: {y}}") - expectPrintedMinify(t, "* { --x:{y }; }", "*{--x:{y }}") - expectPrintedMinify(t, "* { --x:{ y}; }", "*{--x:{ y}}") + expectPrintedMinify(t, "* { --x:{y} ; }", "*{--x:{y}}") + expectPrintedMinify(t, "* { --x: {y}; }", "*{--x:{y}}") + expectPrintedMinify(t, "* { --x:{y }; }", "*{--x:{y}}") + expectPrintedMinify(t, "* { --x:{ y}; }", "*{--x:{y}}") expectPrintedMinify(t, "@supports ( --x : y , z ) { a { color: red; } }", "@supports ( --x : y , z ){a{color:red}}") expectPrintedMinify(t, "@supports ( --x : ) { a { color: red; } }", "@supports ( --x : ){a{color:red}}")