Fix tree-shaking DCE (#1575)

parcel-bundler · Jun 25, 2018 · b62132c · b62132c
1 parent 12ddda7
commit b62132c
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 24 deletions.
diff --git a/src/scope-hoisting/shake.js b/src/scope-hoisting/shake.js
@@ -27,30 +27,7 @@ function treeShake(scope) {
 
       // Remove the binding and all references to it.
       binding.path.remove();
-      binding.referencePaths
-        .concat(binding.constantViolations)
-        .forEach(path => {
-          if (path.parentPath.isMemberExpression()) {
-            let parent = path.parentPath.parentPath;
-            if (
-              parent.parentPath.isSequenceExpression() &&
-              parent.parent.expressions.length === 1
-            ) {
-              parent.parentPath.remove();
-            } else if (!parent.removed) {
-              parent.remove();
-            }
-          } else if (isUnusedWildcard(path) && !path.parentPath.removed) {
-            path.parentPath.remove();
-          } else if (path.isAssignmentExpression()) {
-            let parent = path.parentPath;
-            if (!parent.isExpressionStatement()) {
-              path.replaceWith(path.node.right);
-            } else {
-              path.remove();
-            }
-          }
-        });
+      binding.referencePaths.concat(binding.constantViolations).forEach(remove);
 
       scope.removeBinding(name);
       removed = true;
@@ -124,3 +101,24 @@ function isUnusedWildcard(path) {
     !getUnusedBinding(path, parent.arguments[1].name)
   );
 }
+
+function remove(path) {
+  if (path.isAssignmentExpression()) {
+    if (!path.parentPath.isExpressionStatement()) {
+      path.replaceWith(path.node.right);
+    } else {
+      path.remove();
+    }
+  } else if (isExportAssignment(path)) {
+    remove(path.parentPath.parentPath);
+  } else if (isUnusedWildcard(path)) {
+    remove(path.parentPath);
+  } else if (
+    path.parentPath.isSequenceExpression() &&
+    path.parent.expressions.length === 1
+  ) {
+    remove(path.parentPath);
+  } else if (!path.removed) {
+    path.remove();
+  }
+}
diff --git a/test/integration/scope-hoisting/commonjs/export-local/a.js b/test/integration/scope-hoisting/commonjs/export-local/a.js
@@ -0,0 +1 @@
+module.exports = require('./b').foo;
diff --git a/test/integration/scope-hoisting/commonjs/export-local/b.js b/test/integration/scope-hoisting/commonjs/export-local/b.js
@@ -0,0 +1 @@
+var x = exports.foo = exports.bar = 3;
diff --git a/test/scope-hoisting.js b/test/scope-hoisting.js
@@ -766,5 +766,17 @@ describe('scope hoisting', function() {
       assert(contents.includes('foo'));
       assert(!contents.includes('bar'));
     });
+
+    it('supports removing an unused inline export with uglify minification', async function() {
+      // Uglify does strange things to multiple assignments in a line.
+      // See https://github.com/parcel-bundler/parcel/issues/1549
+      let b = await bundle(
+        __dirname + '/integration/scope-hoisting/commonjs/export-local/a.js',
+        {minify: true}
+      );
+
+      let output = await run(b);
+      assert.deepEqual(output, 3);
+    });
   });
 });