fix some bugs in PathHoister - fixes babel-plugins/babel-plugin-react-constant-elements#1

- Don't hoist constant elements to the same function as their original paths function parent.
 - Push each violation paths ancestry to the breakOnScopePaths collection to avoid constant hoisting to nested paths.

src/babel/traversal/path/ancestry.js

@@ -8,13 +8,20 @@ import NodePath from "./index";
 
 export function findParent(callback) {
   var path = this;
-  while (path) {
+  while (path = path.parentPath) {
     if (callback(path)) return path;
-    path = path.parentPath;
   }
   return null;
 }
 
+/**
+ * Get the parent function of the current path.
+ */
+
+export function getFunctionParent() {
+  return this.findParent((path) => path.isFunction() || path.isProgram());
+}
+
 /**
  * Walk up the tree until we hit a parent node path in a list.
  */

src/babel/traversal/path/lib/hoister.js

@@ -19,7 +19,7 @@ var referenceVisitor = {
       state.bindings[node.name] = binding;
     } else {
       for (var violationPath of (binding.constantViolations: Array)) {
-        state.breakOnScopePaths.push(violationPath.scope.path);
+        state.breakOnScopePaths = state.breakOnScopePaths.concat(violationPath.getAncestry());
       }
     }
   }
@@ -106,12 +106,15 @@ export default class PathHoister {
 
     this.getCompatibleScopes();
 
-    var path = this.getAttachmentPath();
-    if (!path) return;
+    var attachTo = this.getAttachmentPath();
+    if (!attachTo) return;
 
-    var uid = path.scope.generateUidIdentifier("ref");
+    // don't bother hoisting to the same function as this will cause multiple branches to be evaluated more than once leading to a bad optimisation
+    if (attachTo.getFunctionParent() === this.path.getFunctionParent()) return;
 
-    path.insertBefore([
+    var uid = attachTo.scope.generateUidIdentifier("ref");
+
+    attachTo.insertBefore([
       t.variableDeclaration("var", [
         t.variableDeclarator(uid, this.path.node)
       ])

test/core/fixtures/transformation/optimisation.react.constant-elements/deep-constant-violation/actual.js

@@ -0,0 +1,9 @@
+function render() {
+  var children = <b></b>;
+
+  if (someCondition) {
+    children = <span></span>;
+  }
+
+  return <div>{children}</div>;
+}

test/core/fixtures/transformation/optimisation.react.constant-elements/deep-constant-violation/expected.js

@@ -0,0 +1,15 @@
+"use strict";
+
+var _ref = <b></b>;
+
+var _ref2 = <span></span>;
+
+function render() {
+  var children = _ref;
+
+  if (someCondition) {
+    children = _ref2;
+  }
+
+  return <div>{children}</div>;
+}

test/core/fixtures/transformation/optimisation.react.constant-elements/dont-hoist-to-same-function/actual.js

@@ -0,0 +1,7 @@
+function renderSome(a, b) {
+  if (a) {
+    return <div>{b}</div>
+  } else {
+    return <span>{b}</span>
+  }
+}

test/core/fixtures/transformation/optimisation.react.constant-elements/dont-hoist-to-same-function/expected.js

@@ -0,0 +1,9 @@
+"use strict";
+
+function renderSome(a, b) {
+  if (a) {
+    return <div>{b}</div>;
+  } else {
+    return <span>{b}</span>;
+  }
+}