Skip to content

Commit

Permalink
fix some --keep-names edge cases
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed May 5, 2024
1 parent d124695 commit d5e343b
Show file tree
Hide file tree
Showing 4 changed files with 196 additions and 6 deletions.
25 changes: 25 additions & 0 deletions CHANGELOG.md
View file
Expand Up @@ -146,6 +146,31 @@
}
```

* Fix some `--keep-names` edge cases

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:

```js
var foo = function() {}
var bar = class {}
console.log(foo.name, bar.name)
```

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).

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:

```js
var obj = { foo: function() {} }
class Foo0 { foo = function() {} }
class Foo1 { static foo = function() {} }
class Foo2 { accessor foo = function() {} }
class Foo3 { static accessor foo = function() {} }
foo ||= function() {}
foo &&= function() {}
foo ??= function() {}
```
## 0.20.2
* Support TypeScript experimental decorators on `abstract` class fields ([#3684](https://github.com/evanw/esbuild/issues/3684))
Expand Down
48 changes: 48 additions & 0 deletions internal/bundler_tests/bundler_default_test.go
View file
Expand Up @@ -5496,6 +5496,54 @@ NOTE: The expression "b['c']" has been configured to be replaced with a constant
})
}

func TestKeepNamesAllForms(t *testing.T) {
default_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry.js": `
// Initializers
function fn() {}
function foo(fn = function() {}) {}
var fn = function() {};
var obj = { "f n": function() {} };
class Foo0 { "f n" = function() {} }
class Foo1 { static "f n" = function() {} }
class Foo2 { accessor "f n" = function() {} }
class Foo3 { static accessor "f n" = function() {} }
class Foo4 { #fn = function() {} }
class Foo5 { static #fn = function() {} }
class Foo6 { accessor #fn = function() {} }
class Foo7 { static accessor #fn = function() {} }
// Assignments
fn = function() {};
fn ||= function() {};
fn &&= function() {};
fn ??= function() {};
// Destructuring
var [fn = function() {}] = [];
var { fn = function() {} } = {};
for (var [fn = function() {}] = []; ; ) ;
for (var { fn = function() {} } = {}; ; ) ;
for (var [fn = function() {}] in obj) ;
for (var { fn = function() {} } in obj) ;
for (var [fn = function() {}] of obj) ;
for (var { fn = function() {} } of obj) ;
function foo([fn = function() {}]) {}
function foo({ fn = function() {} }) {}
[fn = function() {}] = [];
({ fn = function() {} } = {});
`,
},
entryPaths: []string{"/entry.js"},
options: config.Options{
Mode: config.ModePassThrough,
AbsOutputFile: "/out.js",
KeepNames: true,
},
})
}

func TestKeepNamesTreeShaking(t *testing.T) {
default_suite.expectBundled(t, bundled{
files: map[string]string{
Expand Down
113 changes: 113 additions & 0 deletions internal/bundler_tests/snapshots/snapshots_default.txt
View file
Expand Up @@ -2706,6 +2706,119 @@ console.log([
]);
};

================================================================================
TestKeepNamesAllForms
---------- /out.js ----------
function fn() {
}
__name(fn, "fn");
function foo(fn2 = function() {
}) {
}
__name(foo, "foo");
var fn = /* @__PURE__ */ __name(function() {
}, "fn");
var obj = { "f n": /* @__PURE__ */ __name(function() {
}, "f n") };
class Foo0 {
static {
__name(this, "Foo0");
}
"f n" = /* @__PURE__ */ __name(function() {
}, "f n");
}
class Foo1 {
static {
__name(this, "Foo1");
}
static "f n" = /* @__PURE__ */ __name(function() {
}, "f n");
}
class Foo2 {
static {
__name(this, "Foo2");
}
accessor "f n" = /* @__PURE__ */ __name(function() {
}, "f n");
}
class Foo3 {
static {
__name(this, "Foo3");
}
static accessor "f n" = /* @__PURE__ */ __name(function() {
}, "f n");
}
class Foo4 {
static {
__name(this, "Foo4");
}
#fn = /* @__PURE__ */ __name(function() {
}, "#fn");
}
class Foo5 {
static {
__name(this, "Foo5");
}
static #fn = /* @__PURE__ */ __name(function() {
}, "#fn");
}
class Foo6 {
static {
__name(this, "Foo6");
}
accessor #fn = /* @__PURE__ */ __name(function() {
}, "#fn");
}
class Foo7 {
static {
__name(this, "Foo7");
}
static accessor #fn = /* @__PURE__ */ __name(function() {
}, "#fn");
}
fn = /* @__PURE__ */ __name(function() {
}, "fn");
fn ||= /* @__PURE__ */ __name(function() {
}, "fn");
fn &&= /* @__PURE__ */ __name(function() {
}, "fn");
fn ??= /* @__PURE__ */ __name(function() {
}, "fn");
var [fn = /* @__PURE__ */ __name(function() {
}, "fn")] = [];
var { fn = /* @__PURE__ */ __name(function() {
}, "fn") } = {};
for (var [fn = /* @__PURE__ */ __name(function() {
}, "fn")] = []; ; )
;
for (var { fn = /* @__PURE__ */ __name(function() {
}, "fn") } = {}; ; )
;
for (var [fn = /* @__PURE__ */ __name(function() {
}, "fn")] in obj)
;
for (var { fn = /* @__PURE__ */ __name(function() {
}, "fn") } in obj)
;
for (var [fn = /* @__PURE__ */ __name(function() {
}, "fn")] of obj)
;
for (var { fn = /* @__PURE__ */ __name(function() {
}, "fn") } of obj)
;
function foo([fn2 = /* @__PURE__ */ __name(function() {
}, "fn")]) {
}
__name(foo, "foo");
function foo({ fn: fn2 = /* @__PURE__ */ __name(function() {
}, "fn") }) {
}
__name(foo, "foo");
[fn = /* @__PURE__ */ __name(function() {
}, "fn")] = [];
({ fn = /* @__PURE__ */ __name(function() {
}, "fn") } = {});

================================================================================
TestKeepNamesClassStaticName
---------- /out.js ----------
Expand Down
16 changes: 10 additions & 6 deletions internal/js_parser/js_parser.go
View file
Expand Up @@ -11628,20 +11628,18 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class, defaul
// We need to explicitly assign the name to the property initializer if it
// will be transformed such that it is no longer an inline initializer.
nameToKeep := ""
if private, isPrivate := property.Key.Data.(*js_ast.EPrivateIdentifier); isPrivate && p.privateSymbolNeedsToBeLowered(private) {
if private, ok := property.Key.Data.(*js_ast.EPrivateIdentifier); ok {
nameToKeep = p.symbols[private.Ref.InnerIndex].OriginalName

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

// Propagate the name to keep from the property into the value
if str, ok := property.Key.Data.(*js_ast.EString); ok {
p.nameToKeep = helpers.UTF16ToString(str.Value)
p.nameToKeepIsFor = property.ValueOrNil.Data
}

property.ValueOrNil, _ = p.visitExprInOut(property.ValueOrNil, exprIn{assignTarget: in.assignTarget})

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

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

0 comments on commit d5e343b

Please sign in to comment.