Skip to content

Commit

Permalink
no side effects for "typeof x != undefined && x"
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Sep 22, 2021
1 parent 7d15c6d commit 9e5e767
Show file tree
Hide file tree
Showing 4 changed files with 223 additions and 3 deletions.
22 changes: 22 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,27 @@
# Changelog

## Unreleased

* Treat `x` guarded by `typeof x !== 'undefined'` as side-effect free

This is a small tree-shaking (i.e. dead code removal) improvement. Global identifier references are considered to potentially have side effects since they will throw a reference error if the global identifier isn't defined, and code with side effects cannot be removed as dead code. However, there's a somewhat-common case where the identifier reference is guarded by a `typeof` check to check that it's defined before accessing it. With this release, code that does this will now be considered to have no side effects which allows it to be tree-shaken:

```js
// Original code
var __foo = typeof foo !== 'undefined' && foo;
var __bar = typeof bar !== 'undefined' && bar;
console.log(__bar);

// Old output (with --bundle, which enables tree-shaking)
var __foo = typeof foo !== 'undefined' && foo;
var __bar = typeof bar !== 'undefined' && bar;
console.log(__bar);

// New output (with --bundle, which enables tree-shaking)
var __bar = typeof bar !== 'undefined' && bar;
console.log(__bar);
```

## 0.12.29

* Fix compilation of abstract class fields in TypeScript ([#1623](https://github.com/evanw/esbuild/issues/1623))
Expand Down
104 changes: 104 additions & 0 deletions internal/bundler/bundler_dce_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1723,3 +1723,107 @@ func TestDCETypeOfEqualsStringMangle(t *testing.T) {
},
})
}

