Skip to content

Commit

Permalink
fix #195: bugs with jsx and whitespace
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Jun 24, 2020
1 parent 1d42a03 commit 25b92bf
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 18 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Changelog

## Unreleased

* Fix a JSX whitespace bug ([#195](https://github.com/evanw/esbuild/issues/195))

Whitespace behavior in JSX has unfortunately been [left out of the JSX specification](https://github.com/facebook/jsx/issues/6), so it's up to each implementation to determine how to handle whitespace characters. Most of the JSX parsers in the ecosystem have converged on similar behavior. When they differ, esbuild follows the behavior of the TypeScript JSX parser.

This release fixes a bug where esbuild's JSX parser behaved differently than TypeScript. Certain whitespace characters between JSX elements were incorrectly removed. For example, the space in `<a><b/> <c/></a>` must be preserved to match the TypeScript JSX parser. These cases now have test coverage.

## 0.5.11

* Fix a JavaScript API crash on node 10.x
Expand Down
2 changes: 1 addition & 1 deletion internal/bundler/bundler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4665,7 +4665,7 @@ const p = "p";
const Internal = () => h(p, null, " Test 2 ");
// /app.jsx
const App = () => h(p, null, h(Internal, null), " T ");
const App = () => h(p, null, " ", h(Internal, null), " T ");
render(h(App, null), document.getElementById("app"));
`,
},
Expand Down
29 changes: 12 additions & 17 deletions internal/lexer/lexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -541,13 +541,17 @@ func IsIdentifierContinue(codePoint rune) bool {
return unicode.Is(idContinue, codePoint)
}

// See the "White Space Code Points" table in the ECMAScript standard
func IsWhitespace(codePoint rune) bool {
switch codePoint {
case
'\u0009', // character tabulation
'\u000B', // line tabulation
'\u000C', // form feed
'\u0020', // space
'\u00A0', // no-break space

// Unicode "Space_Separator" code points
'\u1680', // ogham space mark
'\u2000', // en quad
'\u2001', // em quad
Expand All @@ -563,6 +567,7 @@ func IsWhitespace(codePoint rune) bool {
'\u202F', // narrow no-break space
'\u205F', // medium mathematical space
'\u3000', // ideographic space

'\uFEFF': // zero width non-breaking space
return true

Expand Down Expand Up @@ -614,15 +619,6 @@ func (lexer *Lexer) NextJSXElementChild() {
case -1: // This indicates the end of the file
lexer.Token = TEndOfFile

case '\r', '\n', '\u2028', '\u2029':
lexer.step()
lexer.HasNewlineBefore = true
continue

case '\t', ' ':
lexer.step()
continue

case '{':
lexer.step()
lexer.Token = TOpenBrace
Expand All @@ -632,14 +628,7 @@ func (lexer *Lexer) NextJSXElementChild() {
lexer.Token = TLessThan

default:
// Check for unusual whitespace characters
if IsWhitespace(lexer.codePoint) {
lexer.step()
continue
}

// This needs fixing if we skipped over whitespace characters earlier
needsFixing := lexer.start != originalStart
needsFixing := false

stringLiteral:
for {
Expand Down Expand Up @@ -672,6 +661,12 @@ func (lexer *Lexer) NextJSXElementChild() {
if needsFixing {
// Slow path
lexer.StringLiteral = fixWhitespaceAndDecodeJSXEntities(text)

// Skip this token if it turned out to be empty after trimming
if len(lexer.StringLiteral) == 0 {
lexer.HasNewlineBefore = true
continue
}
} else {
// Fast path
n := len(text)
Expand Down
29 changes: 29 additions & 0 deletions internal/parser/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1504,6 +1504,35 @@ func TestJSX(t *testing.T) {
expectPrintedJSX(t, "<a> \nπŸ™‚</a>", "React.createElement(\"a\", null, \"πŸ™‚\");\n")
expectPrintedJSX(t, "<a>\n πŸ™‚</a>", "React.createElement(\"a\", null, \"πŸ™‚\");\n")

// "<a>{x}</b></a>" with all combinations of "", " ", and "\n" inserted in between
expectPrintedJSX(t, "<a>{x}<b/></a>;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null));\n")
expectPrintedJSX(t, "<a>\n{x}<b/></a>;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null));\n")
expectPrintedJSX(t, "<a>{x}\n<b/></a>;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null));\n")
expectPrintedJSX(t, "<a>\n{x}\n<b/></a>;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null));\n")
expectPrintedJSX(t, "<a>{x}<b/>\n</a>;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null));\n")
expectPrintedJSX(t, "<a>\n{x}<b/>\n</a>;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null));\n")
expectPrintedJSX(t, "<a>{x}\n<b/>\n</a>;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null));\n")
expectPrintedJSX(t, "<a>\n{x}\n<b/>\n</a>;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null));\n")
expectPrintedJSX(t, "<a> {x}<b/></a>;", "React.createElement(\"a\", null, \" \", x, React.createElement(\"b\", null));\n")
expectPrintedJSX(t, "<a> {x}\n<b/></a>;", "React.createElement(\"a\", null, \" \", x, React.createElement(\"b\", null));\n")
expectPrintedJSX(t, "<a> {x}<b/>\n</a>;", "React.createElement(\"a\", null, \" \", x, React.createElement(\"b\", null));\n")
expectPrintedJSX(t, "<a> {x}\n<b/>\n</a>;", "React.createElement(\"a\", null, \" \", x, React.createElement(\"b\", null));\n")
expectPrintedJSX(t, "<a>{x} <b/></a>;", "React.createElement(\"a\", null, x, \" \", React.createElement(\"b\", null));\n")
expectPrintedJSX(t, "<a>\n{x} <b/></a>;", "React.createElement(\"a\", null, x, \" \", React.createElement(\"b\", null));\n")
expectPrintedJSX(t, "<a>{x} <b/>\n</a>;", "React.createElement(\"a\", null, x, \" \", React.createElement(\"b\", null));\n")
expectPrintedJSX(t, "<a>\n{x} <b/>\n</a>;", "React.createElement(\"a\", null, x, \" \", React.createElement(\"b\", null));\n")
expectPrintedJSX(t, "<a> {x} <b/></a>;", "React.createElement(\"a\", null, \" \", x, \" \", React.createElement(\"b\", null));\n")
expectPrintedJSX(t, "<a> {x} <b/>\n</a>;", "React.createElement(\"a\", null, \" \", x, \" \", React.createElement(\"b\", null));\n")
expectPrintedJSX(t, "<a>{x}<b/> </a>;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null), \" \");\n")
expectPrintedJSX(t, "<a>\n{x}<b/> </a>;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null), \" \");\n")
expectPrintedJSX(t, "<a>{x}\n<b/> </a>;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null), \" \");\n")
expectPrintedJSX(t, "<a>\n{x}\n<b/> </a>;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null), \" \");\n")
expectPrintedJSX(t, "<a> {x}<b/> </a>;", "React.createElement(\"a\", null, \" \", x, React.createElement(\"b\", null), \" \");\n")
expectPrintedJSX(t, "<a> {x}\n<b/> </a>;", "React.createElement(\"a\", null, \" \", x, React.createElement(\"b\", null), \" \");\n")
expectPrintedJSX(t, "<a>{x} <b/> </a>;", "React.createElement(\"a\", null, x, \" \", React.createElement(\"b\", null), \" \");\n")
expectPrintedJSX(t, "<a>\n{x} <b/> </a>;", "React.createElement(\"a\", null, x, \" \", React.createElement(\"b\", null), \" \");\n")
expectPrintedJSX(t, "<a> {x} <b/> </a>;", "React.createElement(\"a\", null, \" \", x, \" \", React.createElement(\"b\", null), \" \");\n")

expectParseErrorJSX(t, "<a b=true/>", "<stdin>: error: Expected \"{\" but found \"true\"\n")
expectParseErrorJSX(t, "</a>", "<stdin>: error: Expected identifier but found \"/\"\n")
expectParseErrorJSX(t, "<a></b>", "<stdin>: error: Expected closing tag \"b\" to match opening tag \"a\"\n")
Expand Down

0 comments on commit 25b92bf

Please sign in to comment.