diff --git a/CHANGELOG.md b/CHANGELOG.md index 91a48ec8fb4..bb419332f2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,41 @@ __publicField(Foo, "y", console.log(3)); ``` +* Use static blocks to implement `--keep-names` on classes ([#2389](https://github.com/evanw/esbuild/issues/2389)) + + This change fixes a bug where the `name` property could previously be incorrect within a class static context when using `--keep-names`. The problem was that the `name` property was being initialized after static blocks were run instead of before. This has been fixed by moving the `name` property initializer into a static block at the top of the class body: + + ```js + // Original code + if (typeof Foo === 'undefined') { + let Foo = class { + static test = this.name + } + console.log(Foo.test) + } + + // Old output (with --keep-names) + if (typeof Foo === "undefined") { + let Foo2 = /* @__PURE__ */ __name(class { + static test = this.name; + }, "Foo"); + console.log(Foo2.test); + } + + // New output (with --keep-names) + if (typeof Foo === "undefined") { + let Foo2 = class { + static { + __name(this, "Foo"); + } + static test = this.name; + }; + console.log(Foo2.test); + } + ``` + + This change was somewhat involved, especially regarding what esbuild considers to be side-effect free. Some unused classes that weren't removed by tree shaking in previous versions of esbuild may now be tree-shaken. One example is classes with static private fields that are transformed by esbuild into code that doesn't use JavaScript's private field syntax. Previously esbuild's tree shaking analysis ran on the class after syntax lowering, but with this release it will run on the class before syntax lowering, meaning it should no longer be confused by class mutations resulting from automatically-generated syntax lowering code. + ## 0.18.1 * Fill in `null` entries in input source maps ([#3144](https://github.com/evanw/esbuild/issues/3144)) diff --git a/internal/bundler_tests/bundler_lower_test.go b/internal/bundler_tests/bundler_lower_test.go index a8b156dade2..8744e76e73c 100644 --- a/internal/bundler_tests/bundler_lower_test.go +++ b/internal/bundler_tests/bundler_lower_test.go @@ -384,10 +384,10 @@ func TestTSLowerPrivateFieldAndMethodAvoidNameCollision2015(t *testing.T) { lower_suite.expectBundled(t, bundled{ files: map[string]string{ "/entry.ts": ` - class WeakMap { + export class WeakMap { #x } - class WeakSet { + export class WeakSet { #y() {} } `, @@ -405,7 +405,7 @@ func TestLowerPrivateGetterSetter2015(t *testing.T) { lower_suite.expectBundled(t, bundled{ files: map[string]string{ "/entry.js": ` - class Foo { + export class Foo { get #foo() { return this.foo } set #bar(val) { this.bar = val } get #prop() { return this.prop } @@ -456,7 +456,7 @@ func TestLowerPrivateGetterSetter2019(t *testing.T) { lower_suite.expectBundled(t, bundled{ files: map[string]string{ "/entry.js": ` - class Foo { + export class Foo { get #foo() { return this.foo } set #bar(val) { this.bar = val } get #prop() { return this.prop } @@ -507,7 +507,7 @@ func TestLowerPrivateGetterSetter2020(t *testing.T) { lower_suite.expectBundled(t, bundled{ files: map[string]string{ "/entry.js": ` - class Foo { + export class Foo { get #foo() { return this.foo } set #bar(val) { this.bar = val } get #prop() { return this.prop } @@ -608,7 +608,7 @@ func TestLowerPrivateMethod2019(t *testing.T) { lower_suite.expectBundled(t, bundled{ files: map[string]string{ "/entry.js": ` - class Foo { + export class Foo { #field #method() {} baseline() { @@ -650,7 +650,7 @@ func TestLowerPrivateMethod2020(t *testing.T) { lower_suite.expectBundled(t, bundled{ files: map[string]string{ "/entry.js": ` - class Foo { + export class Foo { #field #method() {} baseline() { @@ -757,7 +757,7 @@ func TestLowerPrivateMethodWithModifiers2020(t *testing.T) { lower_suite.expectBundled(t, bundled{ files: map[string]string{ "/entry.js": ` - class Foo { + export class Foo { *#g() {} async #a() {} async *#ag() {} diff --git a/internal/bundler_tests/bundler_ts_test.go b/internal/bundler_tests/bundler_ts_test.go index 65ae11cef5d..4f6863debf8 100644 --- a/internal/bundler_tests/bundler_ts_test.go +++ b/internal/bundler_tests/bundler_ts_test.go @@ -1057,7 +1057,7 @@ func TestTSExperimentalDecoratorsKeepNames(t *testing.T) { files: map[string]string{ "/entry.ts": ` @decoratorMustComeAfterName - class Foo {} + export class Foo {} `, "/tsconfig.json": `{ "compilerOptions": { diff --git a/internal/bundler_tests/snapshots/snapshots_default.txt b/internal/bundler_tests/snapshots/snapshots_default.txt index aeb2e48f22c..beea18c0690 100644 --- a/internal/bundler_tests/snapshots/snapshots_default.txt +++ b/internal/bundler_tests/snapshots/snapshots_default.txt @@ -2408,9 +2408,11 @@ console.log([ TestKeepNamesClassStaticName ---------- /out.js ---------- class A { + static { + __name(this, "A"); + } static foo; } -__name(A, "A"); class B { static name; } @@ -2429,9 +2431,12 @@ class E { class F { static ["name"] = 0; } -let a = /* @__PURE__ */ __name(class a3 { +let a = class a3 { + static { + __name(this, "a"); + } static foo; -}, "a"); +}; let b = class b3 { static name; }; @@ -2450,9 +2455,12 @@ let e = class e3 { let f = class f3 { static ["name"] = 0; }; -let a2 = /* @__PURE__ */ __name(class { +let a2 = class { + static { + __name(this, "a2"); + } static foo; -}, "a2"); +}; let b2 = class { static name; }; @@ -2476,6 +2484,9 @@ let f2 = class { TestKeepNamesTreeShaking ---------- /out.js ---------- // entry.js +function fnStmtRemove() { +} +__name(fnStmtRemove, "fnStmtRemove"); function fnStmtKeep() { } __name(fnStmtKeep, "fnStmtKeep"); @@ -2483,12 +2494,12 @@ x = fnStmtKeep; var fnExprKeep = /* @__PURE__ */ __name(function() { }, "keep"); x = fnExprKeep; -var clsStmtKeep = class { -}; -__name(clsStmtKeep, "clsStmtKeep"); +var _clsStmtKeep = class { +}, clsStmtKeep = _clsStmtKeep; +__name(_clsStmtKeep, "clsStmtKeep"); new clsStmtKeep(); -var clsExprKeep = /* @__PURE__ */ __name(class { -}, "keep"); +var _a, clsExprKeep = (_a = class { +}, __name(_a, "keep"), _a); new clsExprKeep(); ================================================================================ @@ -2947,9 +2958,7 @@ var { "_doNotMangleThis": x } = y; ================================================================================ TestMangleNoQuotedPropsMinifySyntax ---------- /out/entry.js ---------- -x._doNotMangleThis, x?._doNotMangleThis, x[y ? "_doNotMangleThis" : z], x?.[y ? "_doNotMangleThis" : z], x[y ? z : "_doNotMangleThis"], x?.[y ? z : "_doNotMangleThis"], class { - _doNotMangleThis = x; -}; +x._doNotMangleThis, x?._doNotMangleThis, x[y ? "_doNotMangleThis" : z], x?.[y ? "_doNotMangleThis" : z], x[y ? z : "_doNotMangleThis"], x?.[y ? z : "_doNotMangleThis"]; var { _doNotMangleThis: x } = y; "_doNotMangleThis" in x, (y ? "_doNotMangleThis" : z) in x, (y ? z : "_doNotMangleThis") in x; @@ -3415,10 +3424,6 @@ foo("_keepThisProperty") in x; ---------- /out/mangle.js ---------- x.a, x?.a, x[y ? "a" : z], x?.[y ? "a" : z], x[y ? z : "a"], x?.[y ? z : "a"], x[y, "a"], x?.[y, "a"], (y, "a") + "", class { - a = x; -}, class { - a = x; -}, class { [(y, "a")] = x; }; var { a: x } = y, { ["a"]: x } = y, { [(z, "a")]: x } = y; diff --git a/internal/bundler_tests/snapshots/snapshots_lower.txt b/internal/bundler_tests/snapshots/snapshots_lower.txt index 5f1ae9661e7..2250d471d6d 100644 --- a/internal/bundler_tests/snapshots/snapshots_lower.txt +++ b/internal/bundler_tests/snapshots/snapshots_lower.txt @@ -1326,6 +1326,9 @@ prop_get = function() { prop_set = function(val) { this.prop = val; }; +export { + Foo +}; ================================================================================ TestLowerPrivateGetterSetter2019 @@ -1385,6 +1388,9 @@ prop_get = function() { prop_set = function(val) { this.prop = val; }; +export { + Foo +}; ================================================================================ TestLowerPrivateGetterSetter2020 @@ -1444,6 +1450,9 @@ prop_get = function() { prop_set = function(val) { this.prop = val; }; +export { + Foo +}; ================================================================================ TestLowerPrivateGetterSetterNext @@ -1538,6 +1547,9 @@ _field = new WeakMap(); _method = new WeakSet(); method_fn = function() { }; +export { + Foo +}; ================================================================================ TestLowerPrivateMethod2020 @@ -1579,6 +1591,9 @@ _field = new WeakMap(); _method = new WeakSet(); method_fn = function() { }; +export { + Foo +}; ================================================================================ TestLowerPrivateMethodNext @@ -1649,6 +1664,9 @@ sag_fn = async function* () { __privateAdd(Foo, _sg); __privateAdd(Foo, _sa); __privateAdd(Foo, _sag); +export { + Foo +}; ================================================================================ TestLowerPrivateSuperES2021 @@ -2596,6 +2614,10 @@ var WeakSet2 = class { _y = new WeakSet(); y_fn = function() { }; +export { + WeakMap2 as WeakMap, + WeakSet2 as WeakSet +}; ================================================================================ TestTSLowerPrivateFieldOptionalChain2015NoBundle diff --git a/internal/bundler_tests/snapshots/snapshots_ts.txt b/internal/bundler_tests/snapshots/snapshots_ts.txt index 4c1a9d36ae4..96990283abb 100644 --- a/internal/bundler_tests/snapshots/snapshots_ts.txt +++ b/internal/bundler_tests/snapshots/snapshots_ts.txt @@ -892,6 +892,9 @@ __name(Foo, "Foo"); Foo = __decorateClass([ decoratorMustComeAfterName ], Foo); +export { + Foo +}; ================================================================================ TestTSExportDefaultTypeIssue316 diff --git a/internal/js_ast/js_ast.go b/internal/js_ast/js_ast.go index 4004e5edf5e..23326d839cd 100644 --- a/internal/js_ast/js_ast.go +++ b/internal/js_ast/js_ast.go @@ -357,6 +357,28 @@ type Class struct { ClassKeyword logger.Range BodyLoc logger.Loc CloseBraceLoc logger.Loc + + // If true, property field initializers cannot be assumed to have no side + // effects. For example: + // + // class Foo { + // static set foo(x) { importantSideEffect(x) } + // } + // class Bar extends Foo { + // foo = 1 + // } + // + // This happens in TypeScript when "useDefineForClassFields" is disabled + // because TypeScript (and esbuild) transforms the above class into this: + // + // class Foo { + // static set foo(x) { importantSideEffect(x); } + // } + // class Bar extends Foo { + // } + // Bar.foo = 1; + // + UseDefineForClassFields bool } type ArrayBinding struct { @@ -436,6 +458,7 @@ func (*EString) isExpr() {} func (*ETemplate) isExpr() {} func (*ERegExp) isExpr() {} func (*EInlinedEnum) isExpr() {} +func (*EAnnotation) isExpr() {} func (*EAwait) isExpr() {} func (*EYield) isExpr() {} func (*EIf) isExpr() {} @@ -545,7 +568,6 @@ const ( NormalCall CallKind = iota DirectEval TargetWasOriginallyPropertyAccess - InternalPublicFieldCall ) type OptionalChain uint8 @@ -791,6 +813,24 @@ type EInlinedEnum struct { Comment string } +type AnnotationFlags uint8 + +const ( + // This is sort of like an IIFE with a "/* @__PURE__ */" comment except it's an + // inline annotation on an expression itself without the nested scope. Sometimes + // we can't easily introduce a new scope (e.g. if the expression uses "await"). + CanBeRemovedIfUnusedFlag AnnotationFlags = 1 << iota +) + +func (flags AnnotationFlags) Has(flag AnnotationFlags) bool { + return (flags & flag) != 0 +} + +type EAnnotation struct { + Value Expr + Flags AnnotationFlags +} + type EAwait struct { Value Expr } @@ -938,10 +978,13 @@ type SLazyExport struct { type SExpr struct { Value Expr - // This is set to true for automatically-generated expressions that should - // not affect tree shaking. For example, calling a function from the runtime - // that doesn't have externally-visible side effects. - DoesNotAffectTreeShaking bool + // This is set to true for automatically-generated expressions that are part + // of class syntax lowering. A single class declaration may end up with many + // generated expressions after it (e.g. class field initializations, a call + // to keep the original value of the "name" property). When this happens we + // can't tell that the class is side-effect free anymore because all of these + // methods mutate the class. We use this annotation for that instead. + IsFromClassThatCanBeRemovedIfUnused bool } type EnumValue struct { diff --git a/internal/js_ast/js_ast_helpers.go b/internal/js_ast/js_ast_helpers.go index 07657c049db..7549a1b064f 100644 --- a/internal/js_ast/js_ast_helpers.go +++ b/internal/js_ast/js_ast_helpers.go @@ -60,6 +60,9 @@ func Not(expr Expr) Expr { // called after the AST has been frozen (i.e. after parsing ends). func MaybeSimplifyNot(expr Expr) (Expr, bool) { switch e := expr.Data.(type) { + case *EAnnotation: + return MaybeSimplifyNot(e.Value) + case *EInlinedEnum: if value, ok := MaybeSimplifyNot(e.Value); ok { return value, true @@ -169,6 +172,9 @@ func MaybeSimplifyEqualityComparison(loc logger.Loc, e *EBinary, unsupportedFeat func IsPrimitiveLiteral(data E) bool { switch e := data.(type) { + case *EAnnotation: + return IsPrimitiveLiteral(e.Value.Data) + case *EInlinedEnum: return IsPrimitiveLiteral(e.Value.Data) @@ -213,6 +219,9 @@ func MergedKnownPrimitiveTypes(a Expr, b Expr) PrimitiveType { // or not. For example, the expression "++x" always returns a primitive. func KnownPrimitiveType(expr E) PrimitiveType { switch e := expr.(type) { + case *EAnnotation: + return KnownPrimitiveType(e.Value.Data) + case *EInlinedEnum: return KnownPrimitiveType(e.Value.Data) @@ -350,6 +359,11 @@ func CanChangeStrictToLoose(a Expr, b Expr) bool { // removed without consequence). func TypeofWithoutSideEffects(data E) (string, bool) { switch e := data.(type) { + case *EAnnotation: + if e.Flags.Has(CanBeRemovedIfUnusedFlag) { + return TypeofWithoutSideEffects(e.Value.Data) + } + case *EInlinedEnum: return TypeofWithoutSideEffects(e.Value.Data) @@ -494,6 +508,11 @@ func ConvertBindingToExpr(binding Binding, wrapIdentifier func(logger.Loc, Ref) // called after the AST has been frozen (i.e. after parsing ends). func SimplifyUnusedExpr(expr Expr, unsupportedFeatures compat.JSFeature, isUnbound func(Ref) bool) Expr { switch e := expr.Data.(type) { + case *EAnnotation: + if e.Flags.Has(CanBeRemovedIfUnusedFlag) { + return Expr{} + } + case *EInlinedEnum: return SimplifyUnusedExpr(e.Value, unsupportedFeatures, isUnbound) @@ -909,6 +928,9 @@ func isInt32OrUint32(data E) bool { func ToNumberWithoutSideEffects(data E) (float64, bool) { switch e := data.(type) { + case *EAnnotation: + return ToNumberWithoutSideEffects(e.Value.Data) + case *EInlinedEnum: return ToNumberWithoutSideEffects(e.Value.Data) @@ -934,6 +956,9 @@ func ToNumberWithoutSideEffects(data E) (float64, bool) { func extractNumericValue(data E) (float64, bool) { switch e := data.(type) { + case *EAnnotation: + return extractNumericValue(e.Value.Data) + case *EInlinedEnum: return extractNumericValue(e.Value.Data) @@ -1565,6 +1590,13 @@ const ( func ToNullOrUndefinedWithSideEffects(data E) (isNullOrUndefined bool, sideEffects SideEffects, ok bool) { switch e := data.(type) { + case *EAnnotation: + isNullOrUndefined, sideEffects, ok = ToNullOrUndefinedWithSideEffects(e.Value.Data) + if e.Flags.Has(CanBeRemovedIfUnusedFlag) { + sideEffects = NoSideEffects + } + return + case *EInlinedEnum: return ToNullOrUndefinedWithSideEffects(e.Value.Data) @@ -1631,8 +1663,15 @@ func ToNullOrUndefinedWithSideEffects(data E) (isNullOrUndefined bool, sideEffec return false, NoSideEffects, false } -func ToBooleanWithSideEffects(data E) (boolean bool, SideEffects SideEffects, ok bool) { +func ToBooleanWithSideEffects(data E) (boolean bool, sideEffects SideEffects, ok bool) { switch e := data.(type) { + case *EAnnotation: + boolean, sideEffects, ok = ToBooleanWithSideEffects(e.Value.Data) + if e.Flags.Has(CanBeRemovedIfUnusedFlag) { + sideEffects = NoSideEffects + } + return + case *EInlinedEnum: return ToBooleanWithSideEffects(e.Value.Data) @@ -1815,15 +1854,22 @@ func StmtsCanBeRemovedIfUnused(stmts []Stmt, flags StmtsCanBeRemovedIfUnusedFlag // its side effects. case *SClass: - if !classCanBeRemovedIfUnused(s.Class, isUnbound) { + if !ClassCanBeRemovedIfUnused(s.Class, isUnbound) { return false } case *SExpr: - if s.DoesNotAffectTreeShaking { - // Expressions marked with this are automatically generated and have - // no side effects by construction. - break + if s.IsFromClassThatCanBeRemovedIfUnused { + // This statement was automatically generated when lowering a class + // that we were able to analyze as having no side effects before + // lowering. So we consider it to be removable. The assumption here + // is that we are seeing at least all of the statements from the + // class lowering operation all at once (although we may possibly be + // seeing even more statements than that). Since we're making a binary + // all-or-nothing decision about the side effects of these statements, + // we can safely consider these to be side-effect free because we + // aren't in danger of partially dropping some of the class setup code. + return true } if !ExprCanBeRemovedIfUnused(s.Value, isUnbound) { @@ -1864,7 +1910,7 @@ func StmtsCanBeRemovedIfUnused(stmts []Stmt, flags StmtsCanBeRemovedIfUnusedFlag // These never have side effects case *SClass: - if !classCanBeRemovedIfUnused(s2.Class, isUnbound) { + if !ClassCanBeRemovedIfUnused(s2.Class, isUnbound) { return false } @@ -1882,7 +1928,17 @@ func StmtsCanBeRemovedIfUnused(stmts []Stmt, flags StmtsCanBeRemovedIfUnusedFlag return true } -func classCanBeRemovedIfUnused(class Class, isUnbound func(Ref) bool) bool { +func ClassCanBeRemovedIfUnused(class Class, isUnbound func(Ref) bool) bool { + // Note: This check is incorrect. Extending a non-constructible object can + // throw an error, which is a side effect: + // + // async function x() {} + // class y extends x {} + // + // But refusing to tree-shake every class with a base class is not a useful + // thing for a bundler to do. So we pretend that this edge case doesn't + // exist. At the time of writing, both Rollup and Terser don't consider this + // to be a side effect either. if class.ExtendsOrNil.Data != nil && !ExprCanBeRemovedIfUnused(class.ExtendsOrNil, isUnbound) { return false } @@ -1907,6 +1963,44 @@ func classCanBeRemovedIfUnused(class Class, isUnbound func(Ref) bool) bool { if property.InitializerOrNil.Data != nil && !ExprCanBeRemovedIfUnused(property.InitializerOrNil, isUnbound) { return false } + + // Legacy TypeScript static class fields are considered to have side + // effects because they use assign semantics, not define semantics, and + // that can trigger getters. For example: + // + // class Foo { + // static set foo(x) { importantSideEffect(x) } + // } + // class Bar extends Foo { + // foo = 1 + // } + // + // This happens in TypeScript when "useDefineForClassFields" is disabled + // because TypeScript (and esbuild) transforms the above class into this: + // + // class Foo { + // static set foo(x) { importantSideEffect(x); } + // } + // class Bar extends Foo { + // } + // Bar.foo = 1; + // + // Note that it's not possible to analyze the base class to determine that + // these assignments are side-effect free. For example: + // + // // Some code that already ran before your code + // Object.defineProperty(Object.prototype, 'foo', { + // set(x) { imporantSideEffect(x) } + // }) + // + // // Your code + // class Foo { + // static foo = 1 + // } + // + if !class.UseDefineForClassFields && !property.Flags.Has(PropertyIsMethod) { + return false + } } } @@ -1915,6 +2009,9 @@ func classCanBeRemovedIfUnused(class Class, isUnbound func(Ref) bool) bool { func ExprCanBeRemovedIfUnused(expr Expr, isUnbound func(Ref) bool) bool { switch e := expr.Data.(type) { + case *EAnnotation: + return e.Flags.Has(CanBeRemovedIfUnusedFlag) + case *EInlinedEnum: return ExprCanBeRemovedIfUnused(e.Value, isUnbound) @@ -1926,7 +2023,7 @@ func ExprCanBeRemovedIfUnused(expr Expr, isUnbound func(Ref) bool) bool { return e.CanBeRemovedIfUnused case *EClass: - return classCanBeRemovedIfUnused(e.Class, isUnbound) + return ClassCanBeRemovedIfUnused(e.Class, isUnbound) case *EIdentifier: if e.MustKeepDueToWithStmt { @@ -2003,13 +2100,6 @@ func ExprCanBeRemovedIfUnused(expr Expr, isUnbound func(Ref) bool) bool { case *ECall: canCallBeRemoved := e.CanBeUnwrappedIfUnused - // Consider calls to our runtime "__publicField" function to be free of - // side effects for the purpose of expression removal. This allows class - // declarations with lowered static fields to be eligible for tree shaking. - if e.Kind == InternalPublicFieldCall { - canCallBeRemoved = true - } - // A call that has been marked "__PURE__" can be removed if all arguments // can be removed. The annotation causes us to ignore the target. if canCallBeRemoved { diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go index 329db3a8a17..f4942299717 100644 --- a/internal/js_parser/js_parser.go +++ b/internal/js_parser/js_parser.go @@ -140,6 +140,15 @@ type parser struct { // binding a name to it in a parent or sibling scope. scopesInOrder []scopeOrder + // These propagate the name from the parent context into an anonymous child + // expression. For example: + // + // let foo = function() {} + // assert.strictEqual(foo.name, 'foo') + // + nameToKeep string + nameToKeepIsFor js_ast.E + // These properties are for the visit pass, which runs after the parse pass. // The visit pass binds identifiers to declared symbols, does constant // folding, substitutes compile-time variable definitions, and lowers certain @@ -6079,6 +6088,32 @@ func (p *parser) parseClass(classKeyword logger.Range, name *js_ast.LocRef, clas BodyLoc: bodyLoc, Properties: properties, CloseBraceLoc: closeBraceLoc, + + // TypeScript has legacy behavior that uses assignment semantics instead of + // define semantics for class fields when "useDefineForClassFields" is enabled + // (in which case TypeScript behaves differently than JavaScript, which is + // arguably "wrong"). + // + // This legacy behavior exists because TypeScript added class fields to + // TypeScript before they were added to JavaScript. They decided to go with + // assignment semantics for whatever reason. Later on TC39 decided to go with + // define semantics for class fields instead. This behaves differently if the + // base class has a setter with the same name. + // + // The value of "useDefineForClassFields" defaults to false when it's not + // specified and the target is earlier than "ES2022" since the class field + // language feature was added in ES2022. However, TypeScript's "target" + // setting currently defaults to "ES3" which unfortunately means that the + // "useDefineForClassFields" setting defaults to false (i.e. to "wrong"). + // + // We default "useDefineForClassFields" to true (i.e. to "correct") instead. + // This is partially because our target defaults to "esnext", and partially + // because this is a legacy behavior that no one should be using anymore. + // Users that want the wrong behavior can either set "useDefineForClassFields" + // to false in "tsconfig.json" explicitly, or set TypeScript's "target" to + // "ES2021" or earlier in their in "tsconfig.json" file. + UseDefineForClassFields: !p.options.ts.Parse || p.options.ts.Config.UseDefineForClassFields == config.True || + (p.options.ts.Config.UseDefineForClassFields == config.Unspecified && p.options.ts.Config.Target != config.TSTargetBelowES2022), } } @@ -8418,7 +8453,6 @@ func (p *parser) mangleStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt prevStmt := result[len(result)-1] if prevS, ok := prevStmt.Data.(*js_ast.SExpr); ok { prevS.Value = js_ast.JoinWithComma(prevS.Value, s.Value) - prevS.DoesNotAffectTreeShaking = prevS.DoesNotAffectTreeShaking && s.DoesNotAffectTreeShaking continue } } @@ -9202,14 +9236,15 @@ func (p *parser) visitBinding(binding js_ast.Binding, opts bindingOpts) { item := &b.Items[i] p.visitBinding(item.Binding, opts) if item.DefaultValueOrNil.Data != nil { - wasAnonymousNamedExpr := p.isAnonymousNamedExpr(item.DefaultValueOrNil) - item.DefaultValueOrNil = p.visitExpr(item.DefaultValueOrNil) - - // Optionally preserve the name - if id, ok := item.Binding.Data.(*js_ast.BIdentifier); ok { - item.DefaultValueOrNil = p.maybeKeepExprSymbolName( - item.DefaultValueOrNil, p.symbols[id.Ref.InnerIndex].OriginalName, wasAnonymousNamedExpr) + // Propagate the name to keep from the binding into the initializer + if p.options.keepNames { + if id, ok := item.Binding.Data.(*js_ast.BIdentifier); ok { + p.nameToKeep = p.symbols[id.Ref.InnerIndex].OriginalName + p.nameToKeepIsFor = item.DefaultValueOrNil.Data + } } + + item.DefaultValueOrNil = p.visitExpr(item.DefaultValueOrNil) } } @@ -9222,14 +9257,15 @@ func (p *parser) visitBinding(binding js_ast.Binding, opts bindingOpts) { } p.visitBinding(property.Value, opts) if property.DefaultValueOrNil.Data != nil { - wasAnonymousNamedExpr := p.isAnonymousNamedExpr(property.DefaultValueOrNil) - property.DefaultValueOrNil = p.visitExpr(property.DefaultValueOrNil) - - // Optionally preserve the name - if id, ok := property.Value.Data.(*js_ast.BIdentifier); ok { - property.DefaultValueOrNil = p.maybeKeepExprSymbolName( - property.DefaultValueOrNil, p.symbols[id.Ref.InnerIndex].OriginalName, wasAnonymousNamedExpr) + // Propagate the name to keep from the binding into the initializer + if p.options.keepNames { + if id, ok := property.Value.Data.(*js_ast.BIdentifier); ok { + p.nameToKeep = p.symbols[id.Ref.InnerIndex].OriginalName + p.nameToKeepIsFor = property.DefaultValueOrNil.Data + } } + + property.DefaultValueOrNil = p.visitExpr(property.DefaultValueOrNil) } b.Properties[i] = property } @@ -9463,32 +9499,6 @@ func (p *parser) mangleIf(stmts []js_ast.Stmt, loc logger.Loc, s *js_ast.SIf) [] return append(stmts, js_ast.Stmt{Loc: loc, Data: s}) } -func (p *parser) isAnonymousNamedExpr(expr js_ast.Expr) bool { - switch e := expr.Data.(type) { - case *js_ast.EArrow: - return true - case *js_ast.EFunction: - return e.Fn.Name == nil - case *js_ast.EClass: - if e.Class.Name == nil { - for _, prop := range e.Class.Properties { - if propertyPreventsKeepNames(&prop) { - return false - } - } - return true - } - } - return false -} - -func (p *parser) maybeKeepExprSymbolName(value js_ast.Expr, name string, wasAnonymousNamedExpr bool) js_ast.Expr { - if p.options.keepNames && wasAnonymousNamedExpr { - return p.keepExprSymbolName(value, name) - } - return value -} - func (p *parser) keepExprSymbolName(value js_ast.Expr, name string) js_ast.Expr { value = p.callRuntime(value.Loc, "__name", []js_ast.Expr{value, {Loc: value.Loc, Data: &js_ast.EString{Value: helpers.StringToUTF16(name)}}, @@ -9499,17 +9509,12 @@ func (p *parser) keepExprSymbolName(value js_ast.Expr, name string) js_ast.Expr return value } -func (p *parser) keepStmtSymbolName(loc logger.Loc, ref js_ast.Ref, name string) js_ast.Stmt { - p.symbols[ref.InnerIndex].Flags |= js_ast.DidKeepName - +func (p *parser) keepStmtSymbolName(loc logger.Loc, expr js_ast.Expr, name string) js_ast.Stmt { return js_ast.Stmt{Loc: loc, Data: &js_ast.SExpr{ Value: p.callRuntime(loc, "__name", []js_ast.Expr{ - {Loc: loc, Data: &js_ast.EIdentifier{Ref: ref}}, + expr, {Loc: loc, Data: &js_ast.EString{Value: helpers.StringToUTF16(name)}}, }), - - // Make sure tree shaking removes this if the function is never used - DoesNotAffectTreeShaking: true, }} } @@ -9640,11 +9645,13 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_ switch s2 := s.Value.Data.(type) { case *js_ast.SExpr: - wasAnonymousNamedExpr := p.isAnonymousNamedExpr(s2.Value) - s2.Value = p.visitExpr(s2.Value) + // Propagate the name to keep from the export into the value + if p.options.keepNames { + p.nameToKeep = "default" + p.nameToKeepIsFor = s2.Value.Data + } - // Optionally preserve the name - s2.Value = p.maybeKeepExprSymbolName(s2.Value, "default", wasAnonymousNamedExpr) + s2.Value = p.visitExpr(s2.Value) // Discard type-only export default statements if p.options.ts.Parse { @@ -9675,16 +9682,27 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_ stmts = append(stmts, stmt) // Optionally preserve the name - if p.options.keepNames && s2.Fn.Name != nil { - stmts = append(stmts, p.keepStmtSymbolName(s2.Fn.Name.Loc, s2.Fn.Name.Ref, name)) + if p.options.keepNames { + p.symbols[s2.Fn.Name.Ref.InnerIndex].Flags |= js_ast.DidKeepName + fn := js_ast.Expr{Loc: s2.Fn.Name.Loc, Data: &js_ast.EIdentifier{Ref: s2.Fn.Name.Ref}} + stmts = append(stmts, p.keepStmtSymbolName(s2.Fn.Name.Loc, fn, name)) } case *js_ast.SClass: - result := p.visitClass(s.Value.Loc, &s2.Class, s.DefaultName.Ref) + result := p.visitClass(s.Value.Loc, &s2.Class, s.DefaultName.Ref, "default") // Lower class field syntax for browsers that don't support it classStmts, _ := p.lowerClass(stmt, js_ast.Expr{}, result) + // Remember if the class was side-effect free before lowering + if result.canBeRemovedIfUnused { + for _, classStmt := range classStmts { + if s2, ok := classStmt.Data.(*js_ast.SExpr); ok { + s2.IsFromClassThatCanBeRemovedIfUnused = true + } + } + } + stmts = append(stmts, classStmts...) default: @@ -9791,22 +9809,22 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_ // Visit the initializer if d.ValueOrNil.Data != nil { - wasAnonymousNamedExpr := p.isAnonymousNamedExpr(d.ValueOrNil) - // Fold numeric constants in the initializer oldShouldFoldTypeScriptConstantExpressions := p.shouldFoldTypeScriptConstantExpressions p.shouldFoldTypeScriptConstantExpressions = p.options.minifySyntax && !p.currentScope.IsAfterConstLocalPrefix + // Propagate the name to keep from the binding into the initializer + if p.options.keepNames { + if id, ok := d.Binding.Data.(*js_ast.BIdentifier); ok { + p.nameToKeep = p.symbols[id.Ref.InnerIndex].OriginalName + p.nameToKeepIsFor = d.ValueOrNil.Data + } + } + d.ValueOrNil = p.visitExpr(d.ValueOrNil) p.shouldFoldTypeScriptConstantExpressions = oldShouldFoldTypeScriptConstantExpressions - // Optionally preserve the name - if id, ok := d.Binding.Data.(*js_ast.BIdentifier); ok { - d.ValueOrNil = p.maybeKeepExprSymbolName( - d.ValueOrNil, p.symbols[id.Ref.InnerIndex].OriginalName, wasAnonymousNamedExpr) - } - // Initializing to undefined is implicit, but be careful to not // accidentally cause a syntax error or behavior change by removing // the value @@ -10274,12 +10292,15 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_ // Optionally preserve the name if p.options.keepNames { - stmts = append(stmts, p.keepStmtSymbolName(s.Fn.Name.Loc, s.Fn.Name.Ref, p.symbols[s.Fn.Name.Ref.InnerIndex].OriginalName)) + symbol := &p.symbols[s.Fn.Name.Ref.InnerIndex] + symbol.Flags |= js_ast.DidKeepName + fn := js_ast.Expr{Loc: s.Fn.Name.Loc, Data: &js_ast.EIdentifier{Ref: s.Fn.Name.Ref}} + stmts = append(stmts, p.keepStmtSymbolName(s.Fn.Name.Loc, fn, symbol.OriginalName)) } return stmts case *js_ast.SClass: - result := p.visitClass(stmt.Loc, &s.Class, js_ast.InvalidRef) + result := p.visitClass(stmt.Loc, &s.Class, js_ast.InvalidRef, "") // Remove the export flag inside a namespace wasExportInsideNamespace := s.IsExport && p.enclosingNamespaceArgRef != nil @@ -10289,6 +10310,16 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_ // Lower class field syntax for browsers that don't support it classStmts, _ := p.lowerClass(stmt, js_ast.Expr{}, result) + + // Remember if the class was side-effect free before lowering + if result.canBeRemovedIfUnused { + for _, classStmt := range classStmts { + if s2, ok := classStmt.Data.(*js_ast.SExpr); ok { + s2.IsFromClassThatCanBeRemovedIfUnused = true + } + } + } + stmts = append(stmts, classStmts...) // Handle exporting this class from a namespace @@ -10834,14 +10865,23 @@ func (p *parser) visitDecorators(decorators []js_ast.Expr, decoratorScope *js_as type visitClassResult struct { shadowRef js_ast.Ref superCtorRef js_ast.Ref + + // If true, the class was determined to be safe to remove if the class is + // never used (i.e. the class definition is side-effect free). This is + // determined after visiting but before lowering since lowering may generate + // class mutations that cannot be automatically analyzed as side-effect free. + canBeRemovedIfUnused bool } -func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class, defaultNameRef js_ast.Ref) (result visitClassResult) { +func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class, defaultNameRef js_ast.Ref, nameToKeep string) (result visitClassResult) { decoratorScope := p.currentScope class.Decorators = p.visitDecorators(class.Decorators, decoratorScope) if class.Name != nil { p.recordDeclaredSymbol(class.Name.Ref) + if p.options.keepNames { + nameToKeep = p.symbols[class.Name.Ref.InnerIndex].OriginalName + } } // Replace "this" with a reference to the class inside static field @@ -10948,7 +10988,6 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class, defaul // A scope is needed for private identifiers p.pushScopeForVisitPass(js_ast.ScopeClassBody, class.BodyLoc) - defer p.popScope() end := 0 @@ -11082,29 +11121,35 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class, defaul } } + // Handle methods if property.ValueOrNil.Data != nil { p.propMethodValue = property.ValueOrNil.Data p.propMethodDecoratorScope = decoratorScope - if nameToKeep != "" { - wasAnonymousNamedExpr := p.isAnonymousNamedExpr(property.ValueOrNil) - property.ValueOrNil = p.maybeKeepExprSymbolName(p.visitExpr(property.ValueOrNil), nameToKeep, wasAnonymousNamedExpr) - } else { - property.ValueOrNil = p.visitExpr(property.ValueOrNil) + + // Propagate the name to keep from the method into the initializer + if p.options.keepNames && nameToKeep != "" { + p.nameToKeep = nameToKeep + p.nameToKeepIsFor = property.ValueOrNil.Data } + + property.ValueOrNil = p.visitExpr(property.ValueOrNil) } + // Handle initialized fields if property.InitializerOrNil.Data != nil { if property.Flags.Has(js_ast.PropertyIsStatic) && classLoweringInfo.lowerAllStaticFields { // Need to lower "this" and "super" since they won't be valid outside the class body p.fnOnlyDataVisit.shouldReplaceThisWithClassNameRef = true p.fnOrArrowDataVisit.shouldLowerSuperPropertyAccess = true } - if nameToKeep != "" { - wasAnonymousNamedExpr := p.isAnonymousNamedExpr(property.InitializerOrNil) - property.InitializerOrNil = p.maybeKeepExprSymbolName(p.visitExpr(property.InitializerOrNil), nameToKeep, wasAnonymousNamedExpr) - } else { - property.InitializerOrNil = p.visitExpr(property.InitializerOrNil) + + // Propagate the name to keep from the field into the initializer + if p.options.keepNames && nameToKeep != "" { + p.nameToKeep = nameToKeep + p.nameToKeepIsFor = property.InitializerOrNil.Data } + + property.InitializerOrNil = p.visitExpr(property.InitializerOrNil) } // Restore "this" so it will take the inherited value in property keys @@ -11122,6 +11167,40 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class, defaul // Finish the filtering operation class.Properties = class.Properties[:end] + // Analyze side effects before adding the name keeping call + result.canBeRemovedIfUnused = js_ast.ClassCanBeRemovedIfUnused(*class, p.isUnbound) + + // Implement name keeping using a static block at the start of the class body + if p.options.keepNames && nameToKeep != "" { + propertyPreventsKeepNames := false + for _, prop := range class.Properties { + // 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") { + propertyPreventsKeepNames = true + break + } + } + } + if !propertyPreventsKeepNames { + var this js_ast.Expr + if classLoweringInfo.lowerAllStaticFields { + p.recordUsage(result.shadowRef) + this = js_ast.Expr{Loc: class.BodyLoc, Data: &js_ast.EIdentifier{Ref: result.shadowRef}} + } else { + this = js_ast.Expr{Loc: class.BodyLoc, Data: js_ast.EThisShared} + } + properties := make([]js_ast.Property, 0, 1+len(class.Properties)) + properties = append(properties, js_ast.Property{ + Kind: js_ast.PropertyClassStaticBlock, + ClassStaticBlock: &js_ast.ClassStaticBlock{Loc: class.BodyLoc, Block: js_ast.SBlock{Stmts: []js_ast.Stmt{ + p.keepStmtSymbolName(class.BodyLoc, this, nameToKeep), + }}}, + }) + class.Properties = append(properties, class.Properties...) + } + } + p.enclosingClassKeyword = oldEnclosingClassKeyword p.superCtorRef = oldSuperCtorRef p.popScope() @@ -11144,6 +11223,15 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class, defaul class.Name = &js_ast.LocRef{Loc: nameScopeLoc, Ref: classNameRef} } + p.popScope() + + // Sanity check that the class lowering info hasn't changed before and after + // visiting. The class transform relies on this because lowering assumes that + // must be able to expect that visiting has done certain things. + if classLoweringInfo != p.computeClassLoweringInfo(class) { + panic("Internal error") + } + return } @@ -13333,15 +13421,17 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO hasSpread = true case *js_ast.EBinary: if in.assignTarget != js_ast.AssignTargetNone && e2.Op == js_ast.BinOpAssign { - wasAnonymousNamedExpr := p.isAnonymousNamedExpr(e2.Right) e2.Left, _ = p.visitExprInOut(e2.Left, exprIn{assignTarget: js_ast.AssignTargetReplace}) - e2.Right = p.visitExpr(e2.Right) - // Optionally preserve the name - if id, ok := e2.Left.Data.(*js_ast.EIdentifier); ok { - e2.Right = p.maybeKeepExprSymbolName( - e2.Right, p.symbols[id.Ref.InnerIndex].OriginalName, wasAnonymousNamedExpr) + // Propagate the name to keep from the binding into the initializer + if p.options.keepNames { + if id, ok := e2.Left.Data.(*js_ast.EIdentifier); ok { + p.nameToKeep = p.symbols[id.Ref.InnerIndex].OriginalName + p.nameToKeepIsFor = e2.Right.Data + } } + + e2.Right = p.visitExpr(e2.Right) } else { item, _ = p.visitExprInOut(item, exprIn{assignTarget: in.assignTarget}) } @@ -13453,16 +13543,15 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO } if property.InitializerOrNil.Data != nil { - wasAnonymousNamedExpr := p.isAnonymousNamedExpr(property.InitializerOrNil) - property.InitializerOrNil = p.visitExpr(property.InitializerOrNil) - - // Optionally preserve the name - if property.ValueOrNil.Data != nil { + // Propagate the name to keep from the binding into the initializer + if p.options.keepNames { if id, ok := property.ValueOrNil.Data.(*js_ast.EIdentifier); ok { - property.InitializerOrNil = p.maybeKeepExprSymbolName( - property.InitializerOrNil, p.symbols[id.Ref.InnerIndex].OriginalName, wasAnonymousNamedExpr) + p.nameToKeep = p.symbols[id.Ref.InnerIndex].OriginalName + p.nameToKeepIsFor = property.InitializerOrNil.Data } } + + property.InitializerOrNil = p.visitExpr(property.InitializerOrNil) } // "{ '123': 4 }" => "{ 123: 4 }" (this is done late to allow "'123'" to be mangled) @@ -14219,6 +14308,12 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO p.maybeMarkKnownGlobalConstructorAsPure(e) case *js_ast.EArrow: + // Check for a propagated name to keep from the parent context + var nameToKeep string + if p.nameToKeepIsFor == e { + nameToKeep = p.nameToKeep + } + asyncArrowNeedsToBeLowered := e.IsAsync && p.options.unsupportedJSFeatures.Has(compat.AsyncAwait) oldFnOrArrowData := p.fnOrArrowDataVisit p.fnOrArrowDataVisit = fnOrArrowDataVisit{ @@ -14265,16 +14360,27 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO // Convert arrow functions to function expressions when lowering if p.options.unsupportedJSFeatures.Has(compat.Arrow) { - return js_ast.Expr{Loc: expr.Loc, Data: &js_ast.EFunction{Fn: js_ast.Fn{ + expr.Data = &js_ast.EFunction{Fn: js_ast.Fn{ Args: e.Args, Body: e.Body, ArgumentsRef: js_ast.InvalidRef, IsAsync: e.IsAsync, HasRestArg: e.HasRestArg, - }}}, exprOut{} + }} + } + + // Optionally preserve the name + if nameToKeep != "" { + expr = p.keepExprSymbolName(expr, nameToKeep) } case *js_ast.EFunction: + // Check for a propagated name to keep from the parent context + var nameToKeep string + if p.nameToKeepIsFor == e { + nameToKeep = p.nameToKeep + } + p.visitFn(&e.Fn, expr.Loc, visitFnOpts{isClassMethod: e == p.propMethodValue}) name := e.Fn.Name @@ -14285,16 +14391,37 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO } // Optionally preserve the name - if p.options.keepNames && name != nil { - expr = p.keepExprSymbolName(expr, p.symbols[name.Ref.InnerIndex].OriginalName) + if p.options.keepNames { + if name != nil { + expr = p.keepExprSymbolName(expr, p.symbols[name.Ref.InnerIndex].OriginalName) + } else if nameToKeep != "" { + expr = p.keepExprSymbolName(expr, nameToKeep) + } } case *js_ast.EClass: - result := p.visitClass(expr.Loc, &e.Class, js_ast.InvalidRef) + // Check for a propagated name to keep from the parent context + var nameToKeep string + if p.nameToKeepIsFor == e { + nameToKeep = p.nameToKeep + } + + result := p.visitClass(expr.Loc, &e.Class, js_ast.InvalidRef, nameToKeep) // Lower class field syntax for browsers that don't support it _, expr = p.lowerClass(js_ast.Stmt{}, expr, result) + // We may be able to determine that a class is side-effect before lowering + // but not after lowering (e.g. due to "--keep-names" mutating the object). + // If that's the case, add a special annotation so this doesn't prevent + // tree-shaking from happening. + if result.canBeRemovedIfUnused { + expr.Data = &js_ast.EAnnotation{ + Value: expr, + Flags: js_ast.CanBeRemovedIfUnusedFlag, + } + } + default: // Note: EPrivateIdentifier and EMangledProperty should have already been handled panic(fmt.Sprintf("Unexpected expression of type %T", expr.Data)) @@ -14353,7 +14480,6 @@ type binaryExprVisitor struct { // "Local variables" passed from "checkAndPrepare" to "visitRightAndFinish" isStmtExpr bool - wasAnonymousNamedExpr bool oldSilenceWarningAboutThisBeingUndefined bool } @@ -14382,7 +14508,6 @@ func (v *binaryExprVisitor) checkAndPrepare(p *parser) js_ast.Expr { } v.isStmtExpr = e == p.stmtExprValue - v.wasAnonymousNamedExpr = p.isAnonymousNamedExpr(e.Right) v.oldSilenceWarningAboutThisBeingUndefined = p.fnOnlyDataVisit.silenceMessageAboutThisBeingUndefined if _, ok := e.Left.Data.(*js_ast.EThis); ok && e.Op == js_ast.BinOpLogicalAnd { @@ -14438,6 +14563,17 @@ func (v *binaryExprVisitor) visitRightAndFinish(p *parser) js_ast.Expr { shouldMangleStringsAsProps: v.in.shouldMangleStringsAsProps, }) + case js_ast.BinOpAssign: + // Check for a propagated name to keep from the parent context + if p.options.keepNames { + if id, ok := e.Left.Data.(*js_ast.EIdentifier); ok { + p.nameToKeep = p.symbols[id.Ref.InnerIndex].OriginalName + p.nameToKeepIsFor = e.Right.Data + } + } + + e.Right = p.visitExpr(e.Right) + default: e.Right = p.visitExpr(e.Right) } @@ -14661,11 +14797,6 @@ func (v *binaryExprVisitor) visitRightAndFinish(p *parser) js_ast.Expr { // All assignment operators below here case js_ast.BinOpAssign: - // Optionally preserve the name - if id, ok := e.Left.Data.(*js_ast.EIdentifier); ok { - e.Right = p.maybeKeepExprSymbolName(e.Right, p.symbols[id.Ref.InnerIndex].OriginalName, v.wasAnonymousNamedExpr) - } - if target, loc, private := p.extractPrivateIndex(e.Left); private != nil { return p.lowerPrivateSet(target, loc, private, e.Right) } diff --git a/internal/js_parser/js_parser_lower.go b/internal/js_parser/js_parser_lower.go index 38d2a36a4a1..a1f6e6dd018 100644 --- a/internal/js_parser/js_parser_lower.go +++ b/internal/js_parser/js_parser_lower.go @@ -1962,43 +1962,23 @@ func (p *parser) captureKeyForObjectRest(originalKey js_ast.Expr) (finalKey js_a } type classLoweringInfo struct { - useDefineForClassFields bool - avoidTDZ bool - lowerAllInstanceFields bool - lowerAllStaticFields bool - shimSuperCtorCalls bool + avoidTDZ bool + lowerAllInstanceFields bool + lowerAllStaticFields bool + shimSuperCtorCalls bool } func (p *parser) computeClassLoweringInfo(class *js_ast.Class) (result classLoweringInfo) { - // TypeScript has legacy behavior that uses assignment semantics instead of - // define semantics for class fields when "useDefineForClassFields" is enabled - // (in which case TypeScript behaves differently than JavaScript, which is - // arguably "wrong"). - // - // This legacy behavior exists because TypeScript added class fields to - // TypeScript before they were added to JavaScript. They decided to go with - // assignment semantics for whatever reason. Later on TC39 decided to go with - // define semantics for class fields instead. This behaves differently if the - // base class has a setter with the same name. - // - // The value of "useDefineForClassFields" defaults to false when it's not - // specified and the target is earlier than "ES2022" since the class field - // language feature was added in ES2022. However, TypeScript's "target" - // setting currently defaults to "ES3" which unfortunately means that the - // "useDefineForClassFields" setting defaults to false (i.e. to "wrong"). - // - // We default "useDefineForClassFields" to true (i.e. to "correct") instead. - // This is partially because our target defaults to "esnext", and partially - // because this is a legacy behavior that no one should be using anymore. - // Users that want the wrong behavior can either set "useDefineForClassFields" - // to false in "tsconfig.json" explicitly, or set TypeScript's "target" to - // "ES2021" or earlier in their in "tsconfig.json" file. - result.useDefineForClassFields = !p.options.ts.Parse || p.options.ts.Config.UseDefineForClassFields == config.True || - (p.options.ts.Config.UseDefineForClassFields == config.Unspecified && p.options.ts.Config.Target != config.TSTargetBelowES2022) - // Safari workaround: Automatically avoid TDZ issues when bundling result.avoidTDZ = p.options.mode == config.ModeBundle && p.currentScope.Parent == nil + // Name keeping for classes is implemented with a static block. So we need to + // lower all static fields if static blocks are unsupported so that the name + // keeping comes first before other static initializers. + if p.options.keepNames && (result.avoidTDZ || p.options.unsupportedJSFeatures.Has(compat.ClassStaticBlocks)) { + result.lowerAllStaticFields = true + } + // Conservatively lower fields of a given type (instance or static) when any // member of that type needs to be lowered. This must be done to preserve // evaluation order. For example: @@ -2164,7 +2144,7 @@ func (p *parser) computeClassLoweringInfo(class *js_ast.Class) (result classLowe // setting for this is enabled. I don't think this matters for private // fields because there's no way for this to call a setter in the base // class, so this isn't done for private fields. - if p.options.ts.Parse && !result.useDefineForClassFields { + if p.options.ts.Parse && !class.UseDefineForClassFields { result.lowerAllStaticFields = true } } else { @@ -2177,7 +2157,7 @@ func (p *parser) computeClassLoweringInfo(class *js_ast.Class) (result classLowe // setting for this is enabled. I don't think this matters for private // fields because there's no way for this to call a setter in the base // class, so this isn't done for private fields. - if p.options.ts.Parse && !result.useDefineForClassFields { + if p.options.ts.Parse && !class.UseDefineForClassFields { result.lowerAllInstanceFields = true } } @@ -2193,16 +2173,6 @@ 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) { @@ -2219,14 +2189,12 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, result visitClas var class *js_ast.Class var classLoc logger.Loc var defaultName js_ast.LocRef - var nameToKeep string if stmt.Data == nil { e, _ := expr.Data.(*js_ast.EClass) class = &e.Class kind = classKindExpr if class.Name != nil { symbol := &p.symbols[class.Name.Ref.InnerIndex] - nameToKeep = symbol.OriginalName // The shadowing name inside the class expression should be the same as // the class expression name itself @@ -2247,18 +2215,12 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, result visitClas } else { kind = classKindStmt } - nameToKeep = p.symbols[class.Name.Ref.InnerIndex].OriginalName } else { s, _ := stmt.Data.(*js_ast.SExportDefault) s2, _ := s.Value.Data.(*js_ast.SClass) class = &s2.Class defaultName = s.DefaultName kind = classKindExportDefaultStmt - if class.Name != nil { - nameToKeep = p.symbols[class.Name.Ref.InnerIndex].OriginalName - } else { - nameToKeep = "default" - } } if stmt.Data == nil { classLoc = expr.Loc @@ -2386,10 +2348,6 @@ 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 { @@ -2425,7 +2383,7 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, result visitClas private, _ := prop.Key.Data.(*js_ast.EPrivateIdentifier) mustLowerPrivate := private != nil && p.privateSymbolNeedsToBeLowered(private) shouldOmitFieldInitializer := p.options.ts.Parse && !prop.Flags.Has(js_ast.PropertyIsMethod) && prop.InitializerOrNil.Data == nil && - !classLoweringInfo.useDefineForClassFields && !mustLowerPrivate + !class.UseDefineForClassFields && !mustLowerPrivate // Class fields must be lowered if the environment doesn't support them mustLowerField := false @@ -2578,7 +2536,7 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, result visitClas init, }) p.recordUsage(ref) - } else if private == nil && classLoweringInfo.useDefineForClassFields { + } else if private == nil && class.UseDefineForClassFields { var args []js_ast.Expr if _, ok := init.Data.(*js_ast.EUndefined); ok { args = []js_ast.Expr{target, prop.Key} @@ -2588,7 +2546,6 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, result visitClas memberExpr = js_ast.Expr{Loc: loc, Data: &js_ast.ECall{ Target: p.importFromRuntime(loc, "__publicField"), Args: args, - Kind: js_ast.InternalPublicFieldCall, }} } else { if key, ok := prop.Key.Data.(*js_ast.EString); ok && !prop.Flags.Has(js_ast.PropertyIsComputed) && !prop.Flags.Has(js_ast.PropertyPreferQuotedKey) { @@ -2787,11 +2744,6 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, result visitClas nameToJoin = nameFunc() } - // Optionally preserve the name - if p.options.keepNames && nameToKeep != "" { - expr = p.keepExprSymbolName(expr, nameToKeep) - } - // Then join "expr" with any other expressions that apply if computedPropertyCache.Data != nil { expr = js_ast.JoinWithComma(expr, computedPropertyCache) @@ -2829,13 +2781,6 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, result visitClas len(staticDecorators) > 0 || len(class.Decorators) > 0) - // Optionally preserve the name - var keepNameStmt js_ast.Stmt - if p.options.keepNames && nameToKeep != "" { - name := nameFunc() - keepNameStmt = p.keepStmtSymbolName(name.Loc, name.Data.(*js_ast.EIdentifier).Ref, nameToKeep) - } - // Pack the class back into a statement, with potentially some extra // statements afterwards var stmts []js_ast.Stmt @@ -2936,9 +2881,6 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, result visitClas p.mergeSymbols(result.shadowRef, class.Name.Ref) } } - if keepNameStmt.Data != nil { - stmts = append(stmts, keepNameStmt) - } // The official TypeScript compiler adds generated code after the class body // in this exact order. Matching this order is important for correctness. diff --git a/internal/js_printer/js_printer.go b/internal/js_printer/js_printer.go index 0d966a1d120..33f769461bf 100644 --- a/internal/js_printer/js_printer.go +++ b/internal/js_printer/js_printer.go @@ -1863,6 +1863,9 @@ func (p *printer) printExpr(expr js_ast.Expr, level js_ast.L, flags printExprFla case *js_ast.EMissing: p.addSourceMapping(expr.Loc) + case *js_ast.EAnnotation: + p.printExpr(e.Value, level, flags) + case *js_ast.EUndefined: p.printUndefined(expr.Loc, level) diff --git a/internal/js_printer/js_printer_test.go b/internal/js_printer/js_printer_test.go index 6ab2cc1fe08..8a37966d04b 100644 --- a/internal/js_printer/js_printer_test.go +++ b/internal/js_printer/js_printer_test.go @@ -847,7 +847,7 @@ func TestMinify(t *testing.T) { expectPrinted(t, "x = '\\n'", "x = \"\\n\";\n") expectPrintedMangle(t, "x = '\\n'", "x = `\n`;\n") expectPrintedMangle(t, "x = {'\\n': 0}", "x = { \"\\n\": 0 };\n") - expectPrintedMangle(t, "(class{'\\n' = 0})", "(class {\n \"\\n\" = 0;\n});\n") + expectPrintedMangle(t, "x = class{'\\n' = 0}", "x = class {\n \"\\n\" = 0;\n};\n") expectPrintedMangle(t, "class Foo{'\\n' = 0}", "class Foo {\n \"\\n\" = 0;\n}\n") // Special identifiers must not be minified diff --git a/scripts/end-to-end-tests.js b/scripts/end-to-end-tests.js index 88a599480aa..c191a0175c4 100644 --- a/scripts/end-to-end-tests.js +++ b/scripts/end-to-end-tests.js @@ -4590,6 +4590,47 @@ for (let flags of [[], ['--target=es6'], ['--bundle'], ['--bundle', '--target=es if (new Foo().foo !== 'foo') throw 'fail' `, }), + + // https://github.com/evanw/esbuild/issues/2389 + test(['in.js', '--outfile=node.js', '--minify', '--keep-names'].concat(flags), { + 'in.js': ` + class DirectlyReferenced { static type = DirectlyReferenced.name } + class ReferencedViaThis { static type = this.name } + class StaticBlockViaThis { static { if (this.name !== 'StaticBlockViaThis') throw 'fail StaticBlockViaThis: ' + this.name } } + class StaticBlockDirectly { static { if (StaticBlockDirectly.name !== 'StaticBlockDirectly') throw 'fail StaticBlockDirectly: ' + StaticBlockDirectly.name } } + if (DirectlyReferenced.type !== 'DirectlyReferenced') throw 'fail DirectlyReferenced: ' + DirectlyReferenced.type + if (ReferencedViaThis.type !== 'ReferencedViaThis') throw 'fail ReferencedViaThis: ' + ReferencedViaThis.type + `, + }), + test(['in.js', '--outfile=node.js', '--minify', '--keep-names'].concat(flags), { + 'in.js': ` + let ReferencedViaThis = class { static type = this.name } + let StaticBlockViaThis = class { static { if (this.name !== 'StaticBlockViaThis') throw 'fail StaticBlockViaThis: ' + this.name } } + if (ReferencedViaThis.type !== 'ReferencedViaThis') throw 'fail ReferencedViaThis: ' + ReferencedViaThis.type + `, + }), + test(['in.js', '--outfile=node.js', '--keep-names', '--format=esm'].concat(flags), { + 'in.js': ` + // Cause the names in the inner scope to be renamed + if ( + typeof DirectlyReferenced !== 'undefined' || + typeof ReferencedViaThis !== 'undefined' || + typeof StaticBlockViaThis !== 'undefined' || + typeof StaticBlockDirectly !== 'undefined' + ) { + throw 'fail' + } + function innerScope() { + class DirectlyReferenced { static type = DirectlyReferenced.name } + class ReferencedViaThis { static type = this.name } + class StaticBlockViaThis { static { if (this.name !== 'StaticBlockViaThis') throw 'fail StaticBlockViaThis: ' + this.name } } + class StaticBlockDirectly { static { if (StaticBlockDirectly.name !== 'StaticBlockDirectly') throw 'fail StaticBlockDirectly: ' + StaticBlockDirectly.name } } + if (DirectlyReferenced.type !== 'DirectlyReferenced') throw 'fail DirectlyReferenced: ' + DirectlyReferenced.type + if (ReferencedViaThis.type !== 'ReferencedViaThis') throw 'fail ReferencedViaThis: ' + ReferencedViaThis.type + } + innerScope() + `, + }), ) } diff --git a/scripts/uglify-tests.js b/scripts/uglify-tests.js index 947101a185d..fbd43afaf38 100644 --- a/scripts/uglify-tests.js +++ b/scripts/uglify-tests.js @@ -259,6 +259,15 @@ async function test_case(esbuild, test, basename) { // Error difference 'dead-code.js: dead_code_2_should_warn', + + // These tests fail because esbuild assumes that declaring a class that + // extends a base class has no side effects. That's not true because + // extending a non-constructible object throws an error. But refusing to + // tree-shake every class with a base class is not a useful thing for a + // bundler to do. So we pretend that this edge case doesn't exist. + 'classes.js: issue_4722_1', + 'classes.js: issue_4722_2', + 'classes.js: issue_4722_3', ].indexOf(`${basename}: ${test.name}`) >= 0 if (!sandbox.same_stdout(test.expect_stdout, actual)) {