Skip to content

Commit

Permalink
Fix PathHoister hoisting JSX member expressions on "this".
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
STRML committed Jan 16, 2017
1 parent 292c3ca commit 8724135
Show file tree
Hide file tree
Showing 9 changed files with 69 additions and 1 deletion.
@@ -0,0 +1,4 @@
function render() {
this.component = "div";
return () => <this.component />;
}
@@ -0,0 +1,7 @@
function render() {
this.component = "div";

var _ref = <this.component />;

return () => _ref;
}
@@ -0,0 +1,5 @@
class Component extends React.Component {
subComponent = () => <span>Sub Component</span>

render = () => <this.subComponent />
}
@@ -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;
}

}
@@ -0,0 +1,3 @@
{
"plugins": ["syntax-jsx", "transform-react-constant-elements", "transform-class-properties"]
}
@@ -0,0 +1,6 @@
const els = {
subComponent: () => <span>Sub Component</span>
};
class Component extends React.Component {
render = () => <els.subComponent />
}
@@ -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;
}

}
@@ -0,0 +1,3 @@
{
"plugins": ["syntax-jsx", "transform-react-constant-elements", "transform-class-properties"]
}
14 changes: 13 additions & 1 deletion packages/babel-traverse/src/path/lib/hoister.js
Expand Up @@ -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;
Expand Down

0 comments on commit 8724135

Please sign in to comment.