From dc015c6914df3567a0d1ae25d0b402dc81d8f6b5 Mon Sep 17 00:00:00 2001 From: Boopathi Rajaa Date: Mon, 11 Dec 2017 02:05:10 +0100 Subject: [PATCH 1/3] 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 --- .../__tests__/fixtures/issue-685/actual.js | 9 +++++++++ .../__tests__/fixtures/issue-685/expected.js | 9 +++++++++ .../src/index.js | 20 +++++++++++++------ 3 files changed, 32 insertions(+), 6 deletions(-) create mode 100644 packages/babel-plugin-minify-dead-code-elimination/__tests__/fixtures/issue-685/actual.js create mode 100644 packages/babel-plugin-minify-dead-code-elimination/__tests__/fixtures/issue-685/expected.js 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 new file mode 100644 index 000000000..e0fefa280 --- /dev/null +++ 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 new file mode 100644 index 000000000..95f5fa4c1 --- /dev/null +++ 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(); \ No newline at end of file diff --git a/packages/babel-plugin-minify-dead-code-elimination/src/index.js b/packages/babel-plugin-minify-dead-code-elimination/src/index.js index 037fccefa..131aee93b 100644 --- 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 + ); } }); } From 03552495bbd9340c3c96362e0e2f0e5fd1f07567 Mon Sep 17 00:00:00 2001 From: Boopathi Rajaa Date: Mon, 11 Dec 2017 02:21:16 +0100 Subject: [PATCH 2/3] Fix binding check and checks for not Identifier --- .../src/index.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/babel-plugin-minify-dead-code-elimination/src/index.js b/packages/babel-plugin-minify-dead-code-elimination/src/index.js index 131aee93b..448fa7d18 100644 --- a/packages/babel-plugin-minify-dead-code-elimination/src/index.js +++ b/packages/babel-plugin-minify-dead-code-elimination/src/index.js @@ -355,6 +355,7 @@ module.exports = ({ types: t, traverse }) => { // ensure that the duplication of replacement is not affected // https://github.com/babel/minify/issues/685 bail = !( + binding && refPath.scope.getBinding(replacement.name) === binding && binding.constantViolations.length === 0 ); @@ -369,10 +370,12 @@ module.exports = ({ types: t, traverse }) => { return; } const binding = scope.getBinding(node.name); - bail = !( - refPath.scope.getBinding(node.name) === binding || - binding.constantViolations.length === 0 - ); + if ( + binding && + refPath.scope.getBinding(node.name) === binding + ) { + bail = binding.constantViolations.length > 0; + } } }); } From 88aa6909a8bdb9cce55ccf5b1a60a8500f6cb461 Mon Sep 17 00:00:00 2001 From: Boopathi Rajaa Date: Mon, 11 Dec 2017 22:11:46 +0100 Subject: [PATCH 3/3] Add testcase for non-identifier replacementPaths --- .../__tests__/fixtures/issue-685/actual.js | 7 +++++++ .../__tests__/fixtures/issue-685/expected.js | 9 ++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) 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 index e0fefa280..c39141aea 100644 --- 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 @@ -7,3 +7,10 @@ function loop() { } } loop(); + +function bar() { + var x = 1; + var y = x + 2; + var x = 3; + return x + y; +} 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 index 95f5fa4c1..62b3623b5 100644 --- 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 @@ -6,4 +6,11 @@ function loop() { var end = end + 1; } } -loop(); \ No newline at end of file +loop(); + +function bar() { + var x = 1; + var y = x + 2; + var x = 3; + return x + y; +} \ No newline at end of file