diff --git a/CHANGELOG.md b/CHANGELOG.md index 1870b45a3cf..8b8116cfaa0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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)) diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go index ee63c90477f..7f45c5b75a0 100644 --- a/internal/js_parser/js_parser.go +++ b/internal/js_parser/js_parser.go @@ -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{ @@ -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 { @@ -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 @@ -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 @@ -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{} @@ -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{} } @@ -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{} @@ -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{} @@ -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 { @@ -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 @@ -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 } } @@ -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 } @@ -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 @@ -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 } } diff --git a/internal/js_parser/js_parser_test.go b/internal/js_parser/js_parser_test.go index 06a8a92a046..0d3a94dc493 100644 --- a/internal/js_parser/js_parser_test.go +++ b/internal/js_parser/js_parser_test.go @@ -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 { diff --git a/scripts/end-to-end-tests.js b/scripts/end-to-end-tests.js index 4eb7e79e5c5..07c906a5c85 100644 --- a/scripts/end-to-end-tests.js +++ b/scripts/end-to-end-tests.js @@ -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'`, }), @@ -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 }), ) }