Skip to content

Commit

Permalink
css: further changes to css nesting syntax
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Aug 13, 2023
1 parent 71f4a5a commit e76780c
Show file tree
Hide file tree
Showing 4 changed files with 143 additions and 104 deletions.
16 changes: 16 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
167 changes: 95 additions & 72 deletions internal/css_parser/css_parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ type parser struct {
enclosingLayer []string
anonLayerCount int
index int
end int
legalCommentIndex int
inSelectorSubtree int
prevError logger.Loc
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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))}},
Expand Down Expand Up @@ -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{})
Expand Down Expand Up @@ -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())
}
}
}
}
Expand Down Expand Up @@ -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
Expand All @@ -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()
}
Expand All @@ -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,
})
}
}

Expand Down
38 changes: 18 additions & 20 deletions internal/css_parser/css_parser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
"<stdin>: 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",
"<stdin>: WARNING: Expected identifier but found \"(\"\n")
expectPrinted(t, ".selector { [;property: value;]; }", ".selector {\n [;property: value;];\n}\n",
"<stdin>: 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", "<stdin>: WARNING: Expected identifier but found \"(\"\n")
expectPrinted(t, ".selector { [;property: value;]; }", ".selector {\n [;property: value;];\n}\n", "<stdin>: WARNING: Expected identifier but found \"[\"\n")
expectPrinted(t, ".selector, {}", ".selector, {\n}\n", "<stdin>: WARNING: Unexpected \"{\"\n")
expectPrinted(t, ".selector\\ {}", ".selector\\ {\n}\n", "")
expectPrinted(t, ".selector { property: value\\9; }", ".selector {\n property: value\\\t;\n}\n", "")
Expand Down Expand Up @@ -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",
"<stdin>: 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", "<stdin>: 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", "<stdin>: 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", "")
Expand Down Expand Up @@ -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", "<stdin>: WARNING: Unexpected \"$\"\n")
expectPrinted(t, "$bad: rule; div { color: red }", "$bad: rule; div {\n color: red;\n}\n", "<stdin>: WARNING: Unexpected \"$\"\n")
expectPrinted(t, "$bad { color: red }", "$bad {\n color: red;\n}\n", "<stdin>: WARNING: Unexpected \"$\"\n")
expectPrinted(t, "a { div.major { color: blue } color: red }", "a {\n div.major { color: blue } color: red;\n}\n",
"<stdin>: 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", "<stdin>: WARNING: Unexpected \"!\"\n")
expectPrinted(t, "a { *width: 100%; height: 1px }", "a {\n *width: 100%;\n height: 1px;\n}\n", "<stdin>: 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", "<stdin>: WARNING: Unexpected \"!\"\n")
expectPrinted(t, "! { x: {} }", "! {\n x: {\n }\n}\n", "<stdin>: WARNING: Unexpected \"!\"\n<stdin>: WARNING: Expected identifier but found whitespace\n")
expectPrinted(t, "a { *width: 100%; height: 1px }", "a {\n *width: 100%;\n height: 1px;\n}\n", "<stdin>: WARNING: Expected identifier but found \"*\"\n")
expectPrinted(t, "a { garbage; height: 1px }", "a {\n garbage;\n height: 1px;\n}\n", "<stdin>: WARNING: Expected \":\"\n")
expectPrinted(t, "a { !; height: 1px }", "a {\n !;\n height: 1px;\n}\n", "<stdin>: WARNING: Expected identifier but found \"!\"\n")
}
Expand Down Expand Up @@ -2251,7 +2248,8 @@ func TestParseErrorRecovery(t *testing.T) {
expectPrinted(t, "x { y: z", "x {\n y: z;\n}\n", "<stdin>: WARNING: Expected \"}\" to go with \"{\"\n<stdin>: NOTE: The unbalanced \"{\" is here:\n")
expectPrinted(t, "x { y: (", "x {\n y: ();\n}\n", "<stdin>: WARNING: Expected \")\" to go with \"(\"\n<stdin>: NOTE: The unbalanced \"(\" is here:\n")
expectPrinted(t, "x { y: [", "x {\n y: [];\n}\n", "<stdin>: WARNING: Expected \"]\" to go with \"[\"\n<stdin>: NOTE: The unbalanced \"[\" is here:\n")
expectPrinted(t, "x { y: {", "x {\n y: {};\n}\n", "<stdin>: WARNING: Expected \"}\" to go with \"{\"\n<stdin>: NOTE: The unbalanced \"{\" is here:\n")
expectPrinted(t, "x { y: {", "x {\n y: {\n }\n}\n",
"<stdin>: WARNING: Expected identifier but found whitespace\n<stdin>: WARNING: Expected \"}\" to go with \"{\"\n<stdin>: NOTE: The unbalanced \"{\" is here:\n")
expectPrinted(t, "x { y: z(", "x {\n y: z();\n}\n", "<stdin>: WARNING: Expected \")\" to go with \"(\"\n<stdin>: NOTE: The unbalanced \"(\" is here:\n")
expectPrinted(t, "x { y: z(abc", "x {\n y: z(abc);\n}\n", "<stdin>: WARNING: Expected \")\" to go with \"(\"\n<stdin>: NOTE: The unbalanced \"(\" is here:\n")
expectPrinted(t, "x { y: url(", "x {\n y: url();\n}\n",
Expand Down
26 changes: 14 additions & 12 deletions internal/css_printer/css_printer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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}}")
Expand Down

0 comments on commit e76780c

Please sign in to comment.