Skip to content

Commit

Permalink
fix #175: lowered class static field side effects
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Dec 29, 2021
1 parent 54cfb1e commit ed2076a
Show file tree
Hide file tree
Showing 4 changed files with 206 additions and 11 deletions.
26 changes: 26 additions & 0 deletions CHANGELOG.md
View file
Expand Up @@ -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:
Expand Down
104 changes: 104 additions & 0 deletions internal/bundler/bundler_dce_test.go
View file
Expand Up @@ -3,6 +3,7 @@ package bundler
import (
"testing"

"github.com/evanw/esbuild/internal/compat"
"github.com/evanw/esbuild/internal/config"
)

Expand Down Expand Up @@ -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,
},
})
}
49 changes: 49 additions & 0 deletions internal/bundler/snapshots/snapshots_dce.txt
View file
Expand Up @@ -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 ----------
Expand Down
38 changes: 27 additions & 11 deletions internal/js_parser/js_parser.go
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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}}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit ed2076a

Please sign in to comment.