From 25b92bfbb7278e1ce1f9005367eccaf3cf1d9d48 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Tue, 23 Jun 2020 23:13:54 -0700 Subject: [PATCH] fix #195: bugs with jsx and whitespace --- CHANGELOG.md | 8 ++++++++ internal/bundler/bundler_test.go | 2 +- internal/lexer/lexer.go | 29 ++++++++++++----------------- internal/parser/parser_test.go | 29 +++++++++++++++++++++++++++++ 4 files changed, 50 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba572bd525b..d34e2f9e5ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 ` ` 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 diff --git a/internal/bundler/bundler_test.go b/internal/bundler/bundler_test.go index e27683009a1..972e0491fc9 100644 --- a/internal/bundler/bundler_test.go +++ b/internal/bundler/bundler_test.go @@ -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")); `, }, diff --git a/internal/lexer/lexer.go b/internal/lexer/lexer.go index 7176b6eeaff..3722e681487 100644 --- a/internal/lexer/lexer.go +++ b/internal/lexer/lexer.go @@ -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 @@ -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 @@ -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 @@ -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 { @@ -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) diff --git a/internal/parser/parser_test.go b/internal/parser/parser_test.go index ce0a6c2fed0..c079e042b8d 100644 --- a/internal/parser/parser_test.go +++ b/internal/parser/parser_test.go @@ -1504,6 +1504,35 @@ func TestJSX(t *testing.T) { expectPrintedJSX(t, " \nšŸ™‚", "React.createElement(\"a\", null, \"šŸ™‚\");\n") expectPrintedJSX(t, "\n šŸ™‚", "React.createElement(\"a\", null, \"šŸ™‚\");\n") + // "{x}" with all combinations of "", " ", and "\n" inserted in between + expectPrintedJSX(t, "{x};", "React.createElement(\"a\", null, x, React.createElement(\"b\", null));\n") + expectPrintedJSX(t, "\n{x};", "React.createElement(\"a\", null, x, React.createElement(\"b\", null));\n") + expectPrintedJSX(t, "{x}\n;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null));\n") + expectPrintedJSX(t, "\n{x}\n;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null));\n") + expectPrintedJSX(t, "{x}\n;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null));\n") + expectPrintedJSX(t, "\n{x}\n;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null));\n") + expectPrintedJSX(t, "{x}\n\n;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null));\n") + expectPrintedJSX(t, "\n{x}\n\n;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null));\n") + expectPrintedJSX(t, " {x};", "React.createElement(\"a\", null, \" \", x, React.createElement(\"b\", null));\n") + expectPrintedJSX(t, " {x}\n;", "React.createElement(\"a\", null, \" \", x, React.createElement(\"b\", null));\n") + expectPrintedJSX(t, " {x}\n;", "React.createElement(\"a\", null, \" \", x, React.createElement(\"b\", null));\n") + expectPrintedJSX(t, " {x}\n\n;", "React.createElement(\"a\", null, \" \", x, React.createElement(\"b\", null));\n") + expectPrintedJSX(t, "{x} ;", "React.createElement(\"a\", null, x, \" \", React.createElement(\"b\", null));\n") + expectPrintedJSX(t, "\n{x} ;", "React.createElement(\"a\", null, x, \" \", React.createElement(\"b\", null));\n") + expectPrintedJSX(t, "{x} \n;", "React.createElement(\"a\", null, x, \" \", React.createElement(\"b\", null));\n") + expectPrintedJSX(t, "\n{x} \n;", "React.createElement(\"a\", null, x, \" \", React.createElement(\"b\", null));\n") + expectPrintedJSX(t, " {x} ;", "React.createElement(\"a\", null, \" \", x, \" \", React.createElement(\"b\", null));\n") + expectPrintedJSX(t, " {x} \n;", "React.createElement(\"a\", null, \" \", x, \" \", React.createElement(\"b\", null));\n") + expectPrintedJSX(t, "{x} ;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null), \" \");\n") + expectPrintedJSX(t, "\n{x} ;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null), \" \");\n") + expectPrintedJSX(t, "{x}\n ;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null), \" \");\n") + expectPrintedJSX(t, "\n{x}\n ;", "React.createElement(\"a\", null, x, React.createElement(\"b\", null), \" \");\n") + expectPrintedJSX(t, " {x} ;", "React.createElement(\"a\", null, \" \", x, React.createElement(\"b\", null), \" \");\n") + expectPrintedJSX(t, " {x}\n ;", "React.createElement(\"a\", null, \" \", x, React.createElement(\"b\", null), \" \");\n") + expectPrintedJSX(t, "{x} ;", "React.createElement(\"a\", null, x, \" \", React.createElement(\"b\", null), \" \");\n") + expectPrintedJSX(t, "\n{x} ;", "React.createElement(\"a\", null, x, \" \", React.createElement(\"b\", null), \" \");\n") + expectPrintedJSX(t, " {x} ;", "React.createElement(\"a\", null, \" \", x, \" \", React.createElement(\"b\", null), \" \");\n") + expectParseErrorJSX(t, "", ": error: Expected \"{\" but found \"true\"\n") expectParseErrorJSX(t, "", ": error: Expected identifier but found \"/\"\n") expectParseErrorJSX(t, "", ": error: Expected closing tag \"b\" to match opening tag \"a\"\n")