From 3b37387e7e436004eb832e995a304fbe5589606d Mon Sep 17 00:00:00 2001 From: Boopathi Rajaa Date: Wed, 4 Oct 2017 13:45:49 +0200 Subject: [PATCH 1/2] Bail for binary 'in' expressions - fix #691 --- .../__tests__/dead-code-elimination-test.js | 13 ++++++++ .../src/index.js | 33 ++++++++++++++++++- 2 files changed, 45 insertions(+), 1 deletion(-) 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 index 045305e7d..3c4519e14 100644 --- 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 @@ -2503,4 +2503,17 @@ describe("dce-plugin", () => { plugins: [[deadcode, { tdz: true }]] } ); + + thePlugin( + "should bail for binary `in` expressions - fix #691", + ` + function foo(props) { + let bar = "width" in props; + delete props.width; + if (bar) { + console.log("foo"); + } + } + ` + ); }); 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 e15fae757..7fcb48c04 100644 --- a/packages/babel-plugin-minify-dead-code-elimination/src/index.js +++ b/packages/babel-plugin-minify-dead-code-elimination/src/index.js @@ -390,7 +390,9 @@ module.exports = ({ types: t, traverse }) => { const isObj = n => t.isFunction(n) || t.isObjectExpression(n) || - t.isArrayExpression(n); + t.isArrayExpression(n) || + t.isBinaryExpression(n, { operator: "in" }); + const isReplacementObj = isObj(replacement) || some(replacement, isObj); @@ -398,6 +400,35 @@ module.exports = ({ types: t, traverse }) => { continue; } + // check if it's safe to replace + // To solve https://github.com/babel/minify/issues/691 + // Here we bail for property checks using the "in" operator + // This is because - `in` is a side-effect-free operation but the property + // could be deleted between the replacementPath and referencePath + // It is expensive to compute the delete operation and we bail for + // all the binary "in" operations + let inExpression = replacementPath.isBinaryExpression({ + operator: "in" + }); + + if (!inExpression) { + replacementPath.traverse({ + Function(path) { + path.skip(); + }, + BinaryExpression(path) { + if (path.node.operator === "in") { + inExpression = true; + path.stop(); + } + } + }); + } + + if (inExpression) { + continue; + } + const replaced = replace(binding.referencePaths[0], { binding, scope, From b710aaf670537bf07aeeedfb3c0a21cb0083763a Mon Sep 17 00:00:00 2001 From: Boopathi Rajaa Date: Wed, 4 Oct 2017 13:52:39 +0200 Subject: [PATCH 2/2] Remove unnecessary code --- .../babel-plugin-minify-dead-code-elimination/src/index.js | 3 +-- 1 file changed, 1 insertion(+), 2 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 7fcb48c04..90e7fc998 100644 --- a/packages/babel-plugin-minify-dead-code-elimination/src/index.js +++ b/packages/babel-plugin-minify-dead-code-elimination/src/index.js @@ -390,8 +390,7 @@ module.exports = ({ types: t, traverse }) => { const isObj = n => t.isFunction(n) || t.isObjectExpression(n) || - t.isArrayExpression(n) || - t.isBinaryExpression(n, { operator: "in" }); + t.isArrayExpression(n); const isReplacementObj = isObj(replacement) || some(replacement, isObj);