Skip to content

Commit

Permalink
fix #2610: template tag edge cases with this
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Oct 14, 2022
1 parent a4a45f3 commit 0d8ec0a
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 12 deletions.
32 changes: 32 additions & 0 deletions CHANGELOG.md
View file
@@ -1,5 +1,37 @@
# Changelog

## Unreleased

* Fix various edge cases regarding template tags and `this` ([#2610](https://github.com/evanw/esbuild/issues/2610))

This release fixes some bugs where the value of `this` wasn't correctly preserved when evaluating template tags in a few edge cases. These edge cases are listed below:

```js
async function test() {
class Foo { foo() { return this } }
class Bar extends Foo {
a = async () => super.foo``
b = async () => super['foo']``
c = async (foo) => super[foo]``
}
function foo() { return this }
const obj = { foo }
const bar = new Bar
console.log(
(await bar.a()) === bar,
(await bar.b()) === bar,
(await bar.c('foo')) === bar,
{ foo }.foo``.foo === foo,
(true && obj.foo)`` !== obj,
(false || obj.foo)`` !== obj,
(null ?? obj.foo)`` !== obj,
)
}
test()
```
Each edge case in the code above previously incorrectly printed `false` when run through esbuild with `--minify --target=es6` but now correctly prints `true`. These edge cases are unlikely to have affected real-world code.
## 0.15.10
* Add support for node's "pattern trailers" syntax ([#2569](https://github.com/evanw/esbuild/issues/2569))
Expand Down
50 changes: 38 additions & 12 deletions internal/js_parser/js_parser.go
View file
Expand Up @@ -11025,7 +11025,7 @@ func (p *parser) instantiateDefineExpr(loc logger.Loc, expr config.DefineExpr, o

// Build up a chain of property access expressions for subsequent parts
for _, part := range parts {
if expr, ok := p.maybeRewritePropertyAccess(loc, js_ast.AssignTargetNone, false, value, part, loc, false, false); ok {
if expr, ok := p.maybeRewritePropertyAccess(loc, js_ast.AssignTargetNone, false, value, part, loc, false, false, false); ok {
value = expr
} else if p.isMangledProp(part) {
value = js_ast.Expr{Loc: loc, Data: &js_ast.EIndex{
Expand Down Expand Up @@ -11237,6 +11237,7 @@ func (p *parser) maybeRewritePropertyAccess(
name string,
nameLoc logger.Loc,
isCallTarget bool,
isTemplateTag bool,
preferQuotedKey bool,
) (js_ast.Expr, bool) {
if id, ok := target.Data.(*js_ast.EIdentifier); ok {
Expand Down Expand Up @@ -11311,7 +11312,7 @@ func (p *parser) maybeRewritePropertyAccess(
}

// Attempt to simplify statically-determined object literal property accesses
if !isCallTarget && p.options.minifySyntax && assignTarget == js_ast.AssignTargetNone {
if !isCallTarget && !isTemplateTag && p.options.minifySyntax && assignTarget == js_ast.AssignTargetNone {
if object, ok := target.Data.(*js_ast.EObject); ok {
var replace js_ast.Expr
hasProtoNull := false
Expand Down Expand Up @@ -12618,8 +12619,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
break
}

isCallTarget := e == p.callTarget
isTemplateTag := e == p.templateTag
isCallTargetOrTemplateTag := e == p.callTarget || e == p.templateTag
isStmtExpr := e == p.stmtExprValue
wasAnonymousNamedExpr := p.isAnonymousNamedExpr(e.Right)
oldSilenceWarningAboutThisBeingUndefined := p.fnOnlyDataVisit.silenceMessageAboutThisBeingUndefined
Expand Down Expand Up @@ -12707,7 +12707,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
// "(1, fn)()" => "fn()"
// "(1, this.fn)" => "this.fn"
// "(1, this.fn)()" => "(0, this.fn)()"
if (isCallTarget || isTemplateTag) && hasValueForThisInCall(e.Right) {
if isCallTargetOrTemplateTag && hasValueForThisInCall(e.Right) {
return js_ast.JoinWithComma(js_ast.Expr{Loc: e.Left.Loc, Data: &js_ast.ENumber{}}, e.Right), exprOut{}
}
return e.Right, exprOut{}
Expand Down Expand Up @@ -12810,7 +12810,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
// "(null ?? fn)()" => "fn()"
// "(null ?? this.fn)" => "this.fn"
// "(null ?? this.fn)()" => "(0, this.fn)()"
if isCallTarget && hasValueForThisInCall(e.Right) {
if isCallTargetOrTemplateTag && hasValueForThisInCall(e.Right) {
return js_ast.JoinWithComma(js_ast.Expr{Loc: e.Left.Loc, Data: &js_ast.ENumber{}}, e.Right), exprOut{}
}

Expand Down Expand Up @@ -12838,7 +12838,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
// "(0 || fn)()" => "fn()"
// "(0 || this.fn)" => "this.fn"
// "(0 || this.fn)()" => "(0, this.fn)()"
if isCallTarget && hasValueForThisInCall(e.Right) {
if isCallTargetOrTemplateTag && hasValueForThisInCall(e.Right) {
return js_ast.JoinWithComma(js_ast.Expr{Loc: e.Left.Loc, Data: &js_ast.ENumber{}}, e.Right), exprOut{}
}
return e.Right, exprOut{}
Expand Down Expand Up @@ -12868,7 +12868,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
// "(1 && fn)()" => "fn()"
// "(1 && this.fn)" => "this.fn"
// "(1 && this.fn)()" => "(0, this.fn)()"
if isCallTarget && hasValueForThisInCall(e.Right) {
if isCallTargetOrTemplateTag && hasValueForThisInCall(e.Right) {
return js_ast.JoinWithComma(js_ast.Expr{Loc: e.Left.Loc, Data: &js_ast.ENumber{}}, e.Right), exprOut{}
}
return e.Right, exprOut{}
Expand Down Expand Up @@ -13187,6 +13187,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
case *js_ast.EDot:
isDeleteTarget := e == p.deleteTarget
isCallTarget := e == p.callTarget
isTemplateTag := e == p.templateTag

// Check both user-specified defines and known globals
if defines, ok := p.options.defines.DotDefines[e.Name]; ok {
Expand Down Expand Up @@ -13241,7 +13242,18 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
!isCallTarget && p.shouldLowerSuperPropertyAccess(e.Target) {
// "super.foo" => "__superGet('foo')"
key := js_ast.Expr{Loc: e.NameLoc, Data: &js_ast.EString{Value: helpers.StringToUTF16(e.Name)}}
return p.lowerSuperPropertyGet(expr.Loc, key), exprOut{}
value := p.lowerSuperPropertyGet(expr.Loc, key)
if isTemplateTag {
value.Data = &js_ast.ECall{
Target: js_ast.Expr{Loc: value.Loc, Data: &js_ast.EDot{
Target: value,
Name: "bind",
NameLoc: value.Loc,
}},
Args: []js_ast.Expr{{Loc: value.Loc, Data: js_ast.EThisShared}},
}
}
return value, exprOut{}
}

// Lower optional chaining if we're the top of the chain
Expand All @@ -13263,7 +13275,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
}
if e.OptionalChain == js_ast.OptionalChainNone {
if value, ok := p.maybeRewritePropertyAccess(expr.Loc, in.assignTarget,
isDeleteTarget, e.Target, e.Name, e.NameLoc, isCallTarget, false); ok {
isDeleteTarget, e.Target, e.Name, e.NameLoc, isCallTarget, isTemplateTag, false); ok {
return value, out
}
}
Expand Down Expand Up @@ -13308,6 +13320,9 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
if isCallTarget {
p.callTarget = dot
}
if isTemplateTag {
p.templateTag = dot
}
if isDeleteTarget {
p.deleteTarget = dot
}
Expand Down Expand Up @@ -13377,7 +13392,18 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
if e.OptionalChain == js_ast.OptionalChainNone && in.assignTarget == js_ast.AssignTargetNone &&
!isCallTarget && p.shouldLowerSuperPropertyAccess(e.Target) {
// "super[foo]" => "__superGet(foo)"
return p.lowerSuperPropertyGet(expr.Loc, e.Index), exprOut{}
value := p.lowerSuperPropertyGet(expr.Loc, e.Index)
if isTemplateTag {
value.Data = &js_ast.ECall{
Target: js_ast.Expr{Loc: value.Loc, Data: &js_ast.EDot{
Target: value,
Name: "bind",
NameLoc: value.Loc,
}},
Args: []js_ast.Expr{{Loc: value.Loc, Data: js_ast.EThisShared}},
}
}
return value, exprOut{}
}

// Lower optional chaining if we're the top of the chain
Expand All @@ -13400,7 +13426,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
if str, ok := e.Index.Data.(*js_ast.EString); ok && e.OptionalChain == js_ast.OptionalChainNone {
preferQuotedKey := !p.options.minifySyntax
if value, ok := p.maybeRewritePropertyAccess(expr.Loc, in.assignTarget, isDeleteTarget,
e.Target, helpers.UTF16ToString(str.Value), e.Index.Loc, isCallTarget, preferQuotedKey); ok {
e.Target, helpers.UTF16ToString(str.Value), e.Index.Loc, isCallTarget, isTemplateTag, preferQuotedKey); ok {
return value, out
}
}
Expand Down
10 changes: 10 additions & 0 deletions internal/js_parser/js_parser_test.go
View file
Expand Up @@ -3774,8 +3774,18 @@ func TestMangleTemplate(t *testing.T) {
expectPrintedMangle(t, "tag`a${'x'}b${y}c`", "tag`a${\"x\"}b${y}c`;\n")
expectPrintedMangle(t, "tag`a${'x'}b${'y'}c`", "tag`a${\"x\"}b${\"y\"}c`;\n")

expectPrintedMangle(t, "(1, x)``", "x``;\n")
expectPrintedMangle(t, "(1, x.y)``", "(0, x.y)``;\n")
expectPrintedMangle(t, "(1, x[y])``", "(0, x[y])``;\n")
expectPrintedMangle(t, "(true && x)``", "x``;\n")
expectPrintedMangle(t, "(true && x.y)``", "(0, x.y)``;\n")
expectPrintedMangle(t, "(true && x[y])``", "(0, x[y])``;\n")
expectPrintedMangle(t, "(false || x)``", "x``;\n")
expectPrintedMangle(t, "(false || x.y)``", "(0, x.y)``;\n")
expectPrintedMangle(t, "(false || x[y])``", "(0, x[y])``;\n")
expectPrintedMangle(t, "(null ?? x)``", "x``;\n")
expectPrintedMangle(t, "(null ?? x.y)``", "(0, x.y)``;\n")
expectPrintedMangle(t, "(null ?? x[y])``", "(0, x[y])``;\n")

expectPrintedMangleTarget(t, 2015, "class Foo { #foo() { return this.#foo`` } }", `var _foo, foo_fn;
class Foo {
Expand Down
32 changes: 32 additions & 0 deletions scripts/end-to-end-tests.js
View file
Expand Up @@ -2569,6 +2569,9 @@ for (const minify of [[], ['--minify-syntax']]) {
test(['in.js', '--outfile=node.js'].concat(minify), {
'in.js': `if ({ a: function() { return this.b }, b: 1 }${access}() !== 1) throw 'fail'`,
}),
test(['in.js', '--outfile=node.js'].concat(minify), {
'in.js': `if ({ a: function() { return this.b }, b: 1 }${access}\`\` !== 1) throw 'fail'`,
}),
test(['in.js', '--outfile=node.js'].concat(minify), {
'in.js': `if (({a: 2}${access} = 1) !== 1) throw 'fail'`,
}),
Expand Down Expand Up @@ -5194,6 +5197,35 @@ for (let flags of [[], ['--target=es2017'], ['--target=es6']]) {
}
`,
}, { async: true }),
test(['in.js', '--outfile=node.js'].concat(flags), {
'in.js': `
class Foo extends (class {
foo() {
return this
}
}) {
x = async (foo) => [
super.foo(),
super.foo\`\`,
super[foo](),
super[foo]\`\`,
super['foo'](),
super['foo']\`\`,
this.#bar(),
this.#bar\`\`,
]
#bar() {
return this
}
}
exports.async = async () => {
const foo = new Foo
for (const bar of await foo.x('foo'))
if (foo !== bar)
throw 'fail'
}
`,
}, { async: true }),
)
}

Expand Down

0 comments on commit 0d8ec0a

Please sign in to comment.