Skip to content

Commit

Permalink
fix #1965: remove css "//" comment error recovery
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Jan 27, 2022
1 parent 34899aa commit 8f0560d
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 14 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
# Changelog

## Unreleased

* Remove error recovery for invalid `//` comments in CSS ([#1965](https://github.com/evanw/esbuild/issues/1965))

Previously esbuild treated `//` as a comment in CSS and generated a warning, even though comments in CSS use `/* ... */` instead. This allowed you to run esbuild on CSS intended for certain CSS preprocessors that support single-line comments.

However, some people are changing from another build tool to esbuild and have a code base that relies on `//` being preserved even though it's nonsense CSS and causes the entire surrounding rule to be discarded by the browser. Presumably this nonsense CSS ended up there at some point due to an incorrectly-configured build pipeline and the site now relies on that entire rule being discarded. If esbuild interprets `//` as a comment, it could cause the rule to no longer be discarded or even cause something else to happen.

With this release, esbuild no longer treats `//` as a comment in CSS. It still warns about it but now passes it through unmodified. This means it's no longer possible to run esbuild on CSS code containing single-line comments but it means that esbuild's behavior regarding these nonsensical CSS rules more accurately represents what happens in a browser.

## 0.14.14

* Fix bug with filename hashes and the `file` loader ([#1957](https://github.com/evanw/esbuild/issues/1957))
Expand Down
35 changes: 23 additions & 12 deletions internal/css_lexer/css_lexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,21 @@ func (t T) IsNumeric() bool {
return t == TNumber || t == TPercentage || t == TDimension
}

type TokenFlags uint8

const (
IsID TokenFlags = 1 << iota
DidWarnAboutSingleLineComment
)

// This token struct is designed to be memory-efficient. It just references a
// range in the input file instead of directly containing the substring of text
// since a range takes up less memory than a string.
type Token struct {
Range logger.Range // 8 bytes
UnitOffset uint16 // 2 bytes
Kind T // 1 byte
IsID bool // 1 byte
Flags TokenFlags // 1 byte
}

func (token Token) DecodedText(contents string) string {
Expand Down Expand Up @@ -159,6 +166,7 @@ type lexer struct {
tracker logger.LineColumnTracker
approximateNewlineCount int
current int
oldSingleLineCommentEnd logger.Loc
codePoint rune
Token Token
}
Expand Down Expand Up @@ -264,9 +272,19 @@ func (lexer *lexer) next() {
lexer.consumeToEndOfMultiLineComment(lexer.Token.Range)
continue
case '/':
lexer.step()
lexer.consumeToEndOfSingleLineComment()
continue
// Warn when people use "//" comments, which are invalid in CSS
loc := lexer.Token.Range.Loc
if loc.Start >= lexer.oldSingleLineCommentEnd.Start {
contents := lexer.source.Contents
end := lexer.current
for end < len(contents) && !isNewline(rune(contents[end])) {
end++
}
lexer.log.Add(logger.Warning, &lexer.tracker, logger.Range{Loc: loc, Len: 2},
"Comments in CSS use \"/* ... */\" instead of \"//\"")
lexer.oldSingleLineCommentEnd.Start = int32(end)
lexer.Token.Flags |= DidWarnAboutSingleLineComment
}
}
lexer.Token.Kind = TDelimSlash

Expand Down Expand Up @@ -294,7 +312,7 @@ func (lexer *lexer) next() {
if IsNameContinue(lexer.codePoint) || lexer.isValidEscape() {
lexer.Token.Kind = THash
if lexer.wouldStartIdentifier() {
lexer.Token.IsID = true
lexer.Token.Flags |= IsID
}
lexer.consumeName()
} else {
Expand Down Expand Up @@ -514,13 +532,6 @@ func containsAtPreserveOrAtLicense(text string) bool {
return false
}

func (lexer *lexer) consumeToEndOfSingleLineComment() {
for !isNewline(lexer.codePoint) && lexer.codePoint != eof {
lexer.step()
}
lexer.log.Add(logger.Warning, &lexer.tracker, lexer.Token.Range, "Comments in CSS use \"/* ... */\" instead of \"//\"")
}

func (lexer *lexer) isValidEscape() bool {
if lexer.codePoint != '\\' {
return false
Expand Down
5 changes: 4 additions & 1 deletion internal/css_parser/css_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ func (p *parser) expect(kind css_lexer.T) bool {
return true
}
t := p.current()
if (t.Flags & css_lexer.DidWarnAboutSingleLineComment) != 0 {
return false
}

var text string
var suggestion string
Expand Down Expand Up @@ -152,7 +155,7 @@ func (p *parser) expect(kind css_lexer.T) bool {
}

func (p *parser) unexpected() {
if t := p.current(); t.Range.Loc.Start > p.prevError.Start {
if t := p.current(); t.Range.Loc.Start > p.prevError.Start && (t.Flags&css_lexer.DidWarnAboutSingleLineComment) == 0 {
var text string
switch t.Kind {
case css_lexer.TEndOfFile, css_lexer.TWhitespace:
Expand Down
2 changes: 1 addition & 1 deletion internal/css_parser/css_parser_selector.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ subclassSelectors:
for {
switch p.current().Kind {
case css_lexer.THash:
if !p.current().IsID {
if (p.current().Flags & css_lexer.IsID) == 0 {
break subclassSelectors
}
name := p.decoded()
Expand Down
11 changes: 11 additions & 0 deletions internal/css_parser/css_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,17 @@ func expectPrintedMangleMinify(t *testing.T, contents string, expected string) {
})
}

func TestSingleLineComment(t *testing.T) {
expectParseError(t, "a, // a\nb // b\n{}",
"<stdin>: WARNING: Comments in CSS use \"/* ... */\" instead of \"//\"\n"+
"<stdin>: WARNING: Comments in CSS use \"/* ... */\" instead of \"//\"\n")
expectParseError(t, "a, ///// a /////\n{}",
"<stdin>: WARNING: Comments in CSS use \"/* ... */\" instead of \"//\"\n")

expectPrinted(t, "a, // a\nb // b\n{}", "a, // a b // b {\n}\n")
expectPrinted(t, "a, ///// a /////\n{}", "a, ///// a ///// {\n}\n")
}

func TestEscapes(t *testing.T) {
// TIdent
expectPrinted(t, "a { value: id\\65nt }", "a {\n value: ident;\n}\n")
Expand Down

0 comments on commit 8f0560d

Please sign in to comment.