Skip to content

Commit

Permalink
fix another super class lowering edge case
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Apr 2, 2022
1 parent ae23f41 commit f4c6b54
Show file tree
Hide file tree
Showing 5 changed files with 135 additions and 70 deletions.
52 changes: 30 additions & 22 deletions CHANGELOG.md
View file
Expand Up @@ -85,35 +85,43 @@
], Class.prototype, "method", 1);
```

* Fix a compiler crash regarding `super` property access
* Fix some obscure edge cases with `super` property access

This release fixes a compiler crash that happened when an async arrow function contained a class with a field initializer that used a `super` property access, which can be seen in the following code below:
This release fixes the following obscure problems with `super` when targeting an older JavaScript environment such as `--target=es6`:

```js
let foo = async () => class extends Object {
bar = super.toString;
};
```
1. The compiler could previously crash when a lowered `async` arrow function contained a class with a field initializer that used a `super` property access:

This crash only happened when targeting an older JavaScript environment that doesn't support `async` such as `--target=es6`.
```js
let foo = async () => class extends Object {
bar = super.toString
}
```

* Fix an obscure edge case with `super` property access
2. The compiler could previously generate incorrect code when a lowered `async` method of a derived class contained a nested class with a computed class member involving a `super` property access on the derived class:

This release fixes incorrect code generation in the case when an async method of a derived class contained a nested class with a computed class member involving a `super` property access on the derived class, which can be seen in the following code below:
```js
class Base {
foo() { return 'bar' }
}
class Derived extends Base {
async foo() {
return new class { [super.foo()] = 'success' }
}
}
new Derived().foo().then(obj => console.log(obj.bar))
```

```js
class Base {
foo() { return 'bar' }
}
class Derived extends Base {
async foo() {
return new class { [super.foo()] = 'success' }
}
}
new Derived().foo().then(obj => console.log(obj.bar))
```
3. The compiler could previously generate incorrect code when a default-exported class containing a `super` property access inside a lowered static private class field:

In that case, esbuild previously generated invalid code that would crash when run due to a missing helper function. This only happened when targeting an older JavaScript environment that doesn't support `async` such as `--target=es6`, and has now been fixed.
```js
class Foo {
static foo = 123
}
export default class extends Foo {
static #foo = super.foo
static bar = this.#foo
}
```

## 0.14.29

Expand Down
84 changes: 42 additions & 42 deletions internal/bundler/snapshots/snapshots_lower.txt
View file
Expand Up @@ -1525,36 +1525,36 @@ var foo4_default = class extends x {
};

// bar1.js
var _this = class extends x {
var _default = class extends x {
};
var this2 = _this;
__publicField(this2, "bar1", () => __async(_this, null, function* () {
return __superStaticGet(_this, "foo").call(this, "bar1");
var default2 = _default;
__publicField(default2, "bar1", () => __async(_default, null, function* () {
return __superStaticGet(_default, "foo").call(this, "bar1");
}));

// bar2.js
var _this2 = class extends x {
var _default2 = class extends x {
};
var this2 = _this2;
__publicField(this2, "bar2", () => __async(_this2, null, function* () {
return () => __superStaticGet(_this2, "foo").call(this, "bar2");
var default2 = _default2;
__publicField(default2, "bar2", () => __async(_default2, null, function* () {
return () => __superStaticGet(_default2, "foo").call(this, "bar2");
}));

// bar3.js
var _this3 = class extends x {
var _default3 = class extends x {
};
var this2 = _this3;
__publicField(this2, "bar3", () => () => __async(_this3, null, function* () {
return __superStaticGet(_this3, "foo").call(this, "bar3");
var default2 = _default3;
__publicField(default2, "bar3", () => () => __async(_default3, null, function* () {
return __superStaticGet(_default3, "foo").call(this, "bar3");
}));

// bar4.js
var _this4 = class extends x {
var _default4 = class extends x {
};
var this2 = _this4;
__publicField(this2, "bar4", () => __async(_this4, null, function* () {
return () => __async(_this4, null, function* () {
return __superStaticGet(_this4, "foo").call(this, "bar4");
var default2 = _default4;
__publicField(default2, "bar4", () => __async(_default4, null, function* () {
return () => __async(_default4, null, function* () {
return __superStaticGet(_default4, "foo").call(this, "bar4");
});
}));

Expand Down Expand Up @@ -1591,10 +1591,10 @@ var outer_default = function() {
});
}();
export {
bar1_default as bar1,
bar2_default as bar2,
bar3_default as bar3,
bar4_default as bar4,
_default as bar1,
_default2 as bar2,
_default3 as bar3,
_default4 as bar4,
baz1_default as baz1,
baz2_default as baz2,
foo1_default as foo1,
Expand Down Expand Up @@ -1657,36 +1657,36 @@ var foo4_default = class extends x {
};

// bar1.js
var _this = class extends x {
var _default = class extends x {
};
var this2 = _this;
__publicField(this2, "bar1", () => __async(_this, null, function* () {
return __superStaticSet(_this, "foo", "bar1");
var default2 = _default;
__publicField(default2, "bar1", () => __async(_default, null, function* () {
return __superStaticSet(_default, "foo", "bar1");
}));

// bar2.js
var _this2 = class extends x {
var _default2 = class extends x {
};
var this2 = _this2;
__publicField(this2, "bar2", () => __async(_this2, null, function* () {
return () => __superStaticSet(_this2, "foo", "bar2");
var default2 = _default2;
__publicField(default2, "bar2", () => __async(_default2, null, function* () {
return () => __superStaticSet(_default2, "foo", "bar2");
}));

// bar3.js
var _this3 = class extends x {
var _default3 = class extends x {
};
var this2 = _this3;
__publicField(this2, "bar3", () => () => __async(_this3, null, function* () {
return __superStaticSet(_this3, "foo", "bar3");
var default2 = _default3;
__publicField(default2, "bar3", () => () => __async(_default3, null, function* () {
return __superStaticSet(_default3, "foo", "bar3");
}));

// bar4.js
var _this4 = class extends x {
var _default4 = class extends x {
};
var this2 = _this4;
__publicField(this2, "bar4", () => __async(_this4, null, function* () {
return () => __async(_this4, null, function* () {
return __superStaticSet(_this4, "foo", "bar4");
var default2 = _default4;
__publicField(default2, "bar4", () => __async(_default4, null, function* () {
return () => __async(_default4, null, function* () {
return __superStaticSet(_default4, "foo", "bar4");
});
}));

Expand Down Expand Up @@ -1723,10 +1723,10 @@ var outer_default = function() {
});
}();
export {
bar1_default as bar1,
bar2_default as bar2,
bar3_default as bar3,
bar4_default as bar4,
_default as bar1,
_default2 as bar2,
_default3 as bar3,
_default4 as bar4,
baz1_default as baz1,
baz2_default as baz2,
foo1_default as foo1,
Expand Down
31 changes: 25 additions & 6 deletions internal/js_parser/js_parser.go
View file
Expand Up @@ -1101,11 +1101,26 @@ func (p *parser) newSymbol(kind js_ast.SymbolKind, name string) js_ast.Ref {
// one-level symbol map instead of the linker's two-level symbol map. It also
// doesn't handle cycles since they shouldn't come up due to the way this
// function is used.
func (p *parser) mergeSymbols(old js_ast.Ref, new js_ast.Ref) {
func (p *parser) mergeSymbols(old js_ast.Ref, new js_ast.Ref) js_ast.Ref {
if old == new {
return new
}

oldSymbol := &p.symbols[old.InnerIndex]
if oldSymbol.Link != js_ast.InvalidRef {
oldSymbol.Link = p.mergeSymbols(oldSymbol.Link, new)
return oldSymbol.Link
}

newSymbol := &p.symbols[new.InnerIndex]
if newSymbol.Link != js_ast.InvalidRef {
newSymbol.Link = p.mergeSymbols(old, newSymbol.Link)
return newSymbol.Link
}

oldSymbol.Link = new
newSymbol.MergeContentsWith(oldSymbol)
return new
}

type mergeResult int
Expand Down Expand Up @@ -9005,7 +9020,7 @@ 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)
shadowRef := 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)
Expand Down Expand Up @@ -9581,7 +9596,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)
shadowRef := p.visitClass(stmt.Loc, &s.Class, false /* isDefaultExport */)

// Remove the export flag inside a namespace
wasExportInsideNamespace := s.IsExport && p.enclosingNamespaceArgRef != nil
Expand Down Expand Up @@ -10101,7 +10116,7 @@ func (p *parser) visitTSDecorators(tsDecorators []js_ast.Expr, tsDecoratorScope
return tsDecorators
}

func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class) js_ast.Ref {
func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class, isDefaultExport bool) js_ast.Ref {
tsDecoratorScope := p.currentScope
class.TSDecorators = p.visitTSDecorators(class.TSDecorators, tsDecoratorScope)

Expand Down Expand Up @@ -10172,7 +10187,11 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class) js_ast
} else {
// Generate a name if one doesn't already exist. This is necessary for
// handling "this" in static class property initializers.
classNameRef = p.newSymbol(js_ast.SymbolOther, "this")
name := "this"
if isDefaultExport {
name = "default" // This is important for "--keep-names"
}
classNameRef = p.newSymbol(js_ast.SymbolOther, name)
}

// Insert a shadowing name that spans the whole class, which matches
Expand Down Expand Up @@ -13485,7 +13504,7 @@ 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)
shadowRef := 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)
Expand Down
7 changes: 7 additions & 0 deletions internal/js_parser/js_parser_lower.go
View file
Expand Up @@ -2038,6 +2038,13 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, shadowRef js_ast
class = &s2.Class
defaultName = s.DefaultName
kind = classKindExportDefaultStmt

// The shadowing name inside the class expression should be the same as
// the default export name
if shadowRef != js_ast.InvalidRef {
p.mergeSymbols(shadowRef, defaultName.Ref)
}

if class.Name != nil {
nameToKeep = p.symbols[class.Name.Ref.InnerIndex].OriginalName
} else {
Expand Down
31 changes: 31 additions & 0 deletions scripts/end-to-end-tests.js
View file
Expand Up @@ -3767,6 +3767,37 @@
if (t.x !== 1 || t.y[0] !== 2 || t.y[1] !== 3 || t.z !== 4) throw 'fail';
`,
}),
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `
import x from './class'
if (x.bar !== 123) throw 'fail'
`,
'class.js': `
class Foo {
static foo = 123
}
export default class extends Foo {
static #foo = super.foo
static bar = this.#foo
}
`,
}),
test(['in.js', '--outfile=node.js', '--bundle', '--keep-names'].concat(flags), {
'in.js': `
import x from './class'
if (x.bar !== 123) throw 'fail'
if (x.name !== 'default') throw 'fail: ' + x.name
`,
'class.js': `
class Foo {
static foo = 123
}
export default class extends Foo {
static #foo = super.foo
static bar = this.#foo
}
`,
}),
test(['in.js', '--outfile=node.js'].concat(flags), {
'in.js': `
class Foo {
Expand Down

0 comments on commit f4c6b54

Please sign in to comment.