diff --git a/CHANGELOG.md b/CHANGELOG.md index f418ae4c31e..bcdab697ad5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,32 @@ ## Unreleased +* Enable tree shaking of classes with lowered static fields ([#175](https://github.com/evanw/esbuild/issues/175)) + + If the configured target environment doesn't support static class fields, they are converted into a call to esbuild's `__publicField` function instead. However, esbuild's tree-shaking pass treated this call as a side effect, which meant that all classes with static fields were ineligible for tree shaking. This release fixes the problem by explicitly ignoring calls to the `__publicField` function during tree shaking side-effect determination. Tree shaking is now enabled for these classes: + + ```js + // Original code + class Foo { static foo = 'foo' } + class Bar { static bar = 'bar' } + new Bar() + + // Old output (with --tree-shaking=true --target=es6) + class Foo { + } + __publicField(Foo, "foo", "foo"); + class Bar { + } + __publicField(Bar, "bar", "bar"); + new Bar(); + + // New output (with --tree-shaking=true --target=es6) + class Bar { + } + __publicField(Bar, "bar", "bar"); + new Bar(); + ``` + * Treat `--define:foo=undefined` as an undefined literal instead of an identifier ([#1407](https://github.com/evanw/esbuild/issues/1407)) References to the global variable `undefined` are automatically replaced with the literal value for undefined, which appears as `void 0` when printed. This allows for additional optimizations such as collapsing `undefined ?? bar` into just `bar`. However, this substitution was not done for values specified via `--define:`. As a result, esbuild could potentially miss out on certain optimizations in these cases. With this release, it's now possible to use `--define:` to substitute something with an undefined literal: diff --git a/internal/bundler/bundler_dce_test.go b/internal/bundler/bundler_dce_test.go index c9d6f84648d..282efadf327 100644 --- a/internal/bundler/bundler_dce_test.go +++ b/internal/bundler/bundler_dce_test.go @@ -3,6 +3,7 @@ package bundler import ( "testing" + "github.com/evanw/esbuild/internal/compat" "github.com/evanw/esbuild/internal/config" ) @@ -1981,3 +1982,106 @@ func TestDCETemplateLiteral(t *testing.T) { }, }) } + +// Calls to the runtime "__publicField" function are not considered side effects +func TestTreeShakingLoweredClassStaticField(t *testing.T) { + dce_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.js": ` + class REMOVE_ME { + static x = 'x' + static y = 'y' + static z = 'z' + } + function REMOVE_ME_TOO() { + new REMOVE_ME() + } + class KeepMe1 { + static x = 'x' + static y = sideEffects() + static z = 'z' + } + class KeepMe2 { + static x = 'x' + static y = 'y' + static z = 'z' + } + new KeepMe2() + `, + }, + entryPaths: []string{"/entry.js"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputDir: "/out", + UnsupportedJSFeatures: compat.ClassField, + }, + }) +} + +func TestTreeShakingLoweredClassStaticFieldMinified(t *testing.T) { + dce_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.js": ` + class REMOVE_ME { + static x = 'x' + static y = 'y' + static z = 'z' + } + function REMOVE_ME_TOO() { + new REMOVE_ME() + } + class KeepMe1 { + static x = 'x' + static y = sideEffects() + static z = 'z' + } + class KeepMe2 { + static x = 'x' + static y = 'y' + static z = 'z' + } + new KeepMe2() + `, + }, + entryPaths: []string{"/entry.js"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputDir: "/out", + UnsupportedJSFeatures: compat.ClassField, + MangleSyntax: true, + }, + }) +} + +// Assignments are considered side effects +func TestTreeShakingLoweredClassStaticFieldAssignment(t *testing.T) { + dce_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.ts": ` + class KeepMe1 { + static x = 'x' + static y = 'y' + static z = 'z' + } + class KeepMe2 { + static x = 'x' + static y = sideEffects() + static z = 'z' + } + class KeepMe3 { + static x = 'x' + static y = 'y' + static z = 'z' + } + new KeepMe3() + `, + }, + entryPaths: []string{"/entry.js"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputDir: "/out", + UnsupportedJSFeatures: compat.ClassField, + UseDefineForClassFields: config.False, + }, + }) +} diff --git a/internal/bundler/snapshots/snapshots_dce.txt b/internal/bundler/snapshots/snapshots_dce.txt index 8fd859be5ef..c55aab3924b 100644 --- a/internal/bundler/snapshots/snapshots_dce.txt +++ b/internal/bundler/snapshots/snapshots_dce.txt @@ -859,6 +859,55 @@ var init_cjs = __esm({ init_lib(); console.log(keep1(), (init_cjs(), __toCommonJS(cjs_exports))); +================================================================================ +TestTreeShakingLoweredClassStaticField +---------- /out/entry.js ---------- +// entry.js +var KeepMe1 = class { +}; +__publicField(KeepMe1, "x", "x"); +__publicField(KeepMe1, "y", sideEffects()); +__publicField(KeepMe1, "z", "z"); +var KeepMe2 = class { +}; +__publicField(KeepMe2, "x", "x"); +__publicField(KeepMe2, "y", "y"); +__publicField(KeepMe2, "z", "z"); +new KeepMe2(); + +================================================================================ +TestTreeShakingLoweredClassStaticFieldAssignment +---------- /out/entry.js ---------- +// entry.ts +var KeepMe1 = class { +}; +KeepMe1.x = "x"; +KeepMe1.y = "y"; +KeepMe1.z = "z"; +var KeepMe2 = class { +}; +KeepMe2.x = "x"; +KeepMe2.y = sideEffects(); +KeepMe2.z = "z"; +var KeepMe3 = class { +}; +KeepMe3.x = "x"; +KeepMe3.y = "y"; +KeepMe3.z = "z"; +new KeepMe3(); + +================================================================================ +TestTreeShakingLoweredClassStaticFieldMinified +---------- /out/entry.js ---------- +// entry.js +var KeepMe1 = class { +}; +__publicField(KeepMe1, "x", "x"), __publicField(KeepMe1, "y", sideEffects()), __publicField(KeepMe1, "z", "z"); +var KeepMe2 = class { +}; +__publicField(KeepMe2, "x", "x"), __publicField(KeepMe2, "y", "y"), __publicField(KeepMe2, "z", "z"); +new KeepMe2(); + ================================================================================ TestTreeShakingNoBundleCJS ---------- /out.js ---------- diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go index 9756bfef260..14ce6799960 100644 --- a/internal/js_parser/js_parser.go +++ b/internal/js_parser/js_parser.go @@ -68,6 +68,7 @@ type parser struct { importSymbolPropertyUses map[js_ast.Ref]map[string]js_ast.SymbolUse declaredSymbols []js_ast.DeclaredSymbol runtimeImports map[string]js_ast.Ref + runtimePublicFieldImport js_ast.Ref duplicateCaseChecker duplicateCaseChecker unrepresentableIdentifiers map[string]bool legacyOctalLiterals map[js_ast.E]logger.Range @@ -1758,6 +1759,9 @@ func (p *parser) importFromRuntime(loc logger.Loc, name string) js_ast.Expr { ref = p.newSymbol(js_ast.SymbolOther, name) p.moduleScope.Generated = append(p.moduleScope.Generated, ref) p.runtimeImports[name] = ref + if name == "__publicField" { + p.runtimePublicFieldImport = ref + } } p.recordUsage(ref) return js_ast.Expr{Loc: loc, Data: &js_ast.EIdentifier{Ref: ref}} @@ -14311,9 +14315,20 @@ func (p *parser) exprCanBeRemovedIfUnused(expr js_ast.Expr) bool { return true case *js_ast.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 !canCallBeRemoved { + if id, ok := e.Target.Data.(*js_ast.EIdentifier); ok && id.Ref == p.runtimePublicFieldImport { + 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 e.CanBeUnwrappedIfUnused { + if canCallBeRemoved { for _, arg := range e.Args { if !p.exprCanBeRemovedIfUnused(arg) { return false @@ -14732,16 +14747,17 @@ func newParser(log logger.Log, source logger.Source, lexer js_lexer.Lexer, optio } p := &parser{ - log: log, - source: source, - tracker: logger.MakeLineColumnTracker(&source), - lexer: lexer, - allowIn: true, - options: *options, - runtimeImports: make(map[string]js_ast.Ref), - promiseRef: js_ast.InvalidRef, - afterArrowBodyLoc: logger.Loc{Start: -1}, - importMetaRef: js_ast.InvalidRef, + log: log, + source: source, + tracker: logger.MakeLineColumnTracker(&source), + lexer: lexer, + allowIn: true, + options: *options, + runtimeImports: make(map[string]js_ast.Ref), + promiseRef: js_ast.InvalidRef, + afterArrowBodyLoc: logger.Loc{Start: -1}, + importMetaRef: js_ast.InvalidRef, + runtimePublicFieldImport: js_ast.InvalidRef, // For lowering private methods weakMapRef: js_ast.InvalidRef,