Skip to content

Commit 0a7ac6f

Browse files
evanwevanw
committed
fix #2039: lower super in private methods
1 parent 7f87558 commit 0a7ac6f

File tree

5 files changed

+500
-51
lines changed

5 files changed

+500
-51
lines changed

‎CHANGELOG.md

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,82 @@
11
# Changelog
22

3+
## Unreleased
4+
5+
* Fix `super` usage in lowered private methods ([#2039](https://github.com/evanw/esbuild/issues/2039))
6+
7+
Previously esbuild failed to transform `super` property accesses inside private methods in the case when private methods have to be lowered because the target environment doesn't support them. The generated code still contained the `super` keyword even though the method was moved outside of the class body, which is a syntax error in JavaScript. This release fixes this transformation issue and now produces valid code:
8+
9+
```js
10+
// Original code
11+
class Derived extends Base {
12+
#foo() { super.foo() }
13+
bar() { this.#foo() }
14+
}
15+
16+
// Old output (with --target=es6)
17+
var _foo, foo_fn;
18+
class Derived extends Base {
19+
constructor() {
20+
super(...arguments);
21+
__privateAdd(this, _foo);
22+
}
23+
bar() {
24+
__privateMethod(this, _foo, foo_fn).call(this);
25+
}
26+
}
27+
_foo = new WeakSet();
28+
foo_fn = function() {
29+
super.foo();
30+
};
31+
32+
// New output (with --target=es6)
33+
var _foo, foo_fn;
34+
const _Derived = class extends Base {
35+
constructor() {
36+
super(...arguments);
37+
__privateAdd(this, _foo);
38+
}
39+
bar() {
40+
__privateMethod(this, _foo, foo_fn).call(this);
41+
}
42+
};
43+
let Derived = _Derived;
44+
_foo = new WeakSet();
45+
foo_fn = function() {
46+
__superGet(_Derived.prototype, this, "foo").call(this);
47+
};
48+
```
49+
50+
Because of this change, lowered `super` property accesses on instances were rewritten so that they can exist outside of the class body. This rewrite affects code generation for a `super` property accesses on instances including those inside lowered `async` functions. The new approach is different but should be equivalent to the old approach:
51+
52+
```js
53+
// Original code
54+
class Foo {
55+
foo = async () => super.foo()
56+
}
57+
58+
// Old output (with --target=es6)
59+
class Foo {
60+
constructor() {
61+
__publicField(this, "foo", () => {
62+
var __superGet = (key) => super[key];
63+
return __async(this, null, function* () {
64+
return __superGet("foo").call(this);
65+
});
66+
});
67+
}
68+
}
69+
70+
// New output (with --target=es6)
71+
class Foo {
72+
constructor() {
73+
__publicField(this, "foo", () => __async(this, null, function* () {
74+
return __superGet(Foo.prototype, this, "foo").call(this);
75+
}));
76+
}
77+
}
78+
```
79+
380
## 0.14.31
481

582
* Add support for parsing "optional variance annotations" from TypeScript 4.7 ([#2102](https://github.com/evanw/esbuild/pull/2102))

‎internal/bundler/bundler_lower_test.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1470,6 +1470,68 @@ func TestLowerStaticAsyncArrowSuperSetterES2016(t *testing.T) {
14701470
})
14711471
}
14721472

1473+
func TestLowerPrivateSuperES2022(t *testing.T) {
1474+
lower_suite.expectBundled(t, bundled{
1475+
files: map[string]string{
1476+
"/entry.js": `
1477+
export { default as foo1 } from "./foo1"
1478+
export { default as foo2 } from "./foo2"
1479+
export { default as foo3 } from "./foo3"
1480+
export { default as foo4 } from "./foo4"
1481+
export { default as foo5 } from "./foo5"
1482+
export { default as foo6 } from "./foo6"
1483+
export { default as foo7 } from "./foo7"
1484+
export { default as foo8 } from "./foo8"
1485+
`,
1486+
"/foo1.js": `export default class extends x { #foo() { super.foo() } }`,
1487+
"/foo2.js": `export default class extends x { #foo() { super.foo++ } }`,
1488+
"/foo3.js": `export default class extends x { static #foo() { super.foo() } }`,
1489+
"/foo4.js": `export default class extends x { static #foo() { super.foo++ } }`,
1490+
"/foo5.js": `export default class extends x { #foo = () => { super.foo() } }`,
1491+
"/foo6.js": `export default class extends x { #foo = () => { super.foo++ } }`,
1492+
"/foo7.js": `export default class extends x { static #foo = () => { super.foo() } }`,
1493+
"/foo8.js": `export default class extends x { static #foo = () => { super.foo++ } }`,
1494+
},
1495+
entryPaths: []string{"/entry.js"},
1496+
options: config.Options{
1497+
Mode: config.ModeBundle,
1498+
UnsupportedJSFeatures: es(2022),
1499+
AbsOutputFile: "/out.js",
1500+
},
1501+
})
1502+
}
1503+
1504+
func TestLowerPrivateSuperES2021(t *testing.T) {
1505+
lower_suite.expectBundled(t, bundled{
1506+
files: map[string]string{
1507+
"/entry.js": `
1508+
export { default as foo1 } from "./foo1"
1509+
export { default as foo2 } from "./foo2"
1510+
export { default as foo3 } from "./foo3"
1511+
export { default as foo4 } from "./foo4"
1512+
export { default as foo5 } from "./foo5"
1513+
export { default as foo6 } from "./foo6"
1514+
export { default as foo7 } from "./foo7"
1515+
export { default as foo8 } from "./foo8"
1516+
`,
1517+
"/foo1.js": `export default class extends x { #foo() { super.foo() } }`,
1518+
"/foo2.js": `export default class extends x { #foo() { super.foo++ } }`,
1519+
"/foo3.js": `export default class extends x { static #foo() { super.foo() } }`,
1520+
"/foo4.js": `export default class extends x { static #foo() { super.foo++ } }`,
1521+
"/foo5.js": `export default class extends x { #foo = () => { super.foo() } }`,
1522+
"/foo6.js": `export default class extends x { #foo = () => { super.foo++ } }`,
1523+
"/foo7.js": `export default class extends x { static #foo = () => { super.foo() } }`,
1524+
"/foo8.js": `export default class extends x { static #foo = () => { super.foo++ } }`,
1525+
},
1526+
entryPaths: []string{"/entry.js"},
1527+
options: config.Options{
1528+
Mode: config.ModeBundle,
1529+
UnsupportedJSFeatures: es(2021),
1530+
AbsOutputFile: "/out.js",
1531+
},
1532+
})
1533+
}
1534+
14731535
func TestLowerClassField2020NoBundle(t *testing.T) {
14741536
lower_suite.expectBundled(t, bundled{
14751537
files: map[string]string{

‎internal/bundler/snapshots/snapshots_lower.txt

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1434,6 +1434,196 @@ __privateAdd(Foo, _sg);
14341434
__privateAdd(Foo, _sa);
14351435
__privateAdd(Foo, _sag);
14361436

1437+
================================================================================
1438+
TestLowerPrivateSuperES2021
1439+
---------- /out.js ----------
1440+
// foo1.js
1441+
var _foo, foo_fn;
1442+
var _foo1_default = class extends x {
1443+
constructor() {
1444+
super(...arguments);
1445+
__privateAdd(this, _foo);
1446+
}
1447+
};
1448+
var foo1_default = _foo1_default;
1449+
_foo = new WeakSet();
1450+
foo_fn = function() {
1451+
__superGet(_foo1_default.prototype, this, "foo").call(this);
1452+
};
1453+
1454+
// foo2.js
1455+
var _foo2, foo_fn2;
1456+
var _foo2_default = class extends x {
1457+
constructor() {
1458+
super(...arguments);
1459+
__privateAdd(this, _foo2);
1460+
}
1461+
};
1462+
var foo2_default = _foo2_default;
1463+
_foo2 = new WeakSet();
1464+
foo_fn2 = function() {
1465+
__superWrapper(_foo2_default.prototype, this, "foo")._++;
1466+
};
1467+
1468+
// foo3.js
1469+
var _foo3, foo_fn3;
1470+
var _foo3_default = class extends x {
1471+
};
1472+
var foo3_default = _foo3_default;
1473+
_foo3 = new WeakSet();
1474+
foo_fn3 = function() {
1475+
__superGet(_foo3_default, this, "foo").call(this);
1476+
};
1477+
__privateAdd(foo3_default, _foo3);
1478+
1479+
// foo4.js
1480+
var _foo4, foo_fn4;
1481+
var _foo4_default = class extends x {
1482+
};
1483+
var foo4_default = _foo4_default;
1484+
_foo4 = new WeakSet();
1485+
foo_fn4 = function() {
1486+
__superWrapper(_foo4_default, this, "foo")._++;
1487+
};
1488+
__privateAdd(foo4_default, _foo4);
1489+
1490+
// foo5.js
1491+
var _foo5;
1492+
var foo5_default = class extends x {
1493+
constructor() {
1494+
super(...arguments);
1495+
__privateAdd(this, _foo5, () => {
1496+
super.foo();
1497+
});
1498+
}
1499+
};
1500+
_foo5 = new WeakMap();
1501+
1502+
// foo6.js
1503+
var _foo6;
1504+
var foo6_default = class extends x {
1505+
constructor() {
1506+
super(...arguments);
1507+
__privateAdd(this, _foo6, () => {
1508+
super.foo++;
1509+
});
1510+
}
1511+
};
1512+
_foo6 = new WeakMap();
1513+
1514+
// foo7.js
1515+
var _foo7;
1516+
var _foo7_default = class extends x {
1517+
};
1518+
var foo7_default = _foo7_default;
1519+
_foo7 = new WeakMap();
1520+
__privateAdd(foo7_default, _foo7, () => {
1521+
__superGet(_foo7_default, _foo7_default, "foo").call(this);
1522+
});
1523+
1524+
// foo8.js
1525+
var _foo8;
1526+
var _foo8_default = class extends x {
1527+
};
1528+
var foo8_default = _foo8_default;
1529+
_foo8 = new WeakMap();
1530+
__privateAdd(foo8_default, _foo8, () => {
1531+
__superWrapper(_foo8_default, _foo8_default, "foo")._++;
1532+
});
1533+
export {
1534+
foo1_default as foo1,
1535+
foo2_default as foo2,
1536+
foo3_default as foo3,
1537+
foo4_default as foo4,
1538+
foo5_default as foo5,
1539+
foo6_default as foo6,
1540+
foo7_default as foo7,
1541+
foo8_default as foo8
1542+
};
1543+
1544+
================================================================================
1545+
TestLowerPrivateSuperES2022
1546+
---------- /out.js ----------
1547+
// foo1.js
1548+
var foo1_default = class extends x {
1549+
#foo() {
1550+
super.foo();
1551+
}
1552+
};
1553+
1554+
// foo2.js
1555+
var foo2_default = class extends x {
1556+
#foo() {
1557+
super.foo++;
1558+
}
1559+
};
1560+
1561+
// foo3.js
1562+
var _foo, foo_fn;
1563+
var _foo3_default = class extends x {
1564+
};
1565+
var foo3_default = _foo3_default;
1566+
_foo = new WeakSet();
1567+
foo_fn = function() {
1568+
__superGet(_foo3_default, this, "foo").call(this);
1569+
};
1570+
__privateAdd(foo3_default, _foo);
1571+
1572+
// foo4.js
1573+
var _foo2, foo_fn2;
1574+
var _foo4_default = class extends x {
1575+
};
1576+
var foo4_default = _foo4_default;
1577+
_foo2 = new WeakSet();
1578+
foo_fn2 = function() {
1579+
__superWrapper(_foo4_default, this, "foo")._++;
1580+
};
1581+
__privateAdd(foo4_default, _foo2);
1582+
1583+
// foo5.js
1584+
var foo5_default = class extends x {
1585+
#foo = () => {
1586+
super.foo();
1587+
};
1588+
};
1589+
1590+
// foo6.js
1591+
var foo6_default = class extends x {
1592+
#foo = () => {
1593+
super.foo++;
1594+
};
1595+
};
1596+
1597+
// foo7.js
1598+
var _foo3;
1599+
var _foo7_default = class extends x {
1600+
};
1601+
var foo7_default = _foo7_default;
1602+
_foo3 = new WeakMap();
1603+
__privateAdd(foo7_default, _foo3, () => {
1604+
__superGet(_foo7_default, _foo7_default, "foo").call(this);
1605+
});
1606+
1607+
// foo8.js
1608+
var _foo4;
1609+
var _foo8_default = class extends x {
1610+
};
1611+
var foo8_default = _foo8_default;
1612+
_foo4 = new WeakMap();
1613+
__privateAdd(foo8_default, _foo4, () => {
1614+
__superWrapper(_foo8_default, _foo8_default, "foo")._++;
1615+
});
1616+
export {
1617+
foo1_default as foo1,
1618+
foo2_default as foo2,
1619+
foo3_default as foo3,
1620+
foo4_default as foo4,
1621+
foo5_default as foo5,
1622+
foo6_default as foo6,
1623+
foo7_default as foo7,
1624+
foo8_default as foo8
1625+
};
1626+
14371627
================================================================================
14381628
TestLowerStaticAsyncArrowSuperES2016
14391629
---------- /out.js ----------

‎internal/js_parser/js_parser.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10353,6 +10353,15 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class, defaul
1035310353
nameToKeep := ""
1035410354
if private, isPrivate := property.Key.Data.(*js_ast.EPrivateIdentifier); isPrivate && p.privateSymbolNeedsToBeLowered(private) {
1035510355
nameToKeep = p.symbols[private.Ref.InnerIndex].OriginalName
10356+
10357+
// Lowered private methods (both instance and static) are initialized
10358+
// outside of the class body, so we must rewrite "super" property
10359+
// accesses inside them. Lowered private instance fields are initialized
10360+
// inside the constructor where "super" is valid, so those don't need to
10361+
// be rewritten.
10362+
if property.IsMethod {
10363+
p.fnOrArrowDataVisit.shouldLowerSuperPropertyAccess = true
10364+
}
1035610365
} else if !property.IsMethod && !property.IsComputed &&
1035710366
((!property.IsStatic && p.options.unsupportedJSFeatures.Has(compat.ClassField)) ||
1035810367
(property.IsStatic && p.options.unsupportedJSFeatures.Has(compat.ClassStaticField))) {
@@ -13932,6 +13941,9 @@ func (p *parser) visitFn(fn *js_ast.Fn, scopeLoc logger.Loc, opts visitFnOpts) {
1393213941
tsDecoratorScope = p.propMethodTSDecoratorScope
1393313942
p.fnOnlyDataVisit.classNameRef = oldFnOnlyData.classNameRef
1393413943
p.fnOnlyDataVisit.isInStaticClassContext = oldFnOnlyData.isInStaticClassContext
13944+
if oldFnOrArrowData.shouldLowerSuperPropertyAccess {
13945+
p.fnOrArrowDataVisit.shouldLowerSuperPropertyAccess = true
13946+
}
1393513947
}
1393613948

1393713949
if fn.Name != nil {

0 commit comments

Comments
 (0)