Ensure left is evaluated before right

jridgewell · jridgewell · commit 82d8aded8ed4 · 2017-09-29T00:55:45.000-04:00
diff --git a/packages/babel-plugin-transform-pipeline-operator/src/index.js b/packages/babel-plugin-transform-pipeline-operator/src/index.js
@@ -6,46 +6,40 @@ export default function({ types: t }) {
 
     visitor: {
       BinaryExpression(path) {
-        const { operator, left, right } = path.node;
+        const { scope, parentPath } = path;
+        const { node } = path;
+        const { operator, left, right } = node;
         if (operator !== "|>") return;
 
-        if (
+        // Why do I have to fix this here?!
+        if (parentPath.isArrowFunctionExpression({ body: node })) {
+          path.replaceWith(t.blockStatement([t.returnStatement(node)]));
+          return;
+        }
+
+        const optimizeArrow =
           t.isArrowFunctionExpression(right) &&
           right.params.length === 1 &&
           t.isIdentifier(right.params[0]) &&
-          t.isExpression(right.body)
-        ) {
-          //
-          // Optimize away arrow function!
-          //
-          // Converts:
-          //    arg |> x => x + x;
-          // To:
-          //    (_x = arg, _x + _x);
-          //
-          const paramName = right.params[0].name;
-          const placeholder = path.scope.generateDeclaredUidIdentifier(
-            paramName,
-          );
+          t.isExpression(right.body);
 
-          path.get("right").scope.rename(paramName, placeholder.name);
-          path.replaceWith(
-            t.sequenceExpression([
-              t.assignmentExpression("=", placeholder, left),
-              right.body,
-            ]),
-          );
-        } else {
-          //
-          // Simple invocation.
-          //
-          // Converts:
-          //    x |> obj.f;
-          // To:
-          //    obj.f(x);
-          //
-          path.replaceWith(t.callExpression(right, [left]));
+        const param = optimizeArrow ? right.params[0] : left;
+        const placeholder = scope.generateUidIdentifierBasedOnNode(param);
+        scope.push({ id: placeholder });
+
+        if (optimizeArrow) {
+          path.get("right").scope.rename(param.name, placeholder.name);
         }
+
+        const call = optimizeArrow
+          ? right.body
+          : t.callExpression(right, [placeholder]);
+        path.replaceWith(
+          t.sequenceExpression([
+            t.assignmentExpression("=", placeholder, left),
+            call,
+          ]),
+        );
       },
     },
   };
diff --git a/packages/babel-plugin-transform-pipeline-operator/test/fixtures/pipeline-operator/arrow-functions/expected.js b/packages/babel-plugin-transform-pipeline-operator/test/fixtures/pipeline-operator/arrow-functions/expected.js
@@ -1,11 +1,15 @@
-var _2, _3, _sum;
+var _ref, _ref2, _sum;
 
-var result = (_2 = [5, 10], (_3 = _2.map(x => x * 2), (_sum = _3.reduce((a, b) => a + b), _sum + 1)));
+var result = (_ref = [5, 10], (_ref2 = _ref.map(x => x * 2), (_sum = _ref2.reduce((a, b) => a + b), _sum + 1)));
 assert.equal(result, 31);
 
 var inc = x => x + 1;
 
 var double = x => x * 2;
 
-var result2 = [4, 9].map(x => double(inc(x)));
+var result2 = [4, 9].map(x => {
+  var _ref3, _x;
+
+  return _ref3 = (_x = x, inc(_x)), double(_ref3);
+});
 assert.deepEqual(result2, [10, 20]);
diff --git a/packages/babel-plugin-transform-pipeline-operator/test/fixtures/pipeline-operator/basic/expected.js b/packages/babel-plugin-transform-pipeline-operator/test/fixtures/pipeline-operator/basic/expected.js
@@ -1,3 +1,5 @@
+var _;
+
 var inc = x => x + 1;
 
-assert.equal(inc(10), 11);
+assert.equal((_ = 10, inc(_)), 11);
diff --git a/packages/babel-plugin-transform-pipeline-operator/test/fixtures/pipeline-operator/chaining/expected.js b/packages/babel-plugin-transform-pipeline-operator/test/fixtures/pipeline-operator/chaining/expected.js
@@ -1,5 +1,7 @@
+var _ref, _;
+
 var inc = x => x + 1;
 
 var double = x => x * 2;
 
-assert.equal(double(inc(10)), 22);
+assert.equal((_ref = (_ = 10, inc(_)), double(_ref)), 22);
diff --git a/packages/babel-plugin-transform-pipeline-operator/test/fixtures/pipeline-operator/currying/expected.js b/packages/babel-plugin-transform-pipeline-operator/test/fixtures/pipeline-operator/currying/expected.js
@@ -1,4 +1,6 @@
+var _ref;
+
 var map = fn => array => array.map(fn);
 
-var result = map(x => x * 20)([10, 20]);
+var result = (_ref = [10, 20], map(x => x * 20)(_ref));
 assert.deepEqual(result, [200, 400]);
diff --git a/packages/babel-plugin-transform-pipeline-operator/test/fixtures/pipeline-operator/evaluation/actual.js b/packages/babel-plugin-transform-pipeline-operator/test/fixtures/pipeline-operator/evaluation/actual.js
@@ -0,0 +1,13 @@
+var obj = {
+  get prop() {
+    return this._prop = 1;
+  },
+
+  get method() {
+    if (!this._prop) throw new Error('invalid evaluation order');
+    return (v) => v;
+  }
+}
+
+var result = obj.prop |> obj.method;
+assert.equal(result, 1);
diff --git a/packages/babel-plugin-transform-pipeline-operator/test/fixtures/pipeline-operator/evaluation/exec.js b/packages/babel-plugin-transform-pipeline-operator/test/fixtures/pipeline-operator/evaluation/exec.js
@@ -0,0 +1,13 @@
+var obj = {
+  get prop() {
+    return this._prop = 1;
+  },
+
+  get method() {
+    if (!this._prop) throw new Error('invalid evaluation order');
+    return (v) => v;
+  }
+}
+
+var result = obj.prop |> obj.method;
+assert.equal(result, 1);
diff --git a/packages/babel-plugin-transform-pipeline-operator/test/fixtures/pipeline-operator/evaluation/expected.js b/packages/babel-plugin-transform-pipeline-operator/test/fixtures/pipeline-operator/evaluation/expected.js
@@ -0,0 +1,15 @@
+var _obj$prop;
+
+var obj = {
+  get prop() {
+    return this._prop = 1;
+  },
+
+  get method() {
+    if (!this._prop) throw new Error('invalid evaluation order');
+    return v => v;
+  }
+
+};
+var result = (_obj$prop = obj.prop, obj.method(_obj$prop));
+assert.equal(result, 1);
diff --git a/packages/babel-plugin-transform-pipeline-operator/test/fixtures/pipeline-operator/precedence/expected.js b/packages/babel-plugin-transform-pipeline-operator/test/fixtures/pipeline-operator/precedence/expected.js
@@ -1,11 +1,13 @@
+var _ref, _ref2, _;
+
 var inc = x => x + 1;
 
-var result = inc(4 || 9);
+var result = (_ref = 4 || 9, inc(_ref));
 assert.equal(result, 5);
 
 var f = x => x + 10;
 
 var h = x => x + 20;
 
-var result2 = inc((f || h)(10));
+var result2 = (_ref2 = (_ = 10, (f || h)(_)), inc(_ref2));
 assert.equal(result2, 21);
