Skip to content

Commit

Permalink
implement recent v8 changes to optional chaining behavior (#1022)
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw authored Mar 20, 2021
1 parent e5127cb commit d012ce1
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 5 deletions.
27 changes: 27 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,33 @@

This approach is experimental and is currently only enabled if the `ESBUILD_WORKER_THREADS` environment variable is present. If this use case matters to you, please try it out and let me know if you find any problems with it.

* Update how optional chains are compiled to match new V8 versions ([#1019](https://github.com/evanw/esbuild/issues/1019))

An optional chain is an expression that uses the `?.` operator, which roughly avoids evaluation of the right-hand side if the left-hand side is `null` or `undefined`. So `a?.b` is basically equivalent to `a == null ? void 0 : a.b`. When the language target is set to `es2019` or below, esbuild will transform optional chain expressions into equivalent expressions that do not use the `?.` operator.

This transform is designed to match the behavior of V8 exactly, and is designed to do something similar to the equivalent transform done by the TypeScript compiler. However, V8 has recently changed its behavior in two cases:

* Forced call of an optional member expression should propagate the object to the method:

```js
const o = { m() { return this; } };
assert((o?.m)() === o);
```

V8 bug: https://bugs.chromium.org/p/v8/issues/detail?id=10024

* Optional call of `eval` must be an indirect eval:

```js
globalThis.a = 'global';
var b = (a => eval?.('a'))('local');
assert(b === 'global');
```

V8 bug: https://bugs.chromium.org/p/v8/issues/detail?id=10630

This release changes esbuild's transform to match V8's new behavior. The transform in the TypeScript compiler is still emulating the old behavior as of version 4.2.3, so these syntax forms should be avoided in TypeScript code for portability.

## 0.9.5

* Fix parsing of the `[dir]` placeholder ([#1013](https://github.com/evanw/esbuild/issues/1013))
Expand Down
18 changes: 16 additions & 2 deletions internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -11214,14 +11214,23 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
}
}

_, wasIdentifierBeforeVisit := e.Target.Data.(*js_ast.EIdentifier)
wasIdentifierBeforeVisit := false
isParenthesizedOptionalChain := false
switch e2 := e.Target.Data.(type) {
case *js_ast.EIdentifier:
wasIdentifierBeforeVisit = true
case *js_ast.EDot:
isParenthesizedOptionalChain = e.OptionalChain == js_ast.OptionalChainNone && e2.OptionalChain != js_ast.OptionalChainNone
case *js_ast.EIndex:
isParenthesizedOptionalChain = e.OptionalChain == js_ast.OptionalChainNone && e2.OptionalChain != js_ast.OptionalChainNone
}
target, out := p.visitExprInOut(e.Target, exprIn{
hasChainParent: e.OptionalChain == js_ast.OptionalChainContinue,

// Signal to our child if this is an ECall at the start of an optional
// chain. If so, the child will need to stash the "this" context for us
// that we need for the ".call(this, ...args)".
storeThisArgForParentOptionalChain: e.OptionalChain == js_ast.OptionalChainStart,
storeThisArgForParentOptionalChain: e.OptionalChain == js_ast.OptionalChainStart || isParenthesizedOptionalChain,
})
e.Target = target
p.warnAboutImportNamespaceCallOrConstruct(e.Target, false /* isConstruct */)
Expand Down Expand Up @@ -11322,6 +11331,11 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
}
}

// Handle parenthesized optional chains
if isParenthesizedOptionalChain && out.thisArgFunc != nil && out.thisArgWrapFunc != nil {
return p.lowerParenthesizedOptionalChain(expr.Loc, e, out), exprOut{}
}

// Lower optional chaining if we're the top of the chain
containsOptionalChain := e.OptionalChain != js_ast.OptionalChainNone
if containsOptionalChain && !in.hasChainParent {
Expand Down
12 changes: 11 additions & 1 deletion internal/js_parser/js_parser_lower.go
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,6 @@ flatten:
result = js_ast.Expr{Loc: loc, Data: &js_ast.ECall{
Target: result,
Args: e.Args,
IsDirectEval: e.IsDirectEval,
CanBeUnwrappedIfUnused: e.CanBeUnwrappedIfUnused,
}}

Expand Down Expand Up @@ -742,6 +741,17 @@ flatten:
}
}

