Handle hoisted variables in dead branches of nested if statements

rollup · Sep 16, 2020 · f3d7a27 · f3d7a27
1 parent 022770a
commit f3d7a27
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 16 deletions.
diff --git a/src/ast/nodes/IfStatement.ts b/src/ast/nodes/IfStatement.ts
@@ -117,7 +117,7 @@ export default class IfStatement extends StatementBase implements DeoptimizableE
 				hoistedDeclarations.push(...this.alternateScope!.hoistedDeclarations);
 			}
 		}
-		renderHoistedDeclarations(hoistedDeclarations, this.start, code);
+		this.renderHoistedDeclarations(hoistedDeclarations, code);
 	}
 
 	private getTestValue(): LiteralValueOrUnknown {
@@ -170,6 +170,27 @@ export default class IfStatement extends StatementBase implements DeoptimizableE
 		}
 	}
 
+	private renderHoistedDeclarations(hoistedDeclarations: Identifier[], code: MagicString) {
+		const hoistedVars = [
+			...new Set(
+				hoistedDeclarations.map(identifier => {
+					const variable = identifier.variable!;
+					return variable.included ? variable.getName() : '';
+				})
+			)
+		]
+			.filter(Boolean)
+			.join(', ');
+		if (hoistedVars) {
+			const parentType = this.parent.type;
+			const needsBraces = parentType !== NodeType.Program && parentType !== NodeType.BlockStatement;
+			code.prependRight(this.start, `${needsBraces ? '{ ' : ''}var ${hoistedVars}; `);
+			if (needsBraces) {
+				code.appendLeft(this.end, ` }`);
+			}
+		}
+	}
+
 	private shouldKeepAlternateBranch() {
 		let currentParent = this.parent;
 		do {
@@ -184,16 +205,3 @@ export default class IfStatement extends StatementBase implements DeoptimizableE
 		return false;
 	}
 }
-
-function renderHoistedDeclarations(
-	hoistedDeclarations: Identifier[],
-	prependPosition: number,
-	code: MagicString
-) {
-	const hoistedVars = [
-		...new Set(hoistedDeclarations.map(identifier => identifier.variable!.getName()))
-	].join(', ');
-	if (hoistedVars) {
-		code.prependRight(prependPosition, `var ${hoistedVars}; `);
-	}
-}
diff --git a/test/form/samples/hoisted-variable-if-stmt/_expected.js b/test/form/samples/hoisted-variable-if-stmt/_expected.js
@@ -5,7 +5,9 @@ if (foo) {
 }
 
 {
-    var bar = true;
+    var bar; {
+        bar = true;
+    }
 }
 
 if (bar) {

diff --git a/test/form/samples/hoisted-variable-if-stmt/main.js b/test/form/samples/hoisted-variable-if-stmt/main.js
@@ -1,13 +1,18 @@
 if (false) {
     var foo = true;
+    var unused;
 }
 
 if (foo) {
     console.log("nope");
 }
 
 if (true) {
-    var bar = true;
+    if (false) {
+        var bar = false;
+    } else {
+        bar = true;
+    }
 }
 
 if (bar) {

diff --git a/test/function/samples/hoisted-variable-if-else/_config.js b/test/function/samples/hoisted-variable-if-else/_config.js
@@ -0,0 +1,11 @@
+const assert = require('assert');
+
+module.exports = {
+	description: 'handles hoisted variables in chained if statements',
+	exports(exports) {
+		exports.test(true);
+		assert.strictEqual(exports.result, 'first');
+		exports.test(false);
+		assert.strictEqual(exports.result, 'third');
+	}
+};
diff --git a/test/function/samples/hoisted-variable-if-else/main.js b/test/function/samples/hoisted-variable-if-else/main.js
@@ -0,0 +1,19 @@
+export let result;
+
+export function test(value) {
+	if (value) {
+		v = 'first';
+		result = v;
+	} else if (false) {
+		var v, w;
+		result = 'second';
+	} else {
+		v = 'third';
+		result = v;
+	}
+
+	for (var foo of []) if (false) var x; else {
+		x = 'fourth';
+		result = x;
+	}
+}