Skip to content

Commit d5e343b

Browse files
evanwevanw
committed
fix some --keep-names edge cases
1 parent d124695 commit d5e343b

File tree

4 files changed

+196
-6
lines changed

4 files changed

+196
-6
lines changed

‎CHANGELOG.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,31 @@
146146
}
147147
```
148148

149+
* Fix some `--keep-names` edge cases
150+
151+
The [`NamedEvaluation` syntax-directed operation](https://tc39.es/ecma262/#sec-runtime-semantics-namedevaluation) in the JavaScript specification gives certain anonymous expressions a `name` property depending on where they are in the syntax tree. For example, the following initializers convey a `name` value:
152+
153+
```js
154+
var foo = function() {}
155+
var bar = class {}
156+
console.log(foo.name, bar.name)
157+
```
158+
159+
When you enable esbuild's `--keep-names` setting, esbuild generates additional code to represent this `NamedEvaluation` operation so that the value of the `name` property persists even when the identifiers are renamed (e.g. due to minification).
160+
161+
However, I recently learned that esbuild's implementation of `NamedEvaluation` is missing a few cases. Specifically esbuild was missing property definitions, class initializers, logical-assignment operators. These cases should now all be handled:
162+
163+
```js
164+
var obj = { foo: function() {} }
165+
class Foo0 { foo = function() {} }
166+
class Foo1 { static foo = function() {} }
167+
class Foo2 { accessor foo = function() {} }
168+
class Foo3 { static accessor foo = function() {} }
169+
foo ||= function() {}
170+
foo &&= function() {}
171+
foo ??= function() {}
172+
```
173+
149174
## 0.20.2
150175

151176
* Support TypeScript experimental decorators on `abstract` class fields ([#3684](https://github.com/evanw/esbuild/issues/3684))

‎internal/bundler_tests/bundler_default_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5496,6 +5496,54 @@ NOTE: The expression "b['c']" has been configured to be replaced with a constant
54965496
})
54975497
}
54985498

5499+
func TestKeepNamesAllForms(t *testing.T) {
5500+
default_suite.expectBundled(t, bundled{
5501+
files: map[string]string{
5502+
"/entry.js": `
5503+
// Initializers
5504+
function fn() {}
5505+
function foo(fn = function() {}) {}
5506+
var fn = function() {};
5507+
var obj = { "f n": function() {} };
5508+
class Foo0 { "f n" = function() {} }
5509+
class Foo1 { static "f n" = function() {} }
5510+
class Foo2 { accessor "f n" = function() {} }
5511+
class Foo3 { static accessor "f n" = function() {} }
5512+
class Foo4 { #fn = function() {} }
5513+
class Foo5 { static #fn = function() {} }
5514+
class Foo6 { accessor #fn = function() {} }
5515+
class Foo7 { static accessor #fn = function() {} }
5516+
5517+
// Assignments
5518+
fn = function() {};
5519+
fn ||= function() {};
5520+
fn &&= function() {};
5521+
fn ??= function() {};
5522+
5523+
// Destructuring
5524+
var [fn = function() {}] = [];
5525+
var { fn = function() {} } = {};
5526+
for (var [fn = function() {}] = []; ; ) ;
5527+
for (var { fn = function() {} } = {}; ; ) ;
5528+
for (var [fn = function() {}] in obj) ;
5529+
for (var { fn = function() {} } in obj) ;
5530+
for (var [fn = function() {}] of obj) ;
5531+
for (var { fn = function() {} } of obj) ;
5532+
function foo([fn = function() {}]) {}
5533+
function foo({ fn = function() {} }) {}
5534+
[fn = function() {}] = [];
5535+
({ fn = function() {} } = {});
5536+
`,
5537+
},
5538+
entryPaths: []string{"/entry.js"},
5539+
options: config.Options{
5540+
Mode: config.ModePassThrough,
5541+
AbsOutputFile: "/out.js",
5542+
KeepNames: true,
5543+
},
5544+
})
5545+
}
5546+
54995547
func TestKeepNamesTreeShaking(t *testing.T) {
55005548
default_suite.expectBundled(t, bundled{
55015549
files: map[string]string{

‎internal/bundler_tests/snapshots/snapshots_default.txt

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2706,6 +2706,119 @@ console.log([
27062706
]);
27072707
};
27082708

2709+
================================================================================
2710+
TestKeepNamesAllForms
2711+
---------- /out.js ----------
2712+
function fn() {
2713+
}
2714+
__name(fn, "fn");
2715+
function foo(fn2 = function() {
2716+
}) {
2717+
}
2718+
__name(foo, "foo");
2719+
var fn = /* @__PURE__ */ __name(function() {
2720+
}, "fn");
2721+
var obj = { "f n": /* @__PURE__ */ __name(function() {
2722+
}, "f n") };
2723+
class Foo0 {
2724+
static {
2725+
__name(this, "Foo0");
2726+
}
2727+
"f n" = /* @__PURE__ */ __name(function() {
2728+
}, "f n");
2729+
}
2730+
class Foo1 {
2731+
static {
2732+
__name(this, "Foo1");
2733+
}
2734+
static "f n" = /* @__PURE__ */ __name(function() {
2735+
}, "f n");
2736+
}
2737+
class Foo2 {
2738+
static {
2739+
__name(this, "Foo2");
2740+
}
2741+
accessor "f n" = /* @__PURE__ */ __name(function() {
2742+
}, "f n");
2743+
}
2744+
class Foo3 {
2745+
static {
2746+
__name(this, "Foo3");
2747+
}
2748+
static accessor "f n" = /* @__PURE__ */ __name(function() {
2749+
}, "f n");
2750+
}
2751+
class Foo4 {
2752+
static {
2753+
__name(this, "Foo4");
2754+
}
2755+
#fn = /* @__PURE__ */ __name(function() {
2756+
}, "#fn");
2757+
}
2758+
class Foo5 {
2759+
static {
2760+
__name(this, "Foo5");
2761+
}
2762+
static #fn = /* @__PURE__ */ __name(function() {
2763+
}, "#fn");
2764+
}
2765+
class Foo6 {
2766+
static {
2767+
__name(this, "Foo6");
2768+
}
2769+
accessor #fn = /* @__PURE__ */ __name(function() {
2770+
}, "#fn");
2771+
}
2772+
class Foo7 {
2773+
static {
2774+
__name(this, "Foo7");
2775+
}
2776+
static accessor #fn = /* @__PURE__ */ __name(function() {
2777+
}, "#fn");
2778+
}
2779+
fn = /* @__PURE__ */ __name(function() {
2780+
}, "fn");
2781+
fn ||= /* @__PURE__ */ __name(function() {
2782+
}, "fn");
2783+
fn &&= /* @__PURE__ */ __name(function() {
2784+
}, "fn");
2785+
fn ??= /* @__PURE__ */ __name(function() {
2786+
}, "fn");
2787+
var [fn = /* @__PURE__ */ __name(function() {
2788+
}, "fn")] = [];
2789+
var { fn = /* @__PURE__ */ __name(function() {
2790+
}, "fn") } = {};
2791+
for (var [fn = /* @__PURE__ */ __name(function() {
2792+
}, "fn")] = []; ; )
2793+
;
2794+
for (var { fn = /* @__PURE__ */ __name(function() {
2795+
}, "fn") } = {}; ; )
2796+
;
2797+
for (var [fn = /* @__PURE__ */ __name(function() {
2798+
}, "fn")] in obj)
2799+
;
2800+
for (var { fn = /* @__PURE__ */ __name(function() {
2801+
}, "fn") } in obj)
2802+
;
2803+
for (var [fn = /* @__PURE__ */ __name(function() {
2804+
}, "fn")] of obj)
2805+
;
2806+
for (var { fn = /* @__PURE__ */ __name(function() {
2807+
}, "fn") } of obj)
2808+
;
2809+
function foo([fn2 = /* @__PURE__ */ __name(function() {
2810+
}, "fn")]) {
2811+
}
2812+
__name(foo, "foo");
2813+
function foo({ fn: fn2 = /* @__PURE__ */ __name(function() {
2814+
}, "fn") }) {
2815+
}
2816+
__name(foo, "foo");
2817+
[fn = /* @__PURE__ */ __name(function() {
2818+
}, "fn")] = [];
2819+
({ fn = /* @__PURE__ */ __name(function() {
2820+
}, "fn") } = {});
2821+
27092822
================================================================================
27102823
TestKeepNamesClassStaticName
27112824
---------- /out.js ----------

‎internal/js_parser/js_parser.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11628,20 +11628,18 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class, defaul
1162811628
// We need to explicitly assign the name to the property initializer if it
1162911629
// will be transformed such that it is no longer an inline initializer.
1163011630
nameToKeep := ""
11631-
if private, isPrivate := property.Key.Data.(*js_ast.EPrivateIdentifier); isPrivate && p.privateSymbolNeedsToBeLowered(private) {
11631+
if private, ok := property.Key.Data.(*js_ast.EPrivateIdentifier); ok {
1163211632
nameToKeep = p.symbols[private.Ref.InnerIndex].OriginalName
1163311633

1163411634
// Lowered private methods (both instance and static) are initialized
1163511635
// outside of the class body, so we must rewrite "super" property
1163611636
// accesses inside them. Lowered private instance fields are initialized
1163711637
// inside the constructor where "super" is valid, so those don't need to
1163811638
// be rewritten.
11639-
if property.Kind.IsMethodDefinition() {
11639+
if property.Kind.IsMethodDefinition() && p.privateSymbolNeedsToBeLowered(private) {
1164011640
p.fnOrArrowDataVisit.shouldLowerSuperPropertyAccess = true
1164111641
}
11642-
} else if !property.Kind.IsMethodDefinition() && !property.Flags.Has(js_ast.PropertyIsComputed) &&
11643-
((!property.Flags.Has(js_ast.PropertyIsStatic) && p.options.unsupportedJSFeatures.Has(compat.ClassField)) ||
11644-
(property.Flags.Has(js_ast.PropertyIsStatic) && p.options.unsupportedJSFeatures.Has(compat.ClassStaticField))) {
11642+
} else if !property.Kind.IsMethodDefinition() && !property.Flags.Has(js_ast.PropertyIsComputed) {
1164511643
if str, ok := property.Key.Data.(*js_ast.EString); ok {
1164611644
nameToKeep = helpers.UTF16ToString(str.Value)
1164711645
}
@@ -14131,6 +14129,12 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
1413114129
}
1413214130
}
1413314131

14132+
// Propagate the name to keep from the property into the value
14133+
if str, ok := property.Key.Data.(*js_ast.EString); ok {
14134+
p.nameToKeep = helpers.UTF16ToString(str.Value)
14135+
p.nameToKeepIsFor = property.ValueOrNil.Data
14136+
}
14137+
1413414138
property.ValueOrNil, _ = p.visitExprInOut(property.ValueOrNil, exprIn{assignTarget: in.assignTarget})
1413514139

1413614140
p.fnOnlyDataVisit.innerClassNameRef = oldInnerClassNameRef
@@ -15233,7 +15237,7 @@ func (v *binaryExprVisitor) visitRightAndFinish(p *parser) js_ast.Expr {
1523315237
shouldMangleStringsAsProps: v.in.shouldMangleStringsAsProps,
1523415238
})
1523515239

15236-
case js_ast.BinOpAssign:
15240+
case js_ast.BinOpAssign, js_ast.BinOpLogicalOrAssign, js_ast.BinOpLogicalAndAssign, js_ast.BinOpNullishCoalescingAssign:
1523715241
// Check for a propagated name to keep from the parent context
1523815242
if id, ok := e.Left.Data.(*js_ast.EIdentifier); ok {
1523915243
p.nameToKeep = p.symbols[id.Ref.InnerIndex].OriginalName

0 commit comments

Comments
 (0)