Skip to content

Commit

Permalink
fix class fields with "keep names"
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Mar 6, 2021
1 parent be77ee6 commit 8ca9f46
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 8 deletions.
17 changes: 17 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,22 @@
# Changelog

## Unreleased

* Fix `--keep-names` for lowered class fields

Anonymous function expressions used in class field initializers are automatically assigned a `.name` property in JavaScript:

```js
class Example {
field1 = () => {}
static field2 = () => {}
}
assert(new Example().field1.name === 'field1')
assert(Example.field2.name === 'field2')
```

This usually doesn't need special handling from esbuild's `--keep-names` option because esbuild doesn't modify field names, so the `.name` property will not change. However, esbuild will relocate the field initializer if the configured language target doesn't support class fields (e.g. `--target=es6`). In that case the `.name` property wasn't preserved even when `--keep-names` was specified. This bug has been fixed. Now the `.name` property should be preserved in this case as long as you enable `--keep-names`.
## 0.8.56
* Fix a discrepancy with esbuild's `tsconfig.json` implementation ([#913](https://github.com/evanw/esbuild/issues/913))
Expand Down
23 changes: 17 additions & 6 deletions internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -9027,11 +9027,23 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class) js_ast
p.fnOnlyDataVisit.isThisNested = true
p.fnOnlyDataVisit.thisClassStaticRef = nil

// 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 isPrivate && p.isPrivateUnsupported(private) {
nameToKeep = p.symbols[private.Ref.InnerIndex].OriginalName
} else if !property.IsMethod && !property.IsComputed &&
((!property.IsStatic && p.options.unsupportedJSFeatures.Has(compat.ClassField)) ||
(property.IsStatic && p.options.unsupportedJSFeatures.Has(compat.ClassStaticField))) {
if str, ok := property.Key.Data.(*js_ast.EString); ok {
nameToKeep = js_lexer.UTF16ToString(str.Value)
}
}

if property.Value != nil {
if isPrivate && p.isPrivateUnsupported(private) {
if nameToKeep != "" {
wasAnonymousNamedExpr := p.isAnonymousNamedExpr(*property.Value)
*property.Value = p.maybeKeepExprSymbolName(p.visitExpr(*property.Value),
p.symbols[private.Ref.InnerIndex].OriginalName, wasAnonymousNamedExpr)
*property.Value = p.maybeKeepExprSymbolName(p.visitExpr(*property.Value), nameToKeep, wasAnonymousNamedExpr)
} else {
*property.Value = p.visitExpr(*property.Value)
}
Expand All @@ -9042,10 +9054,9 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class) js_ast
// Replace "this" with the class name inside static property initializers
p.fnOnlyDataVisit.thisClassStaticRef = &shadowRef
}
if isPrivate && p.isPrivateUnsupported(private) {
if nameToKeep != "" {
wasAnonymousNamedExpr := p.isAnonymousNamedExpr(*property.Initializer)
*property.Initializer = p.maybeKeepExprSymbolName(p.visitExpr(*property.Initializer),
p.symbols[private.Ref.InnerIndex].OriginalName, wasAnonymousNamedExpr)
*property.Initializer = p.maybeKeepExprSymbolName(p.visitExpr(*property.Initializer), nameToKeep, wasAnonymousNamedExpr)
} else {
*property.Initializer = p.visitExpr(*property.Initializer)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/js_parser/js_parser_lower.go
Original file line number Diff line number Diff line change
Expand Up @@ -1697,7 +1697,7 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, shadowRef js_ast

// Class fields must be lowered if the environment doesn't support them
mustLowerField := !prop.IsMethod &&
(!prop.IsStatic && p.options.unsupportedJSFeatures.Has(compat.ClassField) ||
((!prop.IsStatic && p.options.unsupportedJSFeatures.Has(compat.ClassField)) ||
(prop.IsStatic && p.options.unsupportedJSFeatures.Has(compat.ClassStaticField)))

// Be conservative and always lower static fields when we're doing TDZ-
Expand Down
30 changes: 29 additions & 1 deletion scripts/end-to-end-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -1405,6 +1405,26 @@
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { let obj = {}; obj['cls'] = class {}; if (obj.cls.name !== '') throw 'fail' })()`,
}),

// Methods
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { let obj = { foo() {} }; if (obj.foo.name !== 'foo') throw 'fail' })()`,
}),
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { let obj = { foo: () => {} }; if (obj.foo.name !== 'foo') throw 'fail' })()`,
}),
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { class Foo { foo() {} }; if (new Foo().foo.name !== 'foo') throw 'fail' })()`,
}),
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { class Foo { static foo() {} }; if (Foo.foo.name !== 'foo') throw 'fail' })()`,
}),
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { let Foo = class { foo() {} }; if (new Foo().foo.name !== 'foo') throw 'fail' })()`,
}),
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { let Foo = class { static foo() {} }; if (Foo.foo.name !== 'foo') throw 'fail' })()`,
}),
)
}
tests.push(
Expand Down Expand Up @@ -1454,7 +1474,15 @@
'in.js': `export default (class {})`,
}),

// Lowered classes
// Class fields
test(['in.js', '--outfile=node.js', '--minify', '--keep-names', '--bundle', '--target=es6'], {
'in.js': `(() => { class Foo { foo = () => {} } if (new Foo().foo.name !== 'foo') throw 'fail' })()`,
}),
test(['in.js', '--outfile=node.js', '--minify', '--keep-names', '--bundle', '--target=es6'], {
'in.js': `(() => { class Foo { static foo = () => {} } if (Foo.foo.name !== 'foo') throw 'fail' })()`,
}),

// Private methods
test(['in.js', '--outfile=node.js', '--minify', '--keep-names', '--bundle', '--target=es6'], {
'in.js': `(() => { class foo { a() { return this.#b } #b() {} } if (foo.name !== 'foo') throw 'fail' })()`,
}),
Expand Down

0 comments on commit 8ca9f46

Please sign in to comment.