From 046f82f6ec8c8947a817ce65ffde08ee314db225 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Thu, 7 Apr 2022 00:30:41 -0400 Subject: [PATCH] Revert "Drop superfluous `__name()` calls (#2062)" This reverts commit 730df4fbaa08e64ad618aaa2b5d30cc9f4e1e6a3. --- internal/bundler/bundler_default_test.go | 71 ------------ .../bundler/snapshots/snapshots_default.txt | 91 +--------------- internal/bundler/snapshots/snapshots_ts.txt | 1 + internal/js_ast/js_ast.go | 7 +- internal/js_parser/js_parser.go | 24 +++-- internal/js_printer/js_printer.go | 101 ++++-------------- 6 files changed, 49 insertions(+), 246 deletions(-) diff --git a/internal/bundler/bundler_default_test.go b/internal/bundler/bundler_default_test.go index 312b7095f68..a0246560a02 100644 --- a/internal/bundler/bundler_default_test.go +++ b/internal/bundler/bundler_default_test.go @@ -4358,77 +4358,6 @@ func TestDefineThis(t *testing.T) { }) } -func TestKeepNamesWithNecessaryHelperFunctionCalls(t *testing.T) { - default_suite.expectBundled(t, bundled{ - files: map[string]string{ - "/entry.js": ` - import { - functionStmt as functionStmt1, - functionExpr as functionExpr1, - classStmt as classStmt1, - classExpr as classExpr1, - functionAnonExpr as functionAnonExpr1, - classAnonExpr as classAnonExpr1, - classAnonExprLowered as classAnonExprLowered1, - } from './copy1' - - import { - functionStmt as functionStmt2, - functionExpr as functionExpr2, - classStmt as classStmt2, - classExpr as classExpr2, - functionAnonExpr as functionAnonExpr2, - classAnonExpr as classAnonExpr2, - classAnonExprLowered as classAnonExprLowered2, - } from './copy2' - - console.log([ - functionStmt1, functionStmt2, - functionExpr1, functionExpr2, - classStmt1, classStmt2, - classExpr1, classExpr2, - functionAnonExpr1, functionAnonExpr2, - classAnonExpr1, classAnonExpr2, - classAnonExprLowered1, classAnonExprLowered2, - ]) - `, - - "/copy1.js": ` - export function functionStmt() { return 'copy1' } - export class classStmt { foo = 'copy1' } - export let functionExpr = function fn() { return 'copy1' } - export let classExpr = class cls { foo = 'copy1' } - export let functionAnonExpr = function() { return 'copy1' } - export let classAnonExpr = class { foo = 'copy1' } - export let classAnonExprLowered = class { static foo = 'copy2' } - - class classStmtSideEffect { static [copy1]() {} } - let classExprSideEffect = class clsSideEffect { static [copy1]() {} } - `, - - "/copy2.js": ` - export function functionStmt() { return 'copy2' } - export class classStmt { foo = 'copy2' } - export let functionExpr = function fn() { return 'copy2' } - export let classExpr = class cls { foo = 'copy2' } - export let functionAnonExpr = function() { return 'copy2' } - export let classAnonExpr = class { foo = 'copy2' } - export let classAnonExprLowered = class { static foo = 'copy2' } - - class classStmtSideEffect { static [copy2]() {} } - let classExprSideEffect = class clsSideEffect { static [copy2]() {} } - `, - }, - entryPaths: []string{"/entry.js"}, - options: config.Options{ - Mode: config.ModeBundle, - AbsOutputFile: "/out.js", - KeepNames: true, - UnsupportedJSFeatures: compat.ClassStaticField, - }, - }) -} - func TestKeepNamesTreeShaking(t *testing.T) { default_suite.expectBundled(t, bundled{ files: map[string]string{ diff --git a/internal/bundler/snapshots/snapshots_default.txt b/internal/bundler/snapshots/snapshots_default.txt index 8b89d7a90e7..83ecfa0b6be 100644 --- a/internal/bundler/snapshots/snapshots_default.txt +++ b/internal/bundler/snapshots/snapshots_default.txt @@ -1401,103 +1401,19 @@ TestKeepNamesTreeShaking // entry.js function fnStmtKeep() { } +__name(fnStmtKeep, "fnStmtKeep"); x = fnStmtKeep; var fnExprKeep = /* @__PURE__ */ __name(function() { }, "keep"); x = fnExprKeep; var clsStmtKeep = class { }; +__name(clsStmtKeep, "clsStmtKeep"); new clsStmtKeep(); var clsExprKeep = /* @__PURE__ */ __name(class { }, "keep"); new clsExprKeep(); -================================================================================ -TestKeepNamesWithNecessaryHelperFunctionCalls ----------- /out.js ---------- -// copy1.js -function functionStmt() { - return "copy1"; -} -var classStmt = class { - foo = "copy1"; -}; -var functionExpr = function fn() { - return "copy1"; -}; -var classExpr = class cls { - foo = "copy1"; -}; -var functionAnonExpr = function() { - return "copy1"; -}; -var classAnonExpr = class { - foo = "copy1"; -}; -var _a; -var classAnonExprLowered = /* @__PURE__ */ __name((_a = class { -}, __publicField(_a, "foo", "copy2"), _a), "classAnonExprLowered"); -var classStmtSideEffect = class { - static [copy1]() { - } -}; -var classExprSideEffect = class clsSideEffect { - static [copy1]() { - } -}; - -// copy2.js -function functionStmt2() { - return "copy2"; -} -__name(functionStmt2, "functionStmt"); -var classStmt2 = class { - foo = "copy2"; -}; -__name(classStmt2, "classStmt"); -var functionExpr2 = /* @__PURE__ */ __name(function fn2() { - return "copy2"; -}, "fn"); -var classExpr2 = /* @__PURE__ */ __name(class cls2 { - foo = "copy2"; -}, "cls"); -var functionAnonExpr2 = /* @__PURE__ */ __name(function() { - return "copy2"; -}, "functionAnonExpr"); -var classAnonExpr2 = /* @__PURE__ */ __name(class { - foo = "copy2"; -}, "classAnonExpr"); -var _a2; -var classAnonExprLowered2 = /* @__PURE__ */ __name((_a2 = class { -}, __publicField(_a2, "foo", "copy2"), _a2), "classAnonExprLowered"); -var classStmtSideEffect2 = class { - static [copy2]() { - } -}; -__name(classStmtSideEffect2, "classStmtSideEffect"); -var classExprSideEffect2 = /* @__PURE__ */ __name(class clsSideEffect2 { - static [copy2]() { - } -}, "clsSideEffect"); - -// entry.js -console.log([ - functionStmt, - functionStmt2, - functionExpr, - functionExpr2, - classStmt, - classStmt2, - classExpr, - classExpr2, - functionAnonExpr, - functionAnonExpr2, - classAnonExpr, - classAnonExpr2, - classAnonExprLowered, - classAnonExprLowered2 -]); - ================================================================================ TestLegalCommentsAvoidSlashTagEndOfFile ---------- /out/entry.js ---------- @@ -2894,11 +2810,12 @@ export function outer() { let inner = function() { return Math.random(); }; + __name(inner, "inner"); const x = inner(); console.log(x); } } -outer(); +__name(outer, "outer"), outer(); ================================================================================ TestSwitchScopeNoBundle diff --git a/internal/bundler/snapshots/snapshots_ts.txt b/internal/bundler/snapshots/snapshots_ts.txt index 0f73f778b60..730140d607d 100644 --- a/internal/bundler/snapshots/snapshots_ts.txt +++ b/internal/bundler/snapshots/snapshots_ts.txt @@ -1153,6 +1153,7 @@ TestTypeScriptDecoratorsKeepNames // entry.ts var Foo = class { }; +__name(Foo, "Foo"); Foo = __decorateClass([ decoratorMustComeAfterName ], Foo); diff --git a/internal/js_ast/js_ast.go b/internal/js_ast/js_ast.go index daa00a8dfbf..f0efbd97991 100644 --- a/internal/js_ast/js_ast.go +++ b/internal/js_ast/js_ast.go @@ -516,8 +516,6 @@ type ECall struct { // call itself is removed due to this annotation, the arguments must remain // if they have side effects. CanBeUnwrappedIfUnused bool - - IsKeepName bool } func (a *ECall) HasSameFlagsAs(b *ECall) bool { @@ -850,6 +848,11 @@ 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 } type EnumValue struct { diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go index 70a2c40c920..550a2f90e6f 100644 --- a/internal/js_parser/js_parser.go +++ b/internal/js_parser/js_parser.go @@ -7617,6 +7617,7 @@ 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 } } @@ -8852,20 +8853,21 @@ func (p *parser) keepExprSymbolName(value js_ast.Expr, name string) js_ast.Expr // Make sure tree shaking removes this if the function is never used value.Data.(*js_ast.ECall).CanBeUnwrappedIfUnused = true - value.Data.(*js_ast.ECall).IsKeepName = true 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 - call := p.callRuntime(loc, "__name", []js_ast.Expr{ - {Loc: loc, Data: &js_ast.EIdentifier{Ref: ref}}, - {Loc: loc, Data: &js_ast.EString{Value: helpers.StringToUTF16(name)}}, - }) - call.Data.(*js_ast.ECall).IsKeepName = true + 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}}, + {Loc: loc, Data: &js_ast.EString{Value: helpers.StringToUTF16(name)}}, + }), - return js_ast.Stmt{Loc: loc, Data: &js_ast.SExpr{Value: call}} + // Make sure tree shaking removes this if the function is never used + DoesNotAffectTreeShaking: true, + }} } func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_ast.Stmt { @@ -14459,6 +14461,12 @@ func (p *parser) stmtsCanBeRemovedIfUnused(stmts []js_ast.Stmt) bool { } case *js_ast.SExpr: + if s.DoesNotAffectTreeShaking { + // Expressions marked with this are automatically generated and have + // no side effects by construction. + break + } + if !p.exprCanBeRemovedIfUnused(s.Value) { return false } @@ -14637,7 +14645,7 @@ func (p *parser) exprCanBeRemovedIfUnused(expr js_ast.Expr) bool { return true case *js_ast.ECall: - canCallBeRemoved := e.CanBeUnwrappedIfUnused || e.IsKeepName + 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 diff --git a/internal/js_printer/js_printer.go b/internal/js_printer/js_printer.go index 018908b6478..63f2fb1aefa 100644 --- a/internal/js_printer/js_printer.go +++ b/internal/js_printer/js_printer.go @@ -441,8 +441,6 @@ type printer struct { callTarget js_ast.E extractedLegalComments map[string]bool js []byte - keepNamesAssignTarget js_ast.E - keepNamesRef js_ast.Ref options Options builder sourcemap.ChunkBuilder stmtStart int @@ -1400,10 +1398,6 @@ func (p *printer) simplifyUnusedExpr(expr js_ast.Expr) js_ast.Expr { } case *js_ast.ECall: - if p.isCallExprSuperfluous(e) { - return js_ast.Expr{Loc: expr.Loc} - } - var symbolFlags js_ast.SymbolFlags switch target := e.Target.Data.(type) { case *js_ast.EIdentifier: @@ -1804,11 +1798,6 @@ func (p *printer) printExpr(expr js_ast.Expr, level js_ast.L, flags printExprFla } } - if p.isCallExprSuperfluous(e) { - p.printExpr(e.Args[0], level, flags) - return - } - wrap := level >= js_ast.LNew || (flags&forbidCall) != 0 var targetFlags printExprFlags if e.OptionalChain == js_ast.OptionalChainNone { @@ -2574,49 +2563,6 @@ func (p *printer) printExpr(expr js_ast.Expr, level js_ast.L, flags printExprFla } } -func (p *printer) isCallExprSuperfluous(value *js_ast.ECall) bool { - if !value.IsKeepName { - return false - } - - var ref *js_ast.Ref - isAnonymousFnOrClass := false - switch e := value.Args[0].Data.(type) { - case *js_ast.EIdentifier: - // "__name(foo, 'foo')" - ref = &e.Ref - - case *js_ast.EFunction: - // "__name(function foo() {}, 'foo')" - if e.Fn.Name != nil { - ref = &e.Fn.Name.Ref - } else { - isAnonymousFnOrClass = true - } - - case *js_ast.EClass: - // "__name(class foo {}, 'foo')" - if e.Class.Name != nil { - ref = &e.Class.Name.Ref - } else { - isAnonymousFnOrClass = true - } - } - - // If this is an anonymous function or class expression but the assign target - // has a name binding, use the name from the name binding instead: - // - // let foo = __name(function() {}, "foo"); - // let bar = __name(class {}, "bar"); - // - if ref == nil && p.keepNamesAssignTarget == value && isAnonymousFnOrClass { - ref = &p.keepNamesRef - } - - keptName := value.Args[1].Data.(*js_ast.EString).Value - return ref != nil && helpers.UTF16EqualsString(keptName, p.renamer.NameForSymbol(*ref)) -} - func (p *printer) isUnboundEvalIdentifier(value js_ast.Expr) bool { if id, ok := value.Data.(*js_ast.EIdentifier); ok { // Using the original name here is ok since unbound symbols are not renamed @@ -2840,11 +2786,6 @@ func (p *printer) printDecls(keyword string, decls []js_ast.Decl, flags printExp p.printBinding(decl.Binding) if decl.ValueOrNil.Data != nil { - if id, ok := decl.Binding.Data.(*js_ast.BIdentifier); ok { - p.keepNamesAssignTarget = decl.ValueOrNil.Data - p.keepNamesRef = id.Ref - } - p.printSpace() p.print("=") p.printSpace() @@ -3444,15 +3385,17 @@ func (p *printer) printStmt(stmt js_ast.Stmt, flags printStmtFlags) { update := s.UpdateOrNil // Omit calls to empty functions from the output completely - if expr, ok := init.Data.(*js_ast.SExpr); ok { - if value := p.simplifyUnusedExpr(expr.Value); value.Data == nil { - init.Data = nil - } else if value.Data != expr.Value.Data { - init.Data = &js_ast.SExpr{Value: value} + if p.options.MinifySyntax { + if expr, ok := init.Data.(*js_ast.SExpr); ok { + if value := p.simplifyUnusedExpr(expr.Value); value.Data == nil { + init.Data = nil + } else if value.Data != expr.Value.Data { + init.Data = &js_ast.SExpr{Value: value} + } + } + if update.Data != nil { + update = p.simplifyUnusedExpr(update) } - } - if update.Data != nil { - update = p.simplifyUnusedExpr(update) } p.printIndent() @@ -3666,18 +3609,20 @@ func (p *printer) printStmt(stmt js_ast.Stmt, flags printStmtFlags) { value := s.Value // Omit calls to empty functions from the output completely - value = p.simplifyUnusedExpr(value) - if value.Data == nil { - // If this statement is not in a block, then we still need to emit something - if (flags & canOmitStatement) == 0 { - // "if (x) empty();" => "if (x) ;" - p.printIndent() - p.print(";") - p.printNewline() - } else { - // "if (x) { empty(); }" => "if (x) {}" + if p.options.MinifySyntax { + value = p.simplifyUnusedExpr(value) + if value.Data == nil { + // If this statement is not in a block, then we still need to emit something + if (flags & canOmitStatement) == 0 { + // "if (x) empty();" => "if (x) ;" + p.printIndent() + p.print(";") + p.printNewline() + } else { + // "if (x) { empty(); }" => "if (x) {}" + } + break } - break } p.printIndent()