fix(dce): bail on non-constant replacement path

In one use variable replacement, if the right hand side contains constant violations, the left-hand side is supposed to be preserved, so we detect this and bail out for this case + Fix #685
babel · Dec 11, 2017 · dc015c6 · dc015c6
1 parent 033d9ab
commit dc015c6
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 6 deletions.
diff --git a/packages/babel-plugin-minify-dead-code-elimination/__tests__/fixtures/issue-685/actual.js b/packages/babel-plugin-minify-dead-code-elimination/__tests__/fixtures/issue-685/actual.js
@@ -0,0 +1,9 @@
+function loop() {
+  var end = 0;
+  var start = end;
+  while (end < 10) {
+    console.log(start, end);
+    var end = end + 1;
+  }
+}
+loop();
diff --git a/packages/babel-plugin-minify-dead-code-elimination/__tests__/fixtures/issue-685/expected.js b/packages/babel-plugin-minify-dead-code-elimination/__tests__/fixtures/issue-685/expected.js
@@ -0,0 +1,9 @@
+function loop() {
+  var end = 0;
+  var start = end;
+  while (end < 10) {
+    console.log(start, end);
+    var end = end + 1;
+  }
+}
+loop();
diff --git a/packages/babel-plugin-minify-dead-code-elimination/src/index.js b/packages/babel-plugin-minify-dead-code-elimination/src/index.js
@@ -349,9 +349,15 @@ module.exports = ({ types: t, traverse }) => {
               let bail = false;
 
               if (replacementPath.isIdentifier()) {
-                bail =
-                  refPath.scope.getBinding(replacement.name) !==
-                  scope.getBinding(replacement.name);
+                const binding = scope.getBinding(replacement.name);
+                // the reference should be in the same scope
+                // and the replacement should be a constant - this is to
+                // ensure that the duplication of replacement is not affected
+                // https://github.com/babel/minify/issues/685
+                bail = !(
+                  refPath.scope.getBinding(replacement.name) === binding &&
+                  binding.constantViolations.length === 0
+                );
               } else {
                 replacementPath.traverse({
                   Function(path) {
@@ -362,9 +368,11 @@ module.exports = ({ types: t, traverse }) => {
                     if (bail) {
                       return;
                     }
-                    bail =
-                      refPath.scope.getBinding(node.name) !==
-                      scope.getBinding(node.name);
+                    const binding = scope.getBinding(node.name);
+                    bail = !(
+                      refPath.scope.getBinding(node.name) === binding ||
+                      binding.constantViolations.length === 0
+                    );
                   }
                 });
               }