Skip to content

Commit

Permalink
fix #3605: print the original JSX AST unmodified
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Jan 23, 2024
1 parent f571399 commit e04a690
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 186 deletions.
24 changes: 24 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,30 @@

## Unreleased

* The "preserve" JSX mode now preserves JSX text verbatim ([#3605](https://github.com/evanw/esbuild/issues/3605))

The [JSX specification](https://facebook.github.io/jsx/) deliberately doesn't specify how JSX text is supposed to be interpreted and there is no canonical way to interpret JSX text. Two most popular interpretations are Babel and TypeScript. Yes [they are different](https://twitter.com/jarredsumner/status/1456118847937781764) (esbuild [deliberately follows TypeScript](https://twitter.com/evanwallace/status/1456122279453208576) by the way). Previously esbuild normalized text to the TypeScript interpretation when the "preserve" JSX mode is active. However, "preserve" should arguably reproduce the original JSX text verbatim so that whatever JSX transform runs after esbuild is free to interpret it however it wants. So with this release, esbuild will now pass JSX text through unmodified:

```jsx
// Original code
let el =
<a href={'/'} title='&apos;&quot;'> some text
{foo}
more text </a>

// Old output (with --loader=jsx --jsx=preserve)
let el = <a href="/" title={`'"`}>
{" some text"}
{foo}
{"more text "}
</a>;

// New output (with --loader=jsx --jsx=preserve)
let el = <a href={"/"} title='&apos;&quot;'> some text
{foo}
more text </a>;
```

* Allow JSX elements as JSX attribute values

JSX has an obscure feature where you can use JSX elements in attribute position without surrounding them with `{...}`. It looks like this:
Expand Down
60 changes: 18 additions & 42 deletions internal/bundler_tests/snapshots/snapshots_default.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -711,48 +711,24 @@ console.log(
x
}</>,
// Comments on absent AST nodes
<div>
{"before"}
{}
{"after"}
</div>,
<div>
{"before"}
{
/* comment 1 */
/* comment 2 */
}
{"after"}
</div>,
<div>
{"before"}
{
// comment 1
// comment 2
}
{"after"}
</div>,
<>
{"before"}
{}
{"after"}
</>,
<>
{"before"}
{
/* comment 1 */
/* comment 2 */
}
{"after"}
</>,
<>
{"before"}
{
// comment 1
// comment 2
}
{"after"}
</>
<div>before{}after</div>,
<div>before{
/* comment 1 */
/* comment 2 */
}after</div>,
<div>before{
// comment 1
// comment 2
}after</div>,
<>before{}after</>,
<>before{
/* comment 1 */
/* comment 2 */
}after</>,
<>before{
// comment 1
// comment 2
}after</>
);

================================================================================
Expand Down
11 changes: 11 additions & 0 deletions internal/js_ast/js_ast.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,7 @@ func (*EImportIdentifier) isExpr() {}
func (*EPrivateIdentifier) isExpr() {}
func (*ENameOfSymbol) isExpr() {}
func (*EJSXElement) isExpr() {}
func (*EJSXText) isExpr() {}
func (*EMissing) isExpr() {}
func (*ENumber) isExpr() {}
func (*EBigInt) isExpr() {}
Expand Down Expand Up @@ -770,6 +771,16 @@ type EJSXElement struct {
IsTagSingleLine bool
}

// The JSX specification doesn't say how JSX text is supposed to be interpreted
// so our "preserve" JSX transform should reproduce the original source code
// verbatim. One reason why this matters is because there is no canonical way
// to interpret JSX text (Babel and TypeScript differ in what newlines mean).
// Another reason is that some people want to do custom things such as this:
// https://github.com/evanw/esbuild/issues/3605
type EJSXText struct {
Raw string
}

type ENumber struct{ Value float64 }

type EBigInt struct{ Value string }
Expand Down
19 changes: 14 additions & 5 deletions internal/js_parser/js_parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5043,12 +5043,17 @@ func (p *parser) parseJSXElement(loc logger.Loc) js_ast.Expr {
if p.lexer.PreviousBackslashQuoteInJSX.Loc.Start > stringLoc.Start {
previousStringWithBackslashLoc = stringLoc
}
value = js_ast.Expr{Loc: stringLoc, Data: &js_ast.EString{Value: p.lexer.StringLiteral()}}
if p.options.jsx.Preserve {
value = js_ast.Expr{Loc: stringLoc, Data: &js_ast.EJSXText{Raw: p.lexer.Raw()}}
} else {
value = js_ast.Expr{Loc: stringLoc, Data: &js_ast.EString{Value: p.lexer.StringLiteral()}}
}
p.lexer.NextInsideJSXElement()
} else if p.lexer.Token == js_lexer.TLessThan {
// This may be removed in the future: https://github.com/facebook/jsx/issues/53
loc := p.lexer.Loc()
p.lexer.NextInsideJSXElement()
flags |= js_ast.PropertyWasShorthand
value = p.parseJSXElement(loc)

// The call to parseJSXElement() above doesn't consume the last
Expand Down Expand Up @@ -5199,7 +5204,11 @@ func (p *parser) parseJSXElement(loc logger.Loc) js_ast.Expr {
for {
switch p.lexer.Token {
case js_lexer.TStringLiteral:
nullableChildren = append(nullableChildren, js_ast.Expr{Loc: p.lexer.Loc(), Data: &js_ast.EString{Value: p.lexer.StringLiteral()}})
if p.options.jsx.Preserve {
nullableChildren = append(nullableChildren, js_ast.Expr{Loc: p.lexer.Loc(), Data: &js_ast.EJSXText{Raw: p.lexer.Raw()}})
} else {
nullableChildren = append(nullableChildren, js_ast.Expr{Loc: p.lexer.Loc(), Data: &js_ast.EString{Value: p.lexer.StringLiteral()}})
}
p.lexer.NextJSXElementChild()

case js_lexer.TOpenBrace:
Expand All @@ -5209,7 +5218,7 @@ func (p *parser) parseJSXElement(loc logger.Loc) js_ast.Expr {
// The expression is optional, and may be absent
if p.lexer.Token == js_lexer.TCloseBrace {
// Save comments even for absent expressions
nullableChildren = append(nullableChildren, js_ast.Expr{Loc: p.saveExprCommentsHere()})
nullableChildren = append(nullableChildren, js_ast.Expr{Loc: p.saveExprCommentsHere(), Data: nil})
} else {
if p.lexer.Token == js_lexer.TDotDotDot {
// TypeScript preserves "..." before JSX child expressions here.
Expand Down Expand Up @@ -12689,7 +12698,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
// it doesn't affect these mitigations by ensuring that the mitigations are not
// applied in those cases (e.g. by adding an additional conditional check).
switch e := expr.Data.(type) {
case *js_ast.ENull, *js_ast.ESuper, *js_ast.EBoolean, *js_ast.EBigInt, *js_ast.EUndefined:
case *js_ast.ENull, *js_ast.ESuper, *js_ast.EBoolean, *js_ast.EBigInt, *js_ast.EUndefined, *js_ast.EJSXText:

case *js_ast.ENameOfSymbol:
e.Ref = p.symbolForMangledProp(p.loadNameFromRef(e.Ref))
Expand Down Expand Up @@ -13097,7 +13106,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
propName := helpers.UTF16ToString(str.Value)
switch propName {
case "key":
if property.Flags.Has(js_ast.PropertyWasShorthand) {
if boolean, ok := property.ValueOrNil.Data.(*js_ast.EBoolean); ok && boolean.Value && property.Flags.Has(js_ast.PropertyWasShorthand) {
r := js_lexer.RangeOfIdentifier(p.source, property.Loc)
msg := logger.Msg{
Kind: logger.Error,
Expand Down
130 changes: 18 additions & 112 deletions internal/js_printer/js_printer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,80 +246,6 @@ func (p *printer) printUnquotedUTF16(text []uint16, quote rune, flags printQuote
p.js = js
}

// Use JS strings for JSX attributes that need escape characters. Technically
// the JSX specification doesn't say anything about using XML character escape
// sequences, so JSX implementations may not be able to consume them. See
// https://facebook.github.io/jsx/ for the specification.
func (p *printer) canPrintTextAsJSXAttribute(text []uint16) (quote string, ok bool) {
single := true
double := true

for _, c := range text {
// Use JS strings for control characters
if c < firstASCII {
return "", false
}

// Use JS strings if we need to escape non-ASCII characters
if p.options.ASCIIOnly && c > lastASCII {
return "", false
}

switch c {
case '&':
// Use JS strings if the text would need to be escaped with "&amp;"
return "", false

case '"':
double = false
if !single {
break
}

case '\'':
single = false
if !double {
break
}
}
}

// Prefer duble quotes to single quotes
if double {
return "\"", true
}
if single {
return "'", true
}
return "", false
}

// Use JS strings for text inside JSX elements that need escape characters.
// Technically the JSX specification doesn't say anything about using XML
// character escape sequences, so JSX implementations may not be able to
// consume them. See https://facebook.github.io/jsx/ for the specification.
func (p *printer) canPrintTextAsJSXChild(text []uint16) bool {
for _, c := range text {
// Use JS strings for control characters
if c < firstASCII {
return false
}

// Use JS strings if we need to escape non-ASCII characters
if p.options.ASCIIOnly && c > lastASCII {
return false
}

switch c {
case '&', '<', '>', '{', '}':
// Use JS strings if the text would need to be escaped
return false
}
}

return true
}

// JSX tag syntax doesn't support character escapes so non-ASCII identifiers
// must be printed as UTF-8 even when the charset is set to ASCII.
func (p *printer) printJSXTag(tagOrNil js_ast.Expr) {
Expand Down Expand Up @@ -2143,26 +2069,28 @@ func (p *printer) printExpr(expr js_ast.Expr, level js_ast.L, flags printExprFla

isMultiLine := p.willPrintExprCommentsAtLoc(property.ValueOrNil.Loc)

// Don't use shorthand syntax if it would discard comments
if !isMultiLine {
// Special-case string values
if str, ok := property.ValueOrNil.Data.(*js_ast.EString); ok {
if quote, ok := p.canPrintTextAsJSXAttribute(str.Value); ok {
p.print("=")
p.addSourceMapping(property.ValueOrNil.Loc)
p.print(quote)
p.print(helpers.UTF16ToString(str.Value))
p.print(quote)
continue
}
if property.Flags.Has(js_ast.PropertyWasShorthand) {
// Implicit "true" value
if boolean, ok := property.ValueOrNil.Data.(*js_ast.EBoolean); ok && boolean.Value {
continue
}

// Implicit "true" value
if boolean, ok := property.ValueOrNil.Data.(*js_ast.EBoolean); ok && boolean.Value && property.Flags.Has(js_ast.PropertyWasShorthand) {
// JSX element as JSX attribute value
if _, ok := property.ValueOrNil.Data.(*js_ast.EJSXElement); ok {
p.print("=")
p.printExpr(property.ValueOrNil, js_ast.LLowest, 0)
continue
}
}

// Special-case raw text
if text, ok := property.ValueOrNil.Data.(*js_ast.EJSXText); ok {
p.print("=")
p.addSourceMapping(property.ValueOrNil.Loc)
p.print(text.Raw)
continue
}

// Generic JS value
p.print("={")
if isMultiLine {
Expand Down Expand Up @@ -2197,30 +2125,13 @@ func (p *printer) printExpr(expr js_ast.Expr, level js_ast.L, flags printExprFla
}
p.print(">")

isSingleLine := true
if !p.options.MinifyWhitespace {
isSingleLine = len(e.NullableChildren) < 2
if len(e.NullableChildren) == 1 {
if _, ok := e.NullableChildren[0].Data.(*js_ast.EJSXElement); !ok {
isSingleLine = true
}
}
}
if !isSingleLine {
p.options.Indent++
}

// Print the children
for _, childOrNil := range e.NullableChildren {
if !isSingleLine {
p.printNewline()
p.printIndent()
}
if _, ok := childOrNil.Data.(*js_ast.EJSXElement); ok {
p.printExpr(childOrNil, js_ast.LLowest, 0)
} else if str, ok := childOrNil.Data.(*js_ast.EString); ok && isSingleLine && p.canPrintTextAsJSXChild(str.Value) {
} else if text, ok := childOrNil.Data.(*js_ast.EJSXText); ok {
p.addSourceMapping(childOrNil.Loc)
p.print(helpers.UTF16ToString(str.Value))
p.print(text.Raw)
} else if childOrNil.Data != nil {
isMultiLine := p.willPrintExprCommentsAtLoc(childOrNil.Loc)
p.print("{")
Expand Down Expand Up @@ -2251,11 +2162,6 @@ func (p *printer) printExpr(expr js_ast.Expr, level js_ast.L, flags printExprFla
}

// Print the closing tag
if !isSingleLine {
p.options.Indent--
p.printNewline()
p.printIndent()
}
p.addSourceMapping(e.CloseLoc)
p.print("</")
p.printJSXTag(e.TagOrNil)
Expand Down
Loading

0 comments on commit e04a690

Please sign in to comment.