From 7bd06b09c89dee45ea207a3ccdce2bd87e63a076 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Thu, 15 Jun 2023 23:19:21 -0400 Subject: [PATCH] fix function tree-shaking with `--keep-names` --- .../bundler_tests/snapshots/snapshots_default.txt | 3 --- internal/js_ast/js_ast.go | 2 +- internal/js_ast/js_ast_helpers.go | 8 ++++---- internal/js_parser/js_parser.go | 13 +++++++------ 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/internal/bundler_tests/snapshots/snapshots_default.txt b/internal/bundler_tests/snapshots/snapshots_default.txt index beea18c0690..e1697babd25 100644 --- a/internal/bundler_tests/snapshots/snapshots_default.txt +++ b/internal/bundler_tests/snapshots/snapshots_default.txt @@ -2484,9 +2484,6 @@ let f2 = class { TestKeepNamesTreeShaking ---------- /out.js ---------- // entry.js -function fnStmtRemove() { -} -__name(fnStmtRemove, "fnStmtRemove"); function fnStmtKeep() { } __name(fnStmtKeep, "fnStmtKeep"); diff --git a/internal/js_ast/js_ast.go b/internal/js_ast/js_ast.go index 23326d839cd..8ccacd476a9 100644 --- a/internal/js_ast/js_ast.go +++ b/internal/js_ast/js_ast.go @@ -984,7 +984,7 @@ type SExpr struct { // 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 + IsFromClassOrFnThatCanBeRemovedIfUnused bool } type EnumValue struct { diff --git a/internal/js_ast/js_ast_helpers.go b/internal/js_ast/js_ast_helpers.go index 7549a1b064f..7d26b3054dc 100644 --- a/internal/js_ast/js_ast_helpers.go +++ b/internal/js_ast/js_ast_helpers.go @@ -1859,11 +1859,11 @@ func StmtsCanBeRemovedIfUnused(stmts []Stmt, flags StmtsCanBeRemovedIfUnusedFlag } case *SExpr: - if s.IsFromClassThatCanBeRemovedIfUnused { + if s.IsFromClassOrFnThatCanBeRemovedIfUnused { // 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 + // or function 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, diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go index 37bedfcca96..e823655ff49 100644 --- a/internal/js_parser/js_parser.go +++ b/internal/js_parser/js_parser.go @@ -9509,12 +9509,13 @@ func (p *parser) keepExprSymbolName(value js_ast.Expr, name string) js_ast.Expr return value } -func (p *parser) keepStmtSymbolName(loc logger.Loc, expr js_ast.Expr, name string) js_ast.Stmt { +func (p *parser) keepClassOrFnSymbolName(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{ expr, {Loc: loc, Data: &js_ast.EString{Value: helpers.StringToUTF16(name)}}, }), + IsFromClassOrFnThatCanBeRemovedIfUnused: true, }} } @@ -9685,7 +9686,7 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_ 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)) + stmts = append(stmts, p.keepClassOrFnSymbolName(s2.Fn.Name.Loc, fn, name)) } case *js_ast.SClass: @@ -9698,7 +9699,7 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_ if result.canBeRemovedIfUnused { for _, classStmt := range classStmts { if s2, ok := classStmt.Data.(*js_ast.SExpr); ok { - s2.IsFromClassThatCanBeRemovedIfUnused = true + s2.IsFromClassOrFnThatCanBeRemovedIfUnused = true } } } @@ -10295,7 +10296,7 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_ 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)) + stmts = append(stmts, p.keepClassOrFnSymbolName(s.Fn.Name.Loc, fn, symbol.OriginalName)) } return stmts @@ -10315,7 +10316,7 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_ if result.canBeRemovedIfUnused { for _, classStmt := range classStmts { if s2, ok := classStmt.Data.(*js_ast.SExpr); ok { - s2.IsFromClassThatCanBeRemovedIfUnused = true + s2.IsFromClassOrFnThatCanBeRemovedIfUnused = true } } } @@ -11178,7 +11179,7 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class, defaul 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), + p.keepClassOrFnSymbolName(class.BodyLoc, this, nameToKeep), }}}, }) class.Properties = append(properties, class.Properties...)