func (p *parser) lowerParenthesizedOptionalChain(loc logger.Loc, e *js_ast.ECall, childOut exprOut) js_ast.Expr {
return childOut.thisArgWrapFunc(js_ast.Expr{Loc: loc, Data: &js_ast.ECall{
Target: js_ast.Expr{Loc: loc, Data: &js_ast.EDot{
Target: e.Target,
Name: "call",
NameLoc: loc,
}},
Args: append(append(make([]js_ast.Expr, 0, len(e.Args)+1), childOut.thisArgFunc()), e.Args...),
}})
}

func (p *parser) lowerAssignmentOperator(value js_ast.Expr, callback func(js_ast.Expr, js_ast.Expr) js_ast.Expr) js_ast.Expr {
switch left := value.Data.(type) {
case *js_ast.EDot:
Expand Down
18 changes: 16 additions & 2 deletions internal/js_parser/js_parser_lower_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,20 @@ func TestLowerOptionalChain(t *testing.T) {
expectPrintedTarget(t, 2020, "undefined?.[x]", "void 0;\n")
expectPrintedTarget(t, 2020, "undefined?.(x)", "void 0;\n")

expectPrintedTarget(t, 2019, "a?.b()", "a == null ? void 0 : a.b();\n")
expectPrintedTarget(t, 2019, "a?.[b]()", "a == null ? void 0 : a[b]();\n")
expectPrintedTarget(t, 2019, "a?.b.c()", "a == null ? void 0 : a.b.c();\n")
expectPrintedTarget(t, 2019, "a?.b[c]()", "a == null ? void 0 : a.b[c]();\n")
expectPrintedTarget(t, 2019, "a()?.b()", "var _a;\n(_a = a()) == null ? void 0 : _a.b();\n")
expectPrintedTarget(t, 2019, "a()?.[b]()", "var _a;\n(_a = a()) == null ? void 0 : _a[b]();\n")

expectPrintedTarget(t, 2019, "(a?.b)()", "(a == null ? void 0 : a.b).call(a);\n")
expectPrintedTarget(t, 2019, "(a?.[b])()", "(a == null ? void 0 : a[b]).call(a);\n")
expectPrintedTarget(t, 2019, "(a?.b.c)()", "var _a;\n(a == null ? void 0 : (_a = a.b).c).call(_a);\n")
expectPrintedTarget(t, 2019, "(a?.b[c])()", "var _a;\n(a == null ? void 0 : (_a = a.b)[c]).call(_a);\n")
expectPrintedTarget(t, 2019, "(a()?.b)()", "var _a;\n((_a = a()) == null ? void 0 : _a.b).call(_a);\n")
expectPrintedTarget(t, 2019, "(a()?.[b])()", "var _a;\n((_a = a()) == null ? void 0 : _a[b]).call(_a);\n")

// Check multiple levels of nesting
expectPrintedTarget(t, 2019, "a?.b?.c?.d", `var _a, _b;
(_b = (_a = a == null ? void 0 : a.b) == null ? void 0 : _a.c) == null ? void 0 : _b.d;
Expand Down Expand Up @@ -487,8 +501,8 @@ func TestLowerOptionalChain(t *testing.T) {
(_b = (_a = a[b])[c]) == null ? void 0 : _b.call(_a, d);
`)

// Check that direct eval status is propagated through optional chaining
expectPrintedTarget(t, 2019, "eval?.(x)", "eval == null ? void 0 : eval(x);\n")
// Check that direct eval status is not propagated through optional chaining
expectPrintedTarget(t, 2019, "eval?.(x)", "eval == null ? void 0 : (0, eval)(x);\n")
expectPrintedMangleTarget(t, 2019, "(1 ? eval : 0)?.(x)", "eval == null || (0, eval)(x);\n")

// Check super property access
Expand Down

0 comments on commit d012ce1

Please sign in to comment.