babel / babel/minify

Dead code elimination can eliminate side-effecting code

Open
#931 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
4.4k
Forks
217
PR merge metrics
No merged PRs in 30d

Description

Copying from https://github.com/babel/minify/issues/930#issuecomment-439339599 to make sure it gets tracked:

```js
'use strict';
function foo() {
if (bar(), true) {
baz();
} else {
NOT_REACHABLE();
}
}
```
with dead code and simplify gets transformed to
```js
"use strict";function foo(){baz()}
```

which is wrong - the call to `bar()` can have side effects.

I expect the bug is [here](https://github.com/babel/minify/blob/245949fac66c2d4637045c4ba80fd4fb1a06d1cc/packages/babel-plugin-minify-dead-code-elimination/src/index.js#L563-L565): this code assumes that anything which `.evaluate()`s confidently can be safely replaced with its evaluation, but that's [not true](https://github.com/babel/babel/blob/1af57e6f71ecffae10cb932415ac849d9cdc14e9/packages/babel-traverse/src/path/evaluation.js#L78-L81) - you can have side effects even for things you're confident about. Compare the [more complex code in the path for `IfStatement`](https://github.com/babel/minify/blob/245949fac66c2d4637045c4ba80fd4fb1a06d1cc/packages/babel-plugin-minify-dead-code-elimination/src/index.js#L871-L894), which was added in #386 to fix #385. I imagine the fix is basically copying that code to the `ConditionalExpression` case; probably a good first issue for someone.

(This issue is pretty much the same as https://github.com/babel/minify/issues/385#issuecomment-274338700, just with slightly different input conditions.)

Contributor guide

Open the contributing guide

Research direction

Start in packages/babel-plugin-minify-dead-code-elimination/src/index.js at the ConditionalExpression handling around lines 563–565, then compare the IfStatement handling around lines 871–894 and Babel's evaluation.js. Verify the example preserves bar() while eliminating unreachable code, and add a regression test for this input.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.