func TestDCETypeOfEqualsStringGuardCondition(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry.js": `
// Everything here should be removed as dead code due to tree shaking
var REMOVE_1 = typeof x !== 'undefined' ? x : null
var REMOVE_1 = typeof x != 'undefined' ? x : null
var REMOVE_1 = typeof x === 'undefined' ? null : x
var REMOVE_1 = typeof x == 'undefined' ? null : x
var REMOVE_1 = typeof x !== 'undefined' && x
var REMOVE_1 = typeof x != 'undefined' && x
var REMOVE_1 = typeof x === 'undefined' || x
var REMOVE_1 = typeof x == 'undefined' || x
var REMOVE_1 = 'undefined' !== typeof x ? x : null
var REMOVE_1 = 'undefined' != typeof x ? x : null
var REMOVE_1 = 'undefined' === typeof x ? null : x
var REMOVE_1 = 'undefined' == typeof x ? null : x
var REMOVE_1 = 'undefined' !== typeof x && x
var REMOVE_1 = 'undefined' != typeof x && x
var REMOVE_1 = 'undefined' === typeof x || x
var REMOVE_1 = 'undefined' == typeof x || x
// Everything here should be removed as dead code due to tree shaking
var REMOVE_2 = typeof x === 'object' ? x : null
var REMOVE_2 = typeof x == 'object' ? x : null
var REMOVE_2 = typeof x !== 'object' ? null : x
var REMOVE_2 = typeof x != 'object' ? null : x
var REMOVE_2 = typeof x === 'object' && x
var REMOVE_2 = typeof x == 'object' && x
var REMOVE_2 = typeof x !== 'object' || x
var REMOVE_2 = typeof x != 'object' || x
var REMOVE_2 = 'object' === typeof x ? x : null
var REMOVE_2 = 'object' == typeof x ? x : null
var REMOVE_2 = 'object' !== typeof x ? null : x
var REMOVE_2 = 'object' != typeof x ? null : x
var REMOVE_2 = 'object' === typeof x && x
var REMOVE_2 = 'object' == typeof x && x
var REMOVE_2 = 'object' !== typeof x || x
var REMOVE_2 = 'object' != typeof x || x
// Everything here should be kept as live code because it has side effects
var keep_1 = typeof x !== 'object' ? x : null
var keep_1 = typeof x != 'object' ? x : null
var keep_1 = typeof x === 'object' ? null : x
var keep_1 = typeof x == 'object' ? null : x
var keep_1 = typeof x !== 'object' && x
var keep_1 = typeof x != 'object' && x
var keep_1 = typeof x === 'object' || x
var keep_1 = typeof x == 'object' || x
var keep_1 = 'object' !== typeof x ? x : null
var keep_1 = 'object' != typeof x ? x : null
var keep_1 = 'object' === typeof x ? null : x
var keep_1 = 'object' == typeof x ? null : x
var keep_1 = 'object' !== typeof x && x
var keep_1 = 'object' != typeof x && x
var keep_1 = 'object' === typeof x || x
var keep_1 = 'object' == typeof x || x
// Everything here should be kept as live code because it has side effects
var keep_2 = typeof x !== 'undefined' ? y : null
var keep_2 = typeof x != 'undefined' ? y : null
var keep_2 = typeof x === 'undefined' ? null : y
var keep_2 = typeof x == 'undefined' ? null : y
var keep_2 = typeof x !== 'undefined' && y
var keep_2 = typeof x != 'undefined' && y
var keep_2 = typeof x === 'undefined' || y
var keep_2 = typeof x == 'undefined' || y
var keep_2 = 'undefined' !== typeof x ? y : null
var keep_2 = 'undefined' != typeof x ? y : null
var keep_2 = 'undefined' === typeof x ? null : y
var keep_2 = 'undefined' == typeof x ? null : y
var keep_2 = 'undefined' !== typeof x && y
var keep_2 = 'undefined' != typeof x && y
var keep_2 = 'undefined' === typeof x || y
var keep_2 = 'undefined' == typeof x || y
// Everything here should be kept as live code because it has side effects
var keep_3 = typeof x !== 'undefined' ? null : x
var keep_3 = typeof x != 'undefined' ? null : x
var keep_3 = typeof x === 'undefined' ? x : null
var keep_3 = typeof x == 'undefined' ? x : null
var keep_3 = typeof x !== 'undefined' || x
var keep_3 = typeof x != 'undefined' || x
var keep_3 = typeof x === 'undefined' && x
var keep_3 = typeof x == 'undefined' && x
var keep_3 = 'undefined' !== typeof x ? null : x
var keep_3 = 'undefined' != typeof x ? null : x
var keep_3 = 'undefined' === typeof x ? x : null
var keep_3 = 'undefined' == typeof x ? x : null
var keep_3 = 'undefined' !== typeof x || x
var keep_3 = 'undefined' != typeof x || x
var keep_3 = 'undefined' === typeof x && x
var keep_3 = 'undefined' == typeof x && x
`,
},
entryPaths: []string{"/entry.js"},
options: config.Options{
Mode: config.ModeBundle,
OutputFormat: config.FormatIIFE,
AbsOutputFile: "/out.js",
},
})
}
55 changes: 55 additions & 0 deletions internal/bundler/snapshots/snapshots_dce.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,61 @@ TestDCETypeOfEqualsString
console.log(hasBar);
})();

================================================================================
TestDCETypeOfEqualsStringGuardCondition
---------- /out.js ----------
(() => {
// entry.js
var keep_1 = typeof x !== "object" ? x : null;
var keep_1 = typeof x != "object" ? x : null;
var keep_1 = typeof x === "object" ? null : x;
var keep_1 = typeof x == "object" ? null : x;
var keep_1 = typeof x !== "object" && x;
var keep_1 = typeof x != "object" && x;
var keep_1 = typeof x === "object" || x;
var keep_1 = typeof x == "object" || x;
var keep_1 = typeof x !== "object" ? x : null;
var keep_1 = typeof x != "object" ? x : null;
var keep_1 = typeof x === "object" ? null : x;
var keep_1 = typeof x == "object" ? null : x;
var keep_1 = typeof x !== "object" && x;
var keep_1 = typeof x != "object" && x;
var keep_1 = typeof x === "object" || x;
var keep_1 = typeof x == "object" || x;
var keep_2 = typeof x !== "undefined" ? y : null;
var keep_2 = typeof x != "undefined" ? y : null;
var keep_2 = typeof x === "undefined" ? null : y;
var keep_2 = typeof x == "undefined" ? null : y;
var keep_2 = typeof x !== "undefined" && y;
var keep_2 = typeof x != "undefined" && y;
var keep_2 = typeof x === "undefined" || y;
var keep_2 = typeof x == "undefined" || y;
var keep_2 = typeof x !== "undefined" ? y : null;
var keep_2 = typeof x != "undefined" ? y : null;
var keep_2 = typeof x === "undefined" ? null : y;
var keep_2 = typeof x == "undefined" ? null : y;
var keep_2 = typeof x !== "undefined" && y;
var keep_2 = typeof x != "undefined" && y;
var keep_2 = typeof x === "undefined" || y;
var keep_2 = typeof x == "undefined" || y;
var keep_3 = typeof x !== "undefined" ? null : x;
var keep_3 = typeof x != "undefined" ? null : x;
var keep_3 = typeof x === "undefined" ? x : null;
var keep_3 = typeof x == "undefined" ? x : null;
var keep_3 = typeof x !== "undefined" || x;
var keep_3 = typeof x != "undefined" || x;
var keep_3 = typeof x === "undefined" && x;
var keep_3 = typeof x == "undefined" && x;
var keep_3 = typeof x !== "undefined" ? null : x;
var keep_3 = typeof x != "undefined" ? null : x;
var keep_3 = typeof x === "undefined" ? x : null;
var keep_3 = typeof x == "undefined" ? x : null;
var keep_3 = typeof x !== "undefined" || x;
var keep_3 = typeof x != "undefined" || x;
var keep_3 = typeof x === "undefined" && x;
var keep_3 = typeof x == "undefined" && x;
})();

================================================================================
TestDCETypeOfEqualsStringMangle
---------- /out.js ----------
Expand Down
45 changes: 42 additions & 3 deletions internal/js_parser/js_parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13340,7 +13340,9 @@ func (p *parser) exprCanBeRemovedIfUnused(expr js_ast.Expr) bool {
return true

case *js_ast.EIf:
return p.exprCanBeRemovedIfUnused(e.Test) && p.exprCanBeRemovedIfUnused(e.Yes) && p.exprCanBeRemovedIfUnused(e.No)
return p.exprCanBeRemovedIfUnused(e.Test) &&
((p.isSideEffectFreeUnboundIdentifierRef(e.Yes, e.Test, true) || p.exprCanBeRemovedIfUnused(e.Yes)) &&
(p.isSideEffectFreeUnboundIdentifierRef(e.No, e.Test, false) || p.exprCanBeRemovedIfUnused(e.No)))

case *js_ast.EArray:
for _, item := range e.Items {
Expand Down Expand Up @@ -13414,10 +13416,19 @@ func (p *parser) exprCanBeRemovedIfUnused(expr js_ast.Expr) bool {
switch e.Op {
// These operators must not have any type conversions that can execute code
// such as "toString" or "valueOf". They must also never throw any exceptions.
case js_ast.BinOpStrictEq, js_ast.BinOpStrictNe, js_ast.BinOpComma,
js_ast.BinOpLogicalOr, js_ast.BinOpLogicalAnd, js_ast.BinOpNullishCoalescing:
case js_ast.BinOpStrictEq, js_ast.BinOpStrictNe, js_ast.BinOpComma, js_ast.BinOpNullishCoalescing:
return p.exprCanBeRemovedIfUnused(e.Left) && p.exprCanBeRemovedIfUnused(e.Right)

// Special-case "||" to make sure "typeof x === 'undefined' || x" can be removed
case js_ast.BinOpLogicalOr:
return p.exprCanBeRemovedIfUnused(e.Left) &&
(p.isSideEffectFreeUnboundIdentifierRef(e.Right, e.Left, false) || p.exprCanBeRemovedIfUnused(e.Right))

// Special-case "&&" to make sure "typeof x !== 'undefined' && x" can be removed
case js_ast.BinOpLogicalAnd:
return p.exprCanBeRemovedIfUnused(e.Left) &&
(p.isSideEffectFreeUnboundIdentifierRef(e.Right, e.Left, true) || p.exprCanBeRemovedIfUnused(e.Right))

// For "==" and "!=", pretend the operator was actually "===" or "!==". If
// we know that we can convert it to "==" or "!=", then we can consider the
// operator itself to have no side effects. This matters because our mangle
Expand All @@ -13433,6 +13444,34 @@ func (p *parser) exprCanBeRemovedIfUnused(expr js_ast.Expr) bool {
return false
}

func (p *parser) isSideEffectFreeUnboundIdentifierRef(value js_ast.Expr, guardCondition js_ast.Expr, isYesBranch bool) bool {
if id, ok := value.Data.(*js_ast.EIdentifier); ok && p.symbols[id.Ref.InnerIndex].Kind == js_ast.SymbolUnbound {
if binary, ok := guardCondition.Data.(*js_ast.EBinary); ok {
switch binary.Op {
case js_ast.BinOpStrictEq, js_ast.BinOpStrictNe, js_ast.BinOpLooseEq, js_ast.BinOpLooseNe:
// Pattern match for "typeof x !== <string>"
typeof, string := binary.Left, binary.Right
if _, ok := typeof.Data.(*js_ast.EString); ok {
typeof, string = string, typeof
}
if typeof, ok := typeof.Data.(*js_ast.EUnary); ok && typeof.Op == js_ast.UnOpTypeof {
if text, ok := string.Data.(*js_ast.EString); ok {
// In "typeof x !== 'undefined' ? x : null", the reference to "x" is side-effect free
// In "typeof x === 'object' ? x : null", the reference to "x" is side-effect free
if (js_lexer.UTF16EqualsString(text.Value, "undefined") == isYesBranch) ==
(binary.Op == js_ast.BinOpStrictNe || binary.Op == js_ast.BinOpLooseNe) {
if id2, ok := typeof.Value.Data.(*js_ast.EIdentifier); ok && id2.Ref == id.Ref {
return true
}
}
}
}
}
}
}
return false
}

// This will return a nil expression if the expression can be totally removed
func (p *parser) simplifyUnusedExpr(expr js_ast.Expr) js_ast.Expr {
switch e := expr.Data.(type) {
Expand Down

0 comments on commit 9e5e767

Please sign in to comment.