Skip to content

Commit

Permalink
fix #2134: nested super() in class transform
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Apr 3, 2022
1 parent 9f0b45f commit 5447e9d
Show file tree
Hide file tree
Showing 7 changed files with 331 additions and 58 deletions.
46 changes: 46 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,52 @@

With this release, esbuild can now parse these new type parameter modifiers. This feature was contributed by [@magic-akari](https://github.com/magic-akari).

* Improve support for `super()` constructor calls in nested locations ([#2134](https://github.com/evanw/esbuild/issues/2134))

In JavaScript, derived classes must call `super()` somewhere in the `constructor` method before being able to access `this`. Class public instance fields, class private instance fields, and TypeScript constructor parameter properties can all potentially cause code which uses `this` to be inserted into the constructor body, which must be inserted after the `super()` call. To make these insertions straightforward to implement, the TypeScript compiler doesn't allow calling `super()` somewhere other than in a root-level statement in the constructor body in these cases.

Previously esbuild's class transformations only worked correctly when `super()` was called in a root-level statement in the constructor body, just like the TypeScript compiler. But with this release, esbuild should now generate correct code as long as the call to `super()` appears anywhere in the constructor body:

```ts
// Original code
class Foo extends Bar {
constructor(public skip = false) {
if (skip) {
super(null)
return
}
super({ keys: [] })
}
}

// Old output (incorrect)
class Foo extends Bar {
constructor(skip = false) {
if (skip) {
super(null);
return;
}
super({ keys: [] });
this.skip = skip;
}
}

// New output (correct)
class Foo extends Bar {
constructor(skip = false) {
var __super = (...args) => {
super(...args);
this.skip = skip;
};
if (skip) {
__super(null);
return;
}
__super({ keys: [] });
}
}
```

## 0.14.30

* Change the context of TypeScript parameter decorators ([#2147](https://github.com/evanw/esbuild/issues/2147))
Expand Down
2 changes: 1 addition & 1 deletion internal/bundler/snapshots/snapshots_lower.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ export class A {
}
export class B extends A {
constructor(c) {
var _a;
super();
__privateAdd(this, _e, void 0);
var _a;
__privateSet(this, _e, (_a = c.d) != null ? _a : "test");
}
f() {
Expand Down
11 changes: 0 additions & 11 deletions internal/js_ast/js_ast.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1021,17 +1021,6 @@ type SContinue struct {
Label *LocRef
}

func IsSuperCall(stmt Stmt) bool {
if expr, ok := stmt.Data.(*SExpr); ok {
if call, ok := expr.Value.Data.(*ECall); ok {
if _, ok := call.Target.Data.(*ESuper); ok {
return true
}
}
}
return false
}

type ClauseItem struct {
Alias string

Expand Down
69 changes: 38 additions & 31 deletions internal/js_parser/js_parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ type parser struct {
importMetaRef js_ast.Ref
promiseRef js_ast.Ref
runtimePublicFieldImport js_ast.Ref
superCtorRef js_ast.Ref

// For lowering private methods
weakMapRef js_ast.Ref
Expand Down Expand Up @@ -7325,15 +7326,8 @@ func (p *parser) visitStmtsAndPrependTempRefs(stmts []js_ast.Stmt, opts prependT
p.recordDeclaredSymbol(temp.ref)
}
}

// If the first statement is a super() call, make sure it stays that way
if len(decls) > 0 {
stmt := js_ast.Stmt{Data: &js_ast.SLocal{Kind: js_ast.LocalVar, Decls: decls}}
if len(stmts) > 0 && js_ast.IsSuperCall(stmts[0]) {
stmts = append([]js_ast.Stmt{stmts[0], stmt}, stmts[1:]...)
} else {
stmts = append([]js_ast.Stmt{stmt}, stmts...)
}
stmts = append([]js_ast.Stmt{{Data: &js_ast.SLocal{Kind: js_ast.LocalVar, Decls: decls}}}, stmts...)
}

p.tempRefsToDeclare = oldTempRefs
Expand Down Expand Up @@ -7590,7 +7584,7 @@ func (p *parser) mangleStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt
// Merge adjacent expression statements
if len(result) > 0 {
prevStmt := result[len(result)-1]
if prevS, ok := prevStmt.Data.(*js_ast.SExpr); ok && !js_ast.IsSuperCall(prevStmt) && !js_ast.IsSuperCall(stmt) {
if prevS, ok := prevStmt.Data.(*js_ast.SExpr); ok {
prevS.Value = js_ast.JoinWithComma(prevS.Value, s.Value)
continue
}
Expand All @@ -7600,7 +7594,7 @@ func (p *parser) mangleStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt
// Absorb a previous expression statement
if len(result) > 0 {
prevStmt := result[len(result)-1]
if prevS, ok := prevStmt.Data.(*js_ast.SExpr); ok && !js_ast.IsSuperCall(prevStmt) {
if prevS, ok := prevStmt.Data.(*js_ast.SExpr); ok {
s.Test = js_ast.JoinWithComma(prevS.Value, s.Test)
result = result[:len(result)-1]
}
Expand All @@ -7610,7 +7604,7 @@ func (p *parser) mangleStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt
// Absorb a previous expression statement
if len(result) > 0 {
prevStmt := result[len(result)-1]
if prevS, ok := prevStmt.Data.(*js_ast.SExpr); ok && !js_ast.IsSuperCall(prevStmt) {
if prevS, ok := prevStmt.Data.(*js_ast.SExpr); ok {
s.Test = js_ast.JoinWithComma(prevS.Value, s.Test)
result = result[:len(result)-1]
}
Expand Down Expand Up @@ -7713,7 +7707,7 @@ func (p *parser) mangleStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt
// Merge return statements with the previous expression statement
if len(result) > 0 && s.ValueOrNil.Data != nil {
prevStmt := result[len(result)-1]
if prevS, ok := prevStmt.Data.(*js_ast.SExpr); ok && !js_ast.IsSuperCall(prevStmt) {
if prevS, ok := prevStmt.Data.(*js_ast.SExpr); ok {
result[len(result)-1] = js_ast.Stmt{Loc: prevStmt.Loc,
Data: &js_ast.SReturn{ValueOrNil: js_ast.JoinWithComma(prevS.Value, s.ValueOrNil)}}
continue
Expand All @@ -7726,7 +7720,7 @@ func (p *parser) mangleStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt
// Merge throw statements with the previous expression statement
if len(result) > 0 {
prevStmt := result[len(result)-1]
if prevS, ok := prevStmt.Data.(*js_ast.SExpr); ok && !js_ast.IsSuperCall(prevStmt) {
if prevS, ok := prevStmt.Data.(*js_ast.SExpr); ok {
result[len(result)-1] = js_ast.Stmt{Loc: prevStmt.Loc, Data: &js_ast.SThrow{Value: js_ast.JoinWithComma(prevS.Value, s.Value)}}
continue
}
Expand All @@ -7740,7 +7734,7 @@ func (p *parser) mangleStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt
case *js_ast.SFor:
if len(result) > 0 {
prevStmt := result[len(result)-1]
if prevS, ok := prevStmt.Data.(*js_ast.SExpr); ok && !js_ast.IsSuperCall(prevStmt) {
if prevS, ok := prevStmt.Data.(*js_ast.SExpr); ok {
// Insert the previous expression into the for loop initializer
if s.InitOrNil.Data == nil {
result[len(result)-1] = stmt
Expand Down Expand Up @@ -7841,11 +7835,6 @@ func (p *parser) mangleStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt
break returnLoop
}

// Do not absorb a "super()" call so that we keep it first
if js_ast.IsSuperCall(prevStmt) {
break returnLoop
}

// "a(); return b;" => "return a(), b;"
lastReturn = &js_ast.SReturn{ValueOrNil: js_ast.JoinWithComma(prevS.Value, lastReturn.ValueOrNil)}

Expand Down Expand Up @@ -7918,11 +7907,6 @@ func (p *parser) mangleStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt

switch prevS := prevStmt.Data.(type) {
case *js_ast.SExpr:
// Do not absorb a "super()" call so that we keep it first
if js_ast.IsSuperCall(prevStmt) {
break throwLoop
}

// "a(); throw b;" => "throw a(), b;"
lastThrow = &js_ast.SThrow{Value: js_ast.JoinWithComma(prevS.Value, lastThrow.Value)}

Expand Down Expand Up @@ -9020,10 +9004,10 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_
return stmts

case *js_ast.SClass:
shadowRef := p.visitClass(s.Value.Loc, &s2.Class, true /* isDefaultExport */)
result := p.visitClass(s.Value.Loc, &s2.Class, true /* isDefaultExport */)

// Lower class field syntax for browsers that don't support it
classStmts, _ := p.lowerClass(stmt, js_ast.Expr{}, shadowRef)
classStmts, _ := p.lowerClass(stmt, js_ast.Expr{}, result)
return append(stmts, classStmts...)

default:
Expand Down Expand Up @@ -9596,7 +9580,7 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_
return stmts

case *js_ast.SClass:
shadowRef := p.visitClass(stmt.Loc, &s.Class, false /* isDefaultExport */)
result := p.visitClass(stmt.Loc, &s.Class, false /* isDefaultExport */)

// Remove the export flag inside a namespace
wasExportInsideNamespace := s.IsExport && p.enclosingNamespaceArgRef != nil
Expand All @@ -9605,7 +9589,7 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_
}

// Lower class field syntax for browsers that don't support it
classStmts, _ := p.lowerClass(stmt, js_ast.Expr{}, shadowRef)
classStmts, _ := p.lowerClass(stmt, js_ast.Expr{}, result)
stmts = append(stmts, classStmts...)

// Handle exporting this class from a namespace
Expand Down Expand Up @@ -10117,7 +10101,8 @@ func (p *parser) visitTSDecorators(tsDecorators []js_ast.Expr, tsDecoratorScope
}

type visitClassResult struct {
shadowRef js_ast.Ref
shadowRef js_ast.Ref
superCtorRef js_ast.Ref
}

func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class, isDefaultExport bool) (result visitClassResult) {
Expand Down Expand Up @@ -10185,6 +10170,18 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class, isDefa
p.validateDeclaredSymbolName(class.Name.Loc, p.symbols[class.Name.Ref.InnerIndex].OriginalName)
}

// Create the "__super" symbol if necessary. This will cause us to replace
// all "super()" call expressions with a call to this symbol, which will
// then be inserted into the "constructor" method.
result.superCtorRef = js_ast.InvalidRef
if classLoweringInfo.shimSuperCtorCalls {
result.superCtorRef = p.newSymbol(js_ast.SymbolOther, "__super")
p.currentScope.Generated = append(p.currentScope.Generated, result.superCtorRef)
p.recordDeclaredSymbol(result.superCtorRef)
}
oldSuperCtorRef := p.superCtorRef
p.superCtorRef = result.superCtorRef

var classNameRef js_ast.Ref
if class.Name != nil {
classNameRef = class.Name.Ref
Expand Down Expand Up @@ -10371,6 +10368,7 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class, isDefa
class.Properties = class.Properties[:end]

p.enclosingClassKeyword = oldEnclosingClassKeyword
p.superCtorRef = oldSuperCtorRef
p.popScope()

if p.symbols[result.shadowRef.InnerIndex].UseCountEstimate == 0 {
Expand Down Expand Up @@ -13288,6 +13286,14 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
if t.CallCanBeUnwrappedIfUnused {
e.CanBeUnwrappedIfUnused = true
}

case *js_ast.ESuper:
// If we're shimming "super()" calls, replace this call with "__super()"
if p.superCtorRef != js_ast.InvalidRef {
p.recordUsage(p.superCtorRef)
target.Data = &js_ast.EIdentifier{Ref: p.superCtorRef}
e.Target.Data = target.Data
}
}

// Handle parenthesized optional chains
Expand Down Expand Up @@ -13502,10 +13508,10 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
}

case *js_ast.EClass:
shadowRef := p.visitClass(expr.Loc, &e.Class, false /* isDefaultExport */)
result := p.visitClass(expr.Loc, &e.Class, false /* isDefaultExport */)

// Lower class field syntax for browsers that don't support it
_, expr = p.lowerClass(js_ast.Stmt{}, expr, shadowRef)
_, expr = p.lowerClass(js_ast.Stmt{}, expr, result)

default:
// Note: EPrivateIdentifier and EMangledProperty should have already been handled
Expand Down Expand Up @@ -14729,6 +14735,7 @@ func newParser(log logger.Log, source logger.Source, lexer js_lexer.Lexer, optio
afterArrowBodyLoc: logger.Loc{Start: -1},
importMetaRef: js_ast.InvalidRef,
runtimePublicFieldImport: js_ast.InvalidRef,
superCtorRef: js_ast.InvalidRef,

// For lowering private methods
weakMapRef: js_ast.InvalidRef,
Expand Down
Loading

0 comments on commit 5447e9d

Please sign in to comment.