Skip to content

Commit

Permalink
fix #1115: add a special-cased error message
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Apr 7, 2021
1 parent 5dc64cb commit 99b2606
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 1 deletion.
36 changes: 36 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,42 @@

Starting with this release, esbuild will now allow `node_modules` directories to take precedence over `NODE_PATH` directories. This is a deviation from the published algorithm.

* Provide a better error message for incorrectly-quoted JSX attributes ([959](https://github.com/evanw/esbuild/issues/959), [#1115](https://github.com/evanw/esbuild/issues/1115))

People sometimes try to use the output of `JSON.stringify()` as a JSX attribute when automatically-generating JSX code. Doing so is incorrect because JSX strings work like XML instead of like JS (since JSX is XML-in-JS). Specifically, using a backslash before a quote does not cause it to be escaped:

```
JSX ends the "content" attribute here and sets "content" to 'some so-called \\'
v
let button = <Button content="some so-called \"button text\"" />
^
There is no "=" after the JSX attribute "text", so we expect a ">"
```
This has come up twice now so it could be worth having a dedicated error message. Previously esbuild had a generic syntax error like this:
```
> example.jsx:1:58: error: Expected ">" but found "\\"
1 │ let button = <Button content="some so-called \"button text\"" />
╵ ^
```
Now esbuild will provide more information if it detects this case:
```
> example.jsx:1:58: error: Unexpected backslash in JSX element
1 │ let button = <Button content="some so-called \"button text\"" />
╵ ^
example.jsx:1:45: note: JSX attributes use XML-style escapes instead of JavaScript-style escapes
1 │ let button = <Button content="some so-called \"button text\"" />
│ ~~
╵ &quot;
example.jsx:1:29: note: Consider using a JavaScript string inside {...} instead of a JSX attribute
1 │ let button = <Button content="some so-called \"button text\"" />
│ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
╵ {"some so-called \"button text\""}
```
## 0.11.5
* Add support for the `override` keyword in TypeScript 4.3 ([#1105](https://github.com/evanw/esbuild/pull/1105))
Expand Down
12 changes: 12 additions & 0 deletions internal/js_lexer/js_lexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ type Lexer struct {
end int
ApproximateNewlineCount int
LegacyOctalLoc logger.Loc
PreviousBackslashQuoteInJSX logger.Range
Token T
HasNewlineBefore bool
HasPureCommentBefore bool
Expand Down Expand Up @@ -890,6 +891,7 @@ func (lexer *Lexer) NextInsideJSXElement() {
}

case '\'', '"':
var backslash logger.Range
quote := lexer.codePoint
needsDecode := false
lexer.step()
Expand All @@ -904,7 +906,16 @@ func (lexer *Lexer) NextInsideJSXElement() {
needsDecode = true
lexer.step()

case '\\':
backslash = logger.Range{Loc: logger.Loc{Start: int32(lexer.end)}, Len: 1}
lexer.step()
continue

case quote:
if backslash.Len > 0 {
backslash.Len++
lexer.PreviousBackslashQuoteInJSX = backslash
}
lexer.step()
break stringLiteral

Expand All @@ -915,6 +926,7 @@ func (lexer *Lexer) NextInsideJSXElement() {
}
lexer.step()
}
backslash = logger.Range{}
}

lexer.Token = TStringLiteral
Expand Down
51 changes: 50 additions & 1 deletion internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -4036,6 +4036,7 @@ func (p *parser) parseJSXElement(loc logger.Loc) js_ast.Expr {
}

// Parse attributes
var previousStringWithBackslashLoc logger.Loc
properties := []js_ast.Property{}
if startTag != nil {
parseAttributes:
Expand All @@ -4056,7 +4057,11 @@ func (p *parser) parseJSXElement(loc logger.Loc) js_ast.Expr {
// Use NextInsideJSXElement() not Next() so we can parse a JSX-style string literal
p.lexer.NextInsideJSXElement()
if p.lexer.Token == js_lexer.TStringLiteral {
value = js_ast.Expr{Loc: p.lexer.Loc(), Data: &js_ast.EString{Value: p.lexer.StringLiteral}}
stringLoc := p.lexer.Loc()
if p.lexer.PreviousBackslashQuoteInJSX.Loc.Start > stringLoc.Start {
previousStringWithBackslashLoc = stringLoc
}
value = js_ast.Expr{Loc: stringLoc, Data: &js_ast.EString{Value: p.lexer.StringLiteral}}
p.lexer.NextInsideJSXElement()
} else {
// Use Expect() not ExpectInsideJSXElement() so we can parse expression tokens
Expand Down Expand Up @@ -4091,6 +4096,50 @@ func (p *parser) parseJSXElement(loc logger.Loc) js_ast.Expr {
}
}

// People sometimes try to use the output of "JSON.stringify()" as a JSX
// attribute when automatically-generating JSX code. Doing so is incorrect
// because JSX strings work like XML instead of like JS (since JSX is XML-in-
// JS). Specifically, using a backslash before a quote does not cause it to
// be escaped:
//
// JSX ends the "content" attribute here and sets "content" to 'some so-called \\'
// v
// <Button content="some so-called \"button text\"" />
// ^
// There is no "=" after the JSX attribute "text", so we expect a ">"
//
// This code special-cases this error to provide a less obscure error message.
if p.lexer.Token == js_lexer.TSyntaxError && p.lexer.Raw() == "\\" && previousStringWithBackslashLoc.Start > 0 {
msg := logger.Msg{Kind: logger.Error, Data: logger.RangeData(&p.source, p.lexer.Range(),
"Unexpected backslash in JSX element")}

// Option 1: Suggest using an XML escape
jsEscape := p.source.TextForRange(p.lexer.PreviousBackslashQuoteInJSX)
xmlEscape := ""
if jsEscape == "\\\"" {
xmlEscape = "&quot;"
} else if jsEscape == "\\'" {
xmlEscape = "&apos;"
}
if xmlEscape != "" {
data := logger.RangeData(&p.source, p.lexer.PreviousBackslashQuoteInJSX,
"JSX attributes use XML-style escapes instead of JavaScript-style escapes")
data.Location.Suggestion = xmlEscape
msg.Notes = append(msg.Notes, data)
}

// Option 2: Suggest using a JavaScript string
if stringRange := p.source.RangeOfString(previousStringWithBackslashLoc); stringRange.Len > 0 {
data := logger.RangeData(&p.source, stringRange,
"Consider using a JavaScript string inside {...} instead of a JSX attribute")
data.Location.Suggestion = fmt.Sprintf("{%s}", p.source.TextForRange(stringRange))
msg.Notes = append(msg.Notes, data)
}

p.log.AddMsg(msg)
panic(js_lexer.LexerPanic{})
}

// A slash here is a self-closing element
if p.lexer.Token == js_lexer.TSlash {
// Use NextInsideJSXElement() not Next() so we can parse ">>" as ">"
Expand Down
38 changes: 38 additions & 0 deletions scripts/end-to-end-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,44 @@
}),
)

// Test coverage for a special JSX error message
tests.push(
test(['example.jsx', '--outfile=node.js'], {
'example.jsx': `let button = <Button content="some so-called \\"button text\\"" />`,
}, {
expectedStderr: ` > example.jsx:1:58: error: Unexpected backslash in JSX element
1 │ let button = <Button content="some so-called \\"button text\\"" />
╵ ^
example.jsx:1:45: note: JSX attributes use XML-style escapes instead of JavaScript-style escapes
1 │ let button = <Button content="some so-called \\"button text\\"" />
│ ~~
╵ &quot;
example.jsx:1:29: note: Consider using a JavaScript string inside {...} instead of a JSX attribute
1 │ let button = <Button content="some so-called \\"button text\\"" />
│ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
╵ {"some so-called \\"button text\\""}
`,
}),
test(['example.jsx', '--outfile=node.js'], {
'example.jsx': `let button = <Button content='some so-called \\'button text\\'' />`,
}, {
expectedStderr: ` > example.jsx:1:58: error: Unexpected backslash in JSX element
1 │ let button = <Button content='some so-called \\'button text\\'' />
╵ ^
example.jsx:1:45: note: JSX attributes use XML-style escapes instead of JavaScript-style escapes
1 │ let button = <Button content='some so-called \\'button text\\'' />
│ ~~
╵ &apos;
example.jsx:1:29: note: Consider using a JavaScript string inside {...} instead of a JSX attribute
1 │ let button = <Button content='some so-called \\'button text\\'' />
│ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
╵ {'some so-called \\'button text\\''}
`,
}),
)

// Tests for symlinks
//
// Note: These are disabled on Windows because they fail when run with GitHub
Expand Down

0 comments on commit 99b2606

Please sign in to comment.