Skip to content

Commit

Permalink
avoid keep names of class static name properties
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Dec 18, 2022
1 parent ff1681e commit 7c249a9
Show file tree
Hide file tree
Showing 6 changed files with 174 additions and 1 deletion.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@

People are currently working on adding a `v` flag to JavaScript regular expresions. You can read more about this flag here: https://v8.dev/features/regexp-v-flag. This release adds support for parsing this flag, so esbuild will now no longer consider regular expression literals with this flag to be a syntax error. If the target is set to something other than `esnext`, esbuild will transform regular expression literals containing this flag into a `new RegExp()` constructor call so the resulting code doesn't have a syntax error. This enables you to provide a polyfill for `RegExp` that implements the `v` flag to get your code to work at run-time. While esbuild doesn't typically adopt proposals until they're already shipping in a real JavaScript run-time, I'm adding it now because a) esbuild's implementation doesn't need to change as the proposal evolves, b) this isn't really new syntax since regular expression literals already have flags, and c) esbuild's implementation is a trivial pass-through anyway.

* Avoid keeping the name of classes with static `name` properties

The `--keep-names` property attempts to preserve the original value of the `name` property for functions and classes even when identifiers are renamed by the minifier or to avoid a name collision. This is currently done by generating code to assign a string to the `name` property on the function or class object. However, this should not be done for classes with a static `name` property since in that case the explicitly-defined `name` property overwrites the automatically-generated class name. With this release, esbuild will now no longer attempt to preserve the `name` property for classes with a static `name` property.

## 0.16.8

* Allow plugins to resolve injected files ([#2754](https://github.com/evanw/esbuild/issues/2754))
Expand Down
35 changes: 35 additions & 0 deletions internal/bundler_tests/bundler_default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4864,6 +4864,41 @@ func TestKeepNamesTreeShaking(t *testing.T) {
})
}

