Add conditional expression optimizations

And a _ton_ of runtime tests to make sure I get it right

Author: jridgewell

Diff for: src/helpers/is-child-element.js

@@ -52,10 +52,16 @@ function ancestorPath(path, useFastRoot) {
       if (last !== child) {
         continue;
       }
-    } else if (path.isConditionalExpression() || path.isLogicalExpression()) {
-      // These expressions "extend" the search, but they do not count as direct children.
-      // That's because the expression could resolve to something other than a JSX element.
+    } else if (path.isConditionalExpression()) {
       continue;
+    } else if (path.isLogicalExpression()) {
+      if (path.get("right") === last || path.node.operator === "||") {
+        // These expressions "extend" the search, but they do not count as direct children.
+        // That's because the expression could resolve to something other than a JSX element.
+        continue;
+      }
+
+      break;
     } else if (!useFastRoot) {
       // In normal mode, nothing else keeps a JSX search going.
       return;

Diff for: test/fixtures/conditional-optimizations/eval.js

@@ -0,0 +1,115 @@
+import { transform } from "babel-core";
+import assert from "assert";
+import plugin from "../../../src/index";
+
+const cases = [
+  [ false, false, false, false ],
+  [ false, false, false, true ],
+  [ false, false, true, false ],
+  [ false, false, true, true ],
+  [ false, true, false, false ],
+  [ false, true, false, true ],
+  [ false, true, true, false ],
+  [ false, true, true, true ],
+  [ true, false, false, false ],
+  [ true, false, false, true ],
+  [ true, false, true, false ],
+  [ true, false, true, true ],
+  [ true, true, false, false ],
+  [ true, true, false, true ],
+  [ true, true, true, false ],
+  [ true, true, true, true ],
+];
+const operations = [
+  ['||', '||', '||'],
+  ['||', '||', '&&'],
+  ['||', '&&', '||'],
+  ['||', '&&', '&&'],
+  ['&&', '||', '||'],
+  ['&&', '||', '&&'],
+  ['&&', '&&', '||'],
+  ['&&', '&&', '&&'],
+];
+const groupings = [
+  [' ', ' ', ' ', ' ', ' ', ' ', ' ', ' '],
+  [' ', ' ', ' ', ' ', '(', ' ', ' ', ')'],
+  [' ', ' ', '(', ' ', ' ', ')', ' ', ' '],
+  ['(', ' ', ' ', ')', ' ', ' ', ' ', ' '],
+  ['(', ' ', ' ', ')', '(', ' ', ' ', ')'],
+  [' ', ' ', '(', ' ', ' ', ' ', ' ', ')'],
+  ['(', ' ', ' ', ' ', ' ', ')', ' ', ' '],
+];
+
+
+let wrappers = 0;
+let passes = 0;
+function elementOpen() { }
+function elementClose() { }
+function renderArbitrary() { }
+function elementVoid() { }
+function jsxWrapper() {
+  wrappers++;
+  return 'wrapper';
+}
+function pass() {
+  passes++;
+  return 'pass';
+}
+
+function mockRequire() {
+  return { renderArbitrary, jsxWrapper };
+}
+
+for (let i = 0; i < cases.length; i++) {
+  const c = cases[i];
+  for (let j = 0; j < c.length; j++) {
+    const bool = c.slice();
+    const jsx = c.slice();
+    bool.splice(j, 1, 'pass()');
+    jsx.splice(j, 1, '<div />');
+
+
+    for (let k = 0; k < operations.length; k++) {
+      const operation = operations[k];
+      for (var l = 0; l < groupings.length; l++) {
+        const grouping = groupings[l];
+
+        test(bool, jsx, operation, grouping);
+      }
+    }
+  }
+}
+
+function test(bool, jsx, operation, grouping) {
+  const boolCode = `${grouping[0]}${bool[0]}${grouping[1]} ${operation[0]} ${grouping[2]}${bool[1]}${grouping[3]} ${operation[1]} ${grouping[4]}${bool[2]}${grouping[5]} ${operation[2]} ${grouping[6]}${bool[3]}${grouping[7]}`;
+  const jsxCode =  `${grouping[0]}${ jsx[0]}${grouping[1]} ${operation[0]} ${grouping[2]}${ jsx[1]}${grouping[3]} ${operation[1]} ${grouping[4]}${ jsx[2]}${grouping[5]} ${operation[2]} ${grouping[6]}${ jsx[3]}${grouping[7]}`;
+
+  it(jsxCode, () => {
+    wrappers = 0;
+    passes = 0;
+
+    // When evaluating the case, the return value tells us two things.
+    // If the return value is 1
+    //   - one elementVoid should have been called
+    //   - the evaluation of the jsx code should return "undefined" and not a jsx wrapper
+    // If the return value is false
+    //   - No elementVoid calls
+    //   - the evaluation of the jsx code should return "false"
+    const expected = eval(boolCode);
+    const transformed = transform(`function render() { return <div>{${jsxCode}}</div>; }; render()`, {
+      plugins: [
+        'transform-es2015-modules-commonjs',
+        [plugin, {runtimeModuleSource: 'test'}]
+      ]
+    }).code;
+
+    const require = mockRequire;
+    const result = eval(transformed);
+
+    if (expected === 'pass') {
+      assert.equal(wrappers, 0, 'wrapper was created');
+    } else {
+      assert.equal(wrappers, passes, passes === 1 ? 'wrapper was not created' : 'wrapper was created');
+    }
+  });
+}

Diff for: test/index.js

@@ -80,7 +80,9 @@ function test(dir) {
 
 function findTests(root) {
   const files = fs.readdirSync(root);
-  if (files.indexOf("actual.js") > -1) {
+  if (files.indexOf("eval.js") > -1) {
+    require(path.join(root, "eval.js"));
+  } else if (files.indexOf("actual.js") > -1) {
     it(path.basename(root), () => test(root));
   } else {
     files.forEach((file) => {