babel / babel/minify

For loops declaration sometimes shadows loop condition variables

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

Description

**Describe the bug**

When outputting `for` loops, it seems like sometimes the loop initializer will shadow outside variables that are also used in the loop condition. e.g.

```
const e = 100
for (let r = 0, e; r < e; r++) {
e = 'foobar'
}
```

I'm actually unsure on what the JavaScript spec says that the `e` in the loop condition (`r < e`) should refer to, and it seems like Safari are referring to the `e` created by the `let` statement in the loop initializer, whereas Chrome is referring to the outer `e` created by the `const` declaration.

This is causing the provided code to fail in Safari for me (the loop won't run and the function will return an empty string), so it would be great if babel-minify could avoid shadowing outer variables that are used in the loop condidtion ☺️

**To Reproduce**

Minimal code to reproduce the bug

```js
window.decodeUtf8 = function(e) {
const t = function(e) {
if (e instanceof Uint8Array) return e;
if (e instanceof ArrayBuffer) return new Uint8Array(e);
throw new TypeError('Expected "input" to be an ArrayBuffer or Uint8Array')
}(e),
r = t.length;
let n = "";
for (let e = 0; e < r; e++) {
let r = t[e];
if (r < 128) n += String.fromCodePoint(r);
else if (192 != (224 & r))
if (224 != (240 & r))
if (240 != (248 & r));
else {
let o = 63 & t[++e],
i = 63 & t[++e],
f = 63 & t[++e];
n += String.fromCodePoint((7 & r) << 18 | o << 12 | i << 6 | f)
}
else {
let o = 63 & t[++e],
i = 63 & t[++e];
n += String.fromCodePoint((15 & r) << 12 | o << 6 | i)
} else {
let o = 63 & t[++e];
n += String.fromCodePoint((31 & r) << 6 | o)
}
}
return n
}
```

**Actual Output**

If there is no Error thrown,

```js
window.decodeUtf8=function(a){var b=String.fromCodePoint;const c=function(a){if(a instanceof Uint8Array)return a;if(a instanceof ArrayBuffer)return new Uint8Array(a);throw new TypeError("Expected \"input\" to be an ArrayBuffer or Uint8Array")}(a),d=c.length;let g="";for(let d,h=0;hd)g+=b(d);else if(192==(224&d)){let a=63&c[++h];g+=b((31&d)<<6|a)}else if(224==(240&d)){let a=63&c[++h],e=63&c[++h];g+=b((15&d)<<12|a<<6|e)}else if(240!=(248&d));else{let a=63&c[++h],e=63&c[++h],i=63&c[++h];g+=b((7&d)<<18|a<<12|e<<6|i)}return g};
```

relevant part, deminified:

```js
const c = /* ... */,
d = c.length;
let g = "";
for (let d, h = 0; h < d; h++)
if (d = c[h], 128 > d) g += b(d);
```

**Expected Output**

I would have expected to not reuse the same outer and inner variable name:

```js
const c = /* ... */,
d = c.length;
let g = "";
for (let X, h = 0; h < d; h++)
if (X = c[h], 128 > X) g += b(X);
```

**Configuration**

How are you using babel-minify?

`babel-preset-minify in babelrc`

babel-minify version: `0.5.0`

babel version : `7.2.3`

babel-minify-config:

```json5
{}
```

babelrc:

```json5
{
"presets": ["minify"],
"comments": false
}
```

Contributor guide

Open the contributing guide

Research direction

Start by reproducing the supplied input with babel-preset-minify using the shown babelrc and babel-minify 0.5.0, then inspect the generated for-loop declaration and condition bindings. Done means the minified output avoids reusing the outer condition variable as the loop body's inner variable and preserves the expected behavior in Safari.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.