From b62132ceaf8d3c2019d7e86a9e987e13ee196c75 Mon Sep 17 00:00:00 2001 From: Fathy Boundjadj Date: Mon, 25 Jun 2018 06:57:00 +0200 Subject: [PATCH] Fix tree-shaking DCE (#1575) --- src/scope-hoisting/shake.js | 46 +++++++++---------- .../scope-hoisting/commonjs/export-local/a.js | 1 + .../scope-hoisting/commonjs/export-local/b.js | 1 + test/scope-hoisting.js | 12 +++++ 4 files changed, 36 insertions(+), 24 deletions(-) create mode 100644 test/integration/scope-hoisting/commonjs/export-local/a.js create mode 100644 test/integration/scope-hoisting/commonjs/export-local/b.js diff --git a/src/scope-hoisting/shake.js b/src/scope-hoisting/shake.js index 3133b52dc1b..701469c3dbd 100644 --- 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 new file mode 100644 index 00000000000..2e209e55fd4 --- /dev/null +++ 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 new file mode 100644 index 00000000000..76f87edaf00 --- /dev/null +++ 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 index ff5094d0c93..2b691ee610b 100644 --- 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); + }); }); });