Skip to content

Commit

Permalink
"typeof identifier" has no side effects
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Sep 22, 2021
1 parent 3249e05 commit 0b90b0a
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 2 deletions.
16 changes: 16 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,22 @@
});
```

* Consider `typeof x` to have no side effects

The `typeof` operator does not itself trigger any code evaluation so it can safely be removed if evaluating the operand does not cause any side effects. However, there is a special case of the `typeof` operator when the operand is an identifier expression. In that case no reference error is thrown if the referenced symbol does not exist (e.g. `typeof x` does not throw an error if there is no symbol named `x`). With this release, esbuild will now consider `typeof x` to have no side effects even if evaluating `x` would have side effects (i.e. would throw a reference error):

```js
// Original code
var unused = typeof React !== 'undefined';

// Old output
var unused = typeof React !== 'undefined';

// New output
```

Note that there is actually an edge case where `typeof x` *can* throw an error: when `x` is being referenced inside of its TDZ, or temporal dead zone (i.e. before it's declared). This applies to `let`, `const`, and `class` symbols. However, esbuild doesn't currently handle TDZ rules so the possibility of errors thrown due to TDZ rules is not currently considered. This typically doesn't matter in real-world code so this hasn't been a priority to fix (and is actually tricky to fix with esbuild's current bundling approach). So esbuild may incorrectly remove a `typeof` expression that actually has side effects. However, esbuild already incorrectly did this in previous releases so its behavior regarding `typeof` and TDZ rules hasn't changed in this release.

## 0.12.28

* Fix U+30FB and U+FF65 in identifier names in ES5 vs. ES6+ ([#1599](https://github.com/evanw/esbuild/issues/1599))
Expand Down
34 changes: 33 additions & 1 deletion internal/bundler/bundler_dce_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1504,7 +1504,6 @@ func TestTreeShakingUnaryOperators(t *testing.T) {
let REMOVE;
!REMOVE;
void REMOVE;
typeof REMOVE;
`,
},
entryPaths: []string{"/entry.js"},
Expand Down Expand Up @@ -1655,3 +1654,36 @@ func TestTreeShakingInESMWrapper(t *testing.T) {
},
})
}

func TestDCETypeOf(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry.js": `
// These should be removed because they have no side effects
typeof x_REMOVE
typeof v_REMOVE
typeof f_REMOVE
typeof g_REMOVE
typeof a_REMOVE
var v_REMOVE
function f_REMOVE() {}
function* g_REMOVE() {}
async function a_REMOVE() {}
// These technically have side effects due to TDZ, but this is not currently handled
typeof c_remove
typeof l_remove
typeof s_remove
const c_remove = 0
let l_remove
class s_remove {}
`,
},
entryPaths: []string{"/entry.js"},
options: config.Options{
Mode: config.ModeBundle,
OutputFormat: config.FormatESModule,
AbsOutputFile: "/out.js",
},
})
}
4 changes: 4 additions & 0 deletions internal/bundler/snapshots/snapshots_dce.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ TestBase64LoaderRemoveUnused
// entry.js
console.log("unused import");

================================================================================
TestDCETypeOf
---------- /out.js ----------

================================================================================
TestDataURLLoaderRemoveUnused
---------- /out.js ----------
Expand Down
18 changes: 17 additions & 1 deletion internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -13390,7 +13390,23 @@ 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.UnOpTypeof, js_ast.UnOpVoid, js_ast.UnOpNot:
case js_ast.UnOpVoid, js_ast.UnOpNot:
return p.exprCanBeRemovedIfUnused(e.Value)

// The "typeof" operator doesn't do any type conversions so it can be removed
// if the result is unused and the operand has no side effects. However, it
// has a special case where if the operand is an identifier expression such
// as "typeof x" and "x" doesn't exist, no reference error is thrown so the
// operation has no side effects.
//
// Note that there *is* actually a case where "typeof x" can throw an error:
// when "x" is being referenced inside of its TDZ (temporal dead zone). TDZ
// checks are not yet handled correctly by esbuild, so this possibility is
// currently ignored.
case js_ast.UnOpTypeof:
if _, ok := e.Value.Data.(*js_ast.EIdentifier); ok {
return true
}
return p.exprCanBeRemovedIfUnused(e.Value)
}

Expand Down

0 comments on commit 0b90b0a

Please sign in to comment.