Skip to content

Commit

Permalink
avoid transforming side effecty member expression checks[fix #469]
Browse files Browse the repository at this point in the history
  • Loading branch information
vigneshshanmugam committed Apr 18, 2017
1 parent 021eead commit 184cede
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -178,4 +178,16 @@ describe("guarded-expressions-plugin", () => {
);
expect(transform(source)).toBe(source);
});

it("should not minify side effecty member expression checks", () => {
let source = unpad(
`
{
var a = {};
}
var b = a && a[foo];
`
);
expect(transform(source)).toBe(source);
});
});
37 changes: 36 additions & 1 deletion packages/babel-plugin-minify-guarded-expressions/src/index.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,39 @@
"use strict";

function getBlockPath(path) {
var parent = path.findParent(function(p) {
return p.isBlockStatement();
});
// return program path if null
return parent !== null ? parent : path.getFunctionParent();
}

module.exports = function({ types: t }) {
const flipExpressions = require("babel-helper-flip-expressions")(t);

// identify defined check patterns like a && a.foo
// a && a['foo']
// should bail in this case if declaration is inside other block
function isDefinedCheck(path, left, right) {
if (t.isIdentifier(left) && t.isMemberExpression(right)) {
const binding = path.scope.getBinding(left.node.name);
// should bail only for var
if (binding === null || binding.kind !== "var") {
return false;
}
const expressionParent = getBlockPath(path);
const bindingParent = getBlockPath(binding.path);
// bail if referenced in the same block
if (expressionParent !== bindingParent) {
return true;
}

const { object } = right.node;
return left.node.name !== object.name;
}
return false;
}

return {
name: "minify-guarded-expressions",
visitor: {
Expand Down Expand Up @@ -32,7 +63,11 @@ module.exports = function({ types: t }) {
if (leftTruthy === false) {
// Short-circuit
path.replaceWith(node.left);
} else if (leftTruthy === true && left.isPure()) {
} else if (
leftTruthy === true &&
left.isPure() &&
!isDefinedCheck(path, left, right)
) {
path.replaceWith(node.right);
} else if (
right.evaluateTruthy() === false &&
Expand Down

0 comments on commit 184cede

Please sign in to comment.