From 87241357f460832bcd23d5260d33e4ab9a6befb6 Mon Sep 17 00:00:00 2001
From: Samuel Reed <samuel.trace.reed@gmail.com>
Date: Mon, 16 Jan 2017 14:17:40 -0600
Subject: [PATCH] Fix PathHoister hoisting JSX member expressions on "this".

The PathHoister ignored member references on "this", causing it
to potentially hoist an expression above its function scope.

This patch tells the hoister to watch for "this", and if seen,
mark the nearest non-arrow function scope as the upper limit
for hoistng.

This fixes #4397 and is an alternative to #4787.
---
 .../member-expression-constant/actual.js         |  4 ++++
 .../member-expression-constant/expected.js       |  7 +++++++
 .../member-expression-this/actual.js             |  5 +++++
 .../member-expression-this/expected.js           | 12 ++++++++++++
 .../member-expression-this/options.json          |  3 +++
 .../member-expression/actual.js                  |  6 ++++++
 .../member-expression/expected.js                | 16 ++++++++++++++++
 .../member-expression/options.json               |  3 +++
 packages/babel-traverse/src/path/lib/hoister.js  | 14 +++++++++++++-
 9 files changed, 69 insertions(+), 1 deletion(-)
 create mode 100644 packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression-constant/actual.js
 create mode 100644 packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression-constant/expected.js
 create mode 100644 packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression-this/actual.js
 create mode 100644 packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression-this/expected.js
 create mode 100644 packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression-this/options.json
 create mode 100644 packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression/actual.js
 create mode 100644 packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression/expected.js
 create mode 100644 packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression/options.json

diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression-constant/actual.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression-constant/actual.js
new file mode 100644
index 000000000000..a6c1f9b6ec88
--- /dev/null
+++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression-constant/actual.js
@@ -0,0 +1,4 @@
+function render() {
+  this.component = "div";
+  return () => <this.component />;
+}
diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression-constant/expected.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression-constant/expected.js
new file mode 100644
index 000000000000..aa7a9994baeb
--- /dev/null
+++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression-constant/expected.js
@@ -0,0 +1,7 @@
+function render() {
+  this.component = "div";
+
+  var _ref = <this.component />;
+
+  return () => _ref;
+}
diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression-this/actual.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression-this/actual.js
new file mode 100644
index 000000000000..bdc61d684a69
--- /dev/null
+++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression-this/actual.js
@@ -0,0 +1,5 @@
+class Component extends React.Component {
+  subComponent = () => <span>Sub Component</span>
+
+  render = () => <this.subComponent />
+}
diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression-this/expected.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression-this/expected.js
new file mode 100644
index 000000000000..9ce3a08e7437
--- /dev/null
+++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression-this/expected.js
@@ -0,0 +1,12 @@
+var _ref = <span>Sub Component</span>;
+
+class Component extends React.Component {
+  constructor(...args) {
+    var _temp;
+
+    var _ref2 = <this.subComponent />;
+
+    return _temp = super(...args), this.subComponent = () => _ref, this.render = () => _ref2, _temp;
+  }
+
+}
diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression-this/options.json b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression-this/options.json
new file mode 100644
index 000000000000..d4789bbda3fb
--- /dev/null
+++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression-this/options.json
@@ -0,0 +1,3 @@
+{
+  "plugins": ["syntax-jsx", "transform-react-constant-elements", "transform-class-properties"]
+}
diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression/actual.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression/actual.js
new file mode 100644
index 000000000000..2ab59df97b89
--- /dev/null
+++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression/actual.js
@@ -0,0 +1,6 @@
+const els = {
+  subComponent: () => <span>Sub Component</span>
+};
+class Component extends React.Component {
+  render = () => <els.subComponent />
+}
diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression/expected.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression/expected.js
new file mode 100644
index 000000000000..fe12eb988d5d
--- /dev/null
+++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression/expected.js
@@ -0,0 +1,16 @@
+var _ref = <span>Sub Component</span>;
+
+const els = {
+  subComponent: () => _ref
+};
+
+var _ref2 = <els.subComponent />;
+
+class Component extends React.Component {
+  constructor(...args) {
+    var _temp;
+
+    return _temp = super(...args), this.render = () => _ref2, _temp;
+  }
+
+}
diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression/options.json b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression/options.json
new file mode 100644
index 000000000000..d4789bbda3fb
--- /dev/null
+++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/member-expression/options.json
@@ -0,0 +1,3 @@
+{
+  "plugins": ["syntax-jsx", "transform-react-constant-elements", "transform-class-properties"]
+}
diff --git a/packages/babel-traverse/src/path/lib/hoister.js b/packages/babel-traverse/src/path/lib/hoister.js
index 8a36031f1e79..3e992e4408cf 100644
--- a/packages/babel-traverse/src/path/lib/hoister.js
+++ b/packages/babel-traverse/src/path/lib/hoister.js
@@ -2,11 +2,23 @@ import { react } from "babel-types";
 import * as t from "babel-types";
 
 const referenceVisitor = {
+  // This visitor looks for bindings to establish a topmost scope for hoisting.
   ReferencedIdentifier(path, state) {
-    if (path.isJSXIdentifier() && react.isCompatTag(path.node.name)) {
+    // Don't hoist regular JSX identifiers ('div', 'span', etc).
+    // We do have to consider member expressions for hoisting (e.g. `this.component`)
+    if (path.isJSXIdentifier() && react.isCompatTag(path.node.name) && !path.parentPath.isJSXMemberExpression()) {
       return;
     }
 
+    // If the identifier refers to `this`, we need to break on the closest non-arrow scope.
+    if (path.node.name === "this") {
+      let scope = path.scope;
+      do {
+        if (scope.path.isFunction() && !scope.path.isArrowFunctionExpression()) break;
+      } while (scope = scope.parent);
+      if (scope) state.breakOnScopePaths.push(scope.path);
+    }
+
     // direct references that we need to track to hoist this to the highest scope we can
     const binding = path.scope.getBinding(path.node.name);
     if (!binding) return;