From 00fa0107ed4fd2b363e2fa3cd2160aeb2f787fc3 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Sun, 19 Nov 2023 16:50:50 -0500 Subject: [PATCH] tree shaking: handle destructuring of an array --- internal/bundler_tests/bundler_dce_test.go | 28 ++++++++++++ .../bundler_tests/bundler_default_test.go | 3 +- .../bundler_tests/snapshots/snapshots_dce.txt | 9 ++++ .../snapshots/snapshots_default.txt | 35 +++++++-------- internal/js_ast/js_ast_helpers.go | 45 +++++++++++++++++-- 5 files changed, 97 insertions(+), 23 deletions(-) diff --git a/internal/bundler_tests/bundler_dce_test.go b/internal/bundler_tests/bundler_dce_test.go index ea09225d434..14295e4520f 100644 --- a/internal/bundler_tests/bundler_dce_test.go +++ b/internal/bundler_tests/bundler_dce_test.go @@ -4335,6 +4335,34 @@ func TestDCEOfIIFE(t *testing.T) { }) } +func TestDCEOfDestructuring(t *testing.T) { + dce_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.js": ` + // Identifier bindings + var remove1 + var remove2 = null + var KEEP1 = x + + // Array patterns + var [remove3] = [] + var [remove4, ...remove5] = [...[1, 2], 3] + var [, , remove6] = [, , 3] + var [KEEP2] = [x] + var [KEEP3] = [...{}] + + // Object patterns (not handled right now) + var { KEEP4 } = {} + `, + }, + entryPaths: []string{"/entry.js"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputDir: "/out", + }, + }) +} + func TestDCEOfDecorators(t *testing.T) { dce_suite.expectBundled(t, bundled{ files: map[string]string{ diff --git a/internal/bundler_tests/bundler_default_test.go b/internal/bundler_tests/bundler_default_test.go index 1c3fcd54402..8726aa1232d 100644 --- a/internal/bundler_tests/bundler_default_test.go +++ b/internal/bundler_tests/bundler_default_test.go @@ -8187,9 +8187,8 @@ func TestCommentPreservation(t *testing.T) { }, entryPaths: []string{"/entry.js"}, options: config.Options{ - Mode: config.ModeBundle, + Mode: config.ModePassThrough, AbsOutputDir: "/out", - OutputFormat: config.FormatCommonJS, ExternalSettings: config.ExternalSettings{ PreResolve: config.ExternalMatchers{ Exact: map[string]bool{"foo": true}, diff --git a/internal/bundler_tests/snapshots/snapshots_dce.txt b/internal/bundler_tests/snapshots/snapshots_dce.txt index 80d5435f26e..d81bd9d52bd 100644 --- a/internal/bundler_tests/snapshots/snapshots_dce.txt +++ b/internal/bundler_tests/snapshots/snapshots_dce.txt @@ -438,6 +438,15 @@ var StaticAccessor = class { static accessor accessor; }; +================================================================================ +TestDCEOfDestructuring +---------- /out/entry.js ---------- +// entry.js +var KEEP1 = x; +var [KEEP2] = [x]; +var [KEEP3] = [...{}]; +var { KEEP4 } = {}; + ================================================================================ TestDCEOfExperimentalDecorators ---------- /out/keep-these.js ---------- diff --git a/internal/bundler_tests/snapshots/snapshots_default.txt b/internal/bundler_tests/snapshots/snapshots_default.txt index f27c81138b3..8a61633709f 100644 --- a/internal/bundler_tests/snapshots/snapshots_default.txt +++ b/internal/bundler_tests/snapshots/snapshots_default.txt @@ -300,7 +300,6 @@ export { ================================================================================ TestCommentPreservation ---------- /out/entry.js ---------- -// entry.js console.log( import( /* before */ @@ -393,54 +392,54 @@ console.log( /* after */ ) ); -var [ +let [ /* foo */ ] = [ /* bar */ ]; -var [ +let [ // foo ] = [ // bar ]; -var [ +let [ /*before*/ ...s ] = [ /*before*/ ...s ]; -var [.../*before*/ +let [.../*before*/ s2] = [.../*before*/ s2]; -var { +let { /* foo */ } = { /* bar */ }; -var { +let { // foo } = { // bar }; -var { +let { /*before*/ ...s3 } = { /*before*/ ...s3 }; -var { .../*before*/ +let { .../*before*/ s4 } = { .../*before*/ s4 }; -var [ +let [ /* before */ x ] = [ /* before */ x ]; -var [ +let [ /* before */ x2 /* after */ @@ -449,7 +448,7 @@ var [ x2 /* after */ ]; -var [ +let [ // before x3 // after @@ -458,14 +457,14 @@ var [ x3 // after ]; -var { +let { /* before */ y } = { /* before */ y }; -var { +let { /* before */ y2 /* after */ @@ -474,7 +473,7 @@ var { y2 /* after */ }; -var { +let { // before y3 // after @@ -483,21 +482,21 @@ var { y3 // after }; -var { +let { /* before */ [y4]: y4 } = { /* before */ [y4]: y4 }; -var { [ +let { [ /* before */ y5 ]: y5 } = { [ /* before */ y5 ]: y5 }; -var { [ +let { [ y6 /* after */ ]: y6 } = { [ diff --git a/internal/js_ast/js_ast_helpers.go b/internal/js_ast/js_ast_helpers.go index 17608743d91..8a27d7e83b2 100644 --- a/internal/js_ast/js_ast_helpers.go +++ b/internal/js_ast/js_ast_helpers.go @@ -2035,14 +2035,46 @@ func StmtsCanBeRemovedIfUnused(stmts []Stmt, flags StmtsCanBeRemovedIfUnusedFlag } for _, decl := range s.Decls { - if _, ok := decl.Binding.Data.(*BIdentifier); !ok { + // Check that the bindings are side-effect free + switch binding := decl.Binding.Data.(type) { + case *BIdentifier: + // An identifier binding has no side effects + + case *BArray: + // Destructuring the initializer has no side effects if the + // initializer is an array, since we assume the iterator is then + // the built-in side-effect free array iterator. + if _, ok := decl.ValueOrNil.Data.(*EArray); ok { + for _, item := range binding.Items { + if item.DefaultValueOrNil.Data != nil && !ExprCanBeRemovedIfUnused(item.DefaultValueOrNil, isUnbound) { + return false + } + + switch item.Binding.Data.(type) { + case *BIdentifier, *BMissing: + // Right now we only handle an array pattern with identifier + // bindings or with empty holes (i.e. "missing" elements) + default: + return false + } + } + break + } + return false + + default: + // Consider anything else to potentially have side effects return false } + + // Check that the initializer is side-effect free if decl.ValueOrNil.Data != nil { if !ExprCanBeRemovedIfUnused(decl.ValueOrNil, isUnbound) { return false - } else if s.Kind.IsUsing() { - // "using" declarations are only side-effect free if they are initialized to null or undefined + } + + // "using" declarations are only side-effect free if they are initialized to null or undefined + if s.Kind.IsUsing() { if t := KnownPrimitiveType(decl.ValueOrNil.Data); t != PrimitiveNull && t != PrimitiveUndefined { return false } @@ -2261,6 +2293,13 @@ func ExprCanBeRemovedIfUnused(expr Expr, isUnbound func(ast.Ref) bool) bool { case *EArray: for _, item := range e.Items { + if spread, ok := item.Data.(*ESpread); ok { + if _, ok := spread.Value.Data.(*EArray); ok { + // Spread of an inline array such as "[...[x]]" is side-effect free + item = spread.Value + } + } + if !ExprCanBeRemovedIfUnused(item, isUnbound) { return false }