Repository: Rich-Harris/buble (js)

Issue:
Loops create redundant "this" variables
Using `this` keyword inside a loop creates redundant variables that aren't necessary because loops don't create closures (`this` inside a loop should always point to the same object as outside). 

[Minimal example](https://buble.surge.sh/#function%20bar()%20%7B%0A%20%20while%20(true)%20this%3B%0A%7D):

```js
function bar() {
  while (true) this;
}
```

Produces:

```js
function bar() {
  var this$1 = this;

  while (true) { this$1; }
}
```

Expected output:

```js
function bar() {
  while (true) { this; }
}
```

Same behavior for `for` and `do ... while` loops. Looks like a bug to me — a harmless one in practice, but quite unpleasant for perfectionists like me. :)

Pull request:
Fix redundant this alias in loops
Fixes #120. This turned out to be more involved that I thought (because loop rewriting is initialized after all its children nodes), but the logic should be more sound now. 

Patch:
diff --git a/src/program/types/ThisExpression.js b/src/program/types/ThisExpression.js
index b672b7b..6f1377f 100644
--- a/src/program/types/ThisExpression.js
+++ b/src/program/types/ThisExpression.js
@@ -3,18 +3,22 @@ import { loopStatement } from '../../utils/patterns.js';
 
 export default class ThisExpression extends Node {
 	initialise(transforms) {
+		const lexicalBoundary = this.findLexicalBoundary();
+
+		if (transforms.letConst) {
+			// save all loops up to the lexical boundary in case we need
+			// to alias them later for block-scoped declarations
+			let node = this.findNearest(loopStatement);
+			while (node && node.depth > lexicalBoundary.depth) {
+				node.thisRefs.push(this);
+				node = node.parent.findNearest(loopStatement);
+			}
+		}
+
 		if (transforms.arrow) {
-			const lexicalBoundary = this.findLexicalBoundary();
 			const arrowFunction = this.findNearest('ArrowFunctionExpression');
-			const loop = this.findNearest(loopStatement);
 
-			if (
-				(arrowFunction && arrowFunction.depth > lexicalBoundary.depth) ||
-				(loop &&
-					loop.body.contains(this) &&
-					loop.depth > lexicalBoundary.depth) ||
-				(loop && loop.right && loop.right.contains(this))
-			) {
+			if (arrowFunction && arrowFunction.depth > lexicalBoundary.depth) {
 				this.alias = lexicalBoundary.getThisAlias();
 			}
 		}
diff --git a/src/program/types/shared/LoopStatement.js b/src/program/types/shared/LoopStatement.js
index 1e6154f..6e664a0 100644
--- a/src/program/types/shared/LoopStatement.js
+++ b/src/program/types/shared/LoopStatement.js
@@ -15,6 +15,8 @@ export default class LoopStatement extends Node {
 		this.reassigned = Object.create(null);
 		this.aliases = Object.create(null);
 
+		this.thisRefs = [];
+
 		super.initialise(transforms);
 
 		if (transforms.letConst) {
@@ -37,6 +39,9 @@ export default class LoopStatement extends Node {
 						nearestFunctionExpression.depth > this.depth
 					) {
 						this.shouldRewriteAsFunction = true;
+						for (const node of this.thisRefs) {
+							node.alias = node.alias || node.findLexicalBoundary().getThisAlias();
+						}
 						break;
 					}
 				}


Test patch:
diff --git a/test/samples/for-of.js b/test/samples/for-of.js
index 2f32cd2..83389cc 100644
--- a/test/samples/for-of.js
+++ b/test/samples/for-of.js
@@ -95,9 +95,7 @@ module.exports = [
 			}`,
 
 		output: `
-			var this$1 = this;
-
-			for (var i = 0, list = this$1.keys; i < list.length; i += 1) {
+			for (var i = 0, list = this.keys; i < list.length; i += 1) {
 				var key = list[i];
 
 				console.log(key);