func TestKeepNamesClassStaticName(t *testing.T) {
default_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry.js": `
class A { static foo }
class B { static name }
class C { static name() {} }
class D { static get name() {} }
class E { static set name(x) {} }
class F { static ['name'] = 0 }
let a = class a { static foo }
let b = class b { static name }
let c = class c { static name() {} }
let d = class d { static get name() {} }
let e = class e { static set name(x) {} }
let f = class f { static ['name'] = 0 }
let a2 = class { static foo }
let b2 = class { static name }
let c2 = class { static name() {} }
let d2 = class { static get name() {} }
let e2 = class { static set name(x) {} }
let f2 = class { static ['name'] = 0 }
`,
},
entryPaths: []string{"/entry.js"},
options: config.Options{
Mode: config.ModePassThrough,
AbsOutputFile: "/out.js",
KeepNames: true,
},
})
}

func TestCharFreqIgnoreComments(t *testing.T) {
default_suite.expectBundled(t, bundled{
files: map[string]string{
Expand Down
68 changes: 68 additions & 0 deletions internal/bundler_tests/snapshots/snapshots_default.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1806,6 +1806,74 @@ console.log([
]);
};

================================================================================
TestKeepNamesClassStaticName
---------- /out.js ----------
class A {
static foo;
}
__name(A, "A");
class B {
static name;
}
class C {
static name() {
}
}
class D {
static get name() {
}
}
class E {
static set name(x) {
}
}
class F {
static ["name"] = 0;
}
let a = /* @__PURE__ */ __name(class a3 {
static foo;
}, "a");
let b = class b3 {
static name;
};
let c = class c3 {
static name() {
}
};
let d = class d3 {
static get name() {
}
};
let e = class e3 {
static set name(x) {
}
};
let f = class f3 {
static ["name"] = 0;
};
let a2 = /* @__PURE__ */ __name(class {
static foo;
}, "a2");
let b2 = class {
static name;
};
let c2 = class {
static name() {
}
};
let d2 = class {
static get name() {
}
};
let e2 = class {
static set name(x) {
}
};
let f2 = class {
static ["name"] = 0;
};

================================================================================
TestKeepNamesTreeShaking
---------- /out.js ----------
Expand Down
9 changes: 8 additions & 1 deletion internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -9080,7 +9080,14 @@ func (p *parser) isAnonymousNamedExpr(expr js_ast.Expr) bool {
case *js_ast.EFunction:
return e.Fn.Name == nil
case *js_ast.EClass:
return e.Class.Name == nil
if e.Class.Name == nil {
for _, prop := range e.Class.Properties {
if propertyPreventsKeepNames(&prop) {
return false
}
}
return true
}
}
return false
}
Expand Down
14 changes: 14 additions & 0 deletions internal/js_parser/js_parser_lower.go
Original file line number Diff line number Diff line change
Expand Up @@ -2187,6 +2187,16 @@ func (p *parser) computeClassLoweringInfo(class *js_ast.Class) (result classLowe
return
}

func propertyPreventsKeepNames(prop *js_ast.Property) bool {
// A static property called "name" shadows the automatically-generated name
if prop.Flags.Has(js_ast.PropertyIsStatic) {
if str, ok := prop.Key.Data.(*js_ast.EString); ok && helpers.UTF16EqualsString(str.Value, "name") {
return true
}
}
return false
}

// Lower class fields for environments that don't support them. This either
// takes a statement or an expression.
func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, result visitClassResult) ([]js_ast.Stmt, js_ast.Expr) {
Expand Down Expand Up @@ -2346,6 +2356,10 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, result visitClas
continue
}

if p.options.keepNames && propertyPreventsKeepNames(&prop) {
nameToKeep = ""
}

// Merge parameter decorators with method decorators
if p.options.ts.Parse && prop.Flags.Has(js_ast.PropertyIsMethod) {
if fn, ok := prop.ValueOrNil.Data.(*js_ast.EFunction); ok {
Expand Down
45 changes: 45 additions & 0 deletions scripts/end-to-end-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -2828,6 +2828,51 @@ for (let flags of [[], ['--minify', '--keep-names']]) {
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { let obj = {}; obj['cls'] = class {}; if (obj.cls.name !== '') throw 'fail: ' + obj.cls.name })()`,
}),
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { class Foo { static foo } if (Foo.name !== 'Foo') throw 'fail: ' + Foo.name })()`,
}),
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { class Foo { static name = 123 } if (Foo.name !== 123) throw 'fail: ' + Foo.name })()`,
}),
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { class Foo { static name() { return 123 } } if (Foo.name() !== 123) throw 'fail: ' + Foo.name })()`,
}),
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { class Foo { static get name() { return 123 } } if (Foo.name !== 123) throw 'fail: ' + Foo.name })()`,
}),
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { class Foo { static ['name'] = 123 } if (Foo.name !== 123) throw 'fail: ' + Foo.name })()`,
}),
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { let Foo = class Bar { static foo }; if (Foo.name !== 'Bar') throw 'fail: ' + Foo.name })()`,
}),
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { let Foo = class Bar { static name = 123 }; if (Foo.name !== 123) throw 'fail: ' + Foo.name })()`,
}),
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { let Foo = class Bar { static name() { return 123 } }; if (Foo.name() !== 123) throw 'fail: ' + Foo.name })()`,
}),
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { let Foo = class Bar { static get name() { return 123 } }; if (Foo.name !== 123) throw 'fail: ' + Foo.name })()`,
}),
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { let Foo = class Bar { static ['name'] = 123 }; if (Foo.name !== 123) throw 'fail: ' + Foo.name })()`,
}),
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { let Foo = class { static foo }; if (Foo.name !== 'Foo') throw 'fail: ' + Foo.name })()`,
}),
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { let Foo = class { static name = 123 }; if (Foo.name !== 123) throw 'fail: ' + Foo.name })()`,
}),
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { let Foo = class { static name() { return 123 } }; if (Foo.name() !== 123) throw 'fail: ' + Foo.name })()`,
}),
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { let Foo = class { static get name() { return 123 } }; if (Foo.name !== 123) throw 'fail: ' + Foo.name })()`,
}),
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
'in.js': `(() => { let Foo = class { static ['name'] = 123 }; if (Foo.name !== 123) throw 'fail: ' + Foo.name })()`,
}),

// Methods
test(['in.js', '--outfile=node.js', '--bundle'].concat(flags), {
Expand Down

0 comments on commit 7c249a9

Please sign in to comment.