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 22efc14e6..3376941d4 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 @@ -2710,4 +2710,14 @@ describe("dce-plugin", () => { `); expect(transform(source)).toBe(expected); }); + + it("should deopt when binding is on different scope - issue #574", () => { + const source = unpad(` + function foo(v) { + if (v) var w = 10; + if (w) console.log("hello", v); + } + `); + expect(transform(source)).toBe(source); + }); }); 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 b030a9330..c1a32d661 100644 --- a/packages/babel-plugin-minify-dead-code-elimination/src/index.js +++ b/packages/babel-plugin-minify-dead-code-elimination/src/index.js @@ -22,6 +22,10 @@ function forEachAncestor(path, callback) { } } +function isDifferentScope(varDeclarationPath, ifStatementPath) { + return varDeclarationPath.parentPath.isDescendant(ifStatementPath.parentPath); +} + module.exports = ({ types: t, traverse }) => { const removeOrVoid = require("babel-helper-remove-or-void")(t); const shouldRevisit = Symbol("shouldRevisit"); @@ -187,11 +191,13 @@ module.exports = ({ types: t, traverse }) => { ) { continue; } else if (binding.path.isVariableDeclarator()) { + const parentPath = binding.path.parentPath.parentPath; if ( - binding.path.parentPath.parentPath && - binding.path.parentPath.parentPath.isForXStatement() + parentPath && + (parentPath.isForXStatement() || parentPath.isIfStatement()) ) { // Can't remove if in a for-in/for-of/for-await statement `for (var x in wat)`. + // Can't remove if (true) {var a = 10}; continue; } } else if (!scope.isPure(binding.path.node)) { @@ -285,14 +291,10 @@ module.exports = ({ types: t, traverse }) => { let replacementPath = binding.path; let isReferencedBefore = false; - if (binding.referencePaths.length > 1) { - throw new Error("Expected only one reference"); - } const refPath = binding.referencePaths[0]; if (t.isVariableDeclarator(replacement)) { const _prevSiblings = prevSiblings(replacementPath); - // traverse ancestors of a reference checking if it's before declaration forEachAncestor(refPath, ancestor => { if (_prevSiblings.indexOf(ancestor) > -1) { @@ -301,7 +303,7 @@ module.exports = ({ types: t, traverse }) => { }); // deopt if reference is in different scope than binding - // since we don't know if it's sync or async execition + // since we don't know if it's sync or async execution // (i.e. whether value has been assigned to a reference or not) if (isReferencedBefore && refPath.scope !== binding.scope) { continue; @@ -765,8 +767,21 @@ module.exports = ({ types: t, traverse }) => { const evalResult = test.evaluate(); const isPure = test.isPure(); - const replacements = []; + const { path: bindingPath } = + path.scope.getBinding(test.node.name) || {}; + // Ref - https://github.com/babel/babili/issues/574 + // deopt if var is declared in other scope + // if (a) { var b = blahl;} if (b) { //something } + if ( + bindingPath && + bindingPath.parentPath.isVariableDeclaration() && + isDifferentScope(bindingPath.parentPath, path) + ) { + return; + } + + const replacements = []; if (evalResult.confident && !isPure && test.isSequenceExpression()) { replacements.push( t.expressionStatement(extractSequenceImpure(test))