[fix #574] deopt when binding is present in diff scope

babel · Jun 21, 2017 · d040155 · d040155
1 parent 2bcf2f6
commit d040155
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 8 deletions.
diff --git a/packages/babel-plugin-minify-dead-code-elimination/__tests__/dead-code-elimination-test.js b/packages/babel-plugin-minify-dead-code-elimination/__tests__/dead-code-elimination-test.js
@@ -2710,4 +2710,14 @@ describe("dce-plugin", () => {
     `);
     expect(transform(source)).toBe(expected);
   });
+
+  it("should deopt when binding is on different scope - issue #574", () => {
+    const source = unpad(`
+      function foo(v) {
+        if (v) var w = 10;
+        if (w) console.log("hello", v);
+      }
+    `);
+    expect(transform(source)).toBe(source);
+  });
 });
diff --git a/packages/babel-plugin-minify-dead-code-elimination/src/index.js b/packages/babel-plugin-minify-dead-code-elimination/src/index.js
@@ -22,6 +22,10 @@ function forEachAncestor(path, callback) {
   }
 }
 
+function isDifferentScope(varDeclarationPath, ifStatementPath) {
+  return varDeclarationPath.parentPath.isDescendant(ifStatementPath.parentPath);
+}
+
 module.exports = ({ types: t, traverse }) => {
   const removeOrVoid = require("babel-helper-remove-or-void")(t);
   const shouldRevisit = Symbol("shouldRevisit");
@@ -187,11 +191,13 @@ module.exports = ({ types: t, traverse }) => {
             ) {
               continue;
             } else if (binding.path.isVariableDeclarator()) {
+              const parentPath = binding.path.parentPath.parentPath;
               if (
-                binding.path.parentPath.parentPath &&
-                binding.path.parentPath.parentPath.isForXStatement()
+                parentPath &&
+                (parentPath.isForXStatement() || parentPath.isIfStatement())
               ) {
                 // Can't remove if in a for-in/for-of/for-await statement `for (var x in wat)`.
+                // Can't remove if (true) {var a = 10};
                 continue;
               }
             } else if (!scope.isPure(binding.path.node)) {
@@ -285,14 +291,10 @@ module.exports = ({ types: t, traverse }) => {
               let replacementPath = binding.path;
               let isReferencedBefore = false;
 
-              if (binding.referencePaths.length > 1) {
-                throw new Error("Expected only one reference");
-              }
               const refPath = binding.referencePaths[0];
 
               if (t.isVariableDeclarator(replacement)) {
                 const _prevSiblings = prevSiblings(replacementPath);
-
                 // traverse ancestors of a reference checking if it's before declaration
                 forEachAncestor(refPath, ancestor => {
                   if (_prevSiblings.indexOf(ancestor) > -1) {
@@ -301,7 +303,7 @@ module.exports = ({ types: t, traverse }) => {
                 });
 
                 // deopt if reference is in different scope than binding
-                // since we don't know if it's sync or async execition
+                // since we don't know if it's sync or async execution
                 // (i.e. whether value has been assigned to a reference or not)
                 if (isReferencedBefore && refPath.scope !== binding.scope) {
                   continue;
@@ -765,8 +767,21 @@ module.exports = ({ types: t, traverse }) => {
           const evalResult = test.evaluate();
           const isPure = test.isPure();
 
-          const replacements = [];
+          const { path: bindingPath } =
+            path.scope.getBinding(test.node.name) || {};
 
+          // Ref - https://github.com/babel/babili/issues/574
+          // deopt if var is declared in other scope
+          // if (a) { var b = blahl;} if (b) { //something }
+          if (
+            bindingPath &&
+            bindingPath.parentPath.isVariableDeclaration() &&
+            isDifferentScope(bindingPath.parentPath, path)
+          ) {
+            return;
+          }
+
+          const replacements = [];
           if (evalResult.confident && !isPure && test.isSequenceExpression()) {
             replacements.push(
               t.expressionStatement(extractSequenceImpure(test))