diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/append-to-end-when-declared-in-scope-3/expected.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/append-to-end-when-declared-in-scope-3/expected.js
index cceca3a5e8..46849febf6 100644
--- a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/append-to-end-when-declared-in-scope-3/expected.js
+++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/append-to-end-when-declared-in-scope-3/expected.js
@@ -1,18 +1,19 @@
-var _ref =
Parent
;
-
var _ref2 = child
;
+var _ref3 = Parent
;
+
(function () {
class App extends React.Component {
render() {
- return ;
+ return _ref;
}
}
const AppItem = () => {
return _ref2;
- };
-});
+ },
+ _ref = ;
+});
\ No newline at end of file
diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/append-to-end-when-declared-in-scope/expected.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/append-to-end-when-declared-in-scope/expected.js
index 5fc8260845..fe04d4853f 100644
--- a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/append-to-end-when-declared-in-scope/expected.js
+++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/append-to-end-when-declared-in-scope/expected.js
@@ -1,16 +1,14 @@
-var _ref = Parent
;
-
export default class App extends React.Component {
render() {
- return ;
+ return _ref;
}
}
-var _ref2 = child
;
-
-const AppItem = () => {
+const _ref2 = child
,
+ AppItem = () => {
return _ref2;
-};
+},
+ _ref = ;
\ No newline at end of file
diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/dont-hoist-before-class/actual.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/dont-hoist-before-class/actual.js
new file mode 100644
index 0000000000..0bcdd9ef7c
--- /dev/null
+++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/dont-hoist-before-class/actual.js
@@ -0,0 +1,15 @@
+import React from "react";
+
+const Parent = ({}) => (
+
+
+
+);
+
+export default Parent;
+
+let Child = () => (
+
+ ChildTextContent
+
+);
diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/dont-hoist-before-class/expected.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/dont-hoist-before-class/expected.js
new file mode 100644
index 0000000000..6bcd6e0ffb
--- /dev/null
+++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/dont-hoist-before-class/expected.js
@@ -0,0 +1,13 @@
+import React from "react";
+
+const Parent = ({}) => _ref;
+
+export default Parent;
+
+let _ref2 =
+ ChildTextContent
+
,
+ Child = () => _ref2,
+ _ref =
+
+
;
diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/dont-hoist-before-declaration/expected.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/dont-hoist-before-declaration/expected.js
index d85f9f3244..45b91bd296 100644
--- a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/dont-hoist-before-declaration/expected.js
+++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/dont-hoist-before-declaration/expected.js
@@ -8,8 +8,9 @@ function render() {
function render() {
const bar = "bar",
- renderFoo = () => ,
- baz = "baz";
+ renderFoo = () => _ref2,
+ baz = "baz",
+ _ref2 = ;
return renderFoo();
}
\ No newline at end of file
diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/dont-hoist-before-hoc/actual.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/dont-hoist-before-hoc/actual.js
new file mode 100644
index 0000000000..21f7b2ed90
--- /dev/null
+++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/dont-hoist-before-hoc/actual.js
@@ -0,0 +1,18 @@
+import React from "react";
+
+const HOC = component => component;
+
+const Parent = ({}) => (
+
+
+
+);
+
+export default Parent;
+
+let Child = () => (
+
+ ChildTextContent
+
+);
+Child = HOC(Child);
diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/dont-hoist-before-hoc/expected.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/dont-hoist-before-hoc/expected.js
new file mode 100644
index 0000000000..8da0cc5ad1
--- /dev/null
+++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/dont-hoist-before-hoc/expected.js
@@ -0,0 +1,18 @@
+import React from "react";
+
+const HOC = component => component;
+
+const Parent = ({}) => _ref;
+
+export default Parent;
+
+var _ref2 =
+ ChildTextContent
+
;
+
+let Child = () => _ref2;
+Child = HOC(Child);
+
+var _ref =
+
+
;
\ No newline at end of file
diff --git a/packages/babel-traverse/src/path/lib/hoister.js b/packages/babel-traverse/src/path/lib/hoister.js
index c7dbf5cf73..d39dcba7c0 100644
--- a/packages/babel-traverse/src/path/lib/hoister.js
+++ b/packages/babel-traverse/src/path/lib/hoister.js
@@ -27,25 +27,27 @@ const referenceVisitor = {
// eg. it's in a closure etc
if (binding !== state.scope.getBinding(path.node.name)) return;
- if (binding.constant) {
- state.bindings[path.node.name] = binding;
- } else {
- for (const violationPath of (binding.constantViolations: Array)) {
- state.breakOnScopePaths = state.breakOnScopePaths.concat(violationPath.getAncestry());
- }
- }
+ state.bindings[path.node.name] = binding;
}
};
export default class PathHoister {
constructor(path, scope) {
+ // Storage for scopes we can't hoist above.
this.breakOnScopePaths = [];
+ // Storage for bindings that may affect what path we can hoist to.
this.bindings = {};
+ // Storage for eligible scopes.
this.scopes = [];
+ // Our original scope and path.
this.scope = scope;
this.path = path;
+ // By default, we attach as far up as we can; but if we're trying
+ // to avoid referencing a binding, we may have to go after.
+ this.attachAfter = false;
}
+ // A scope is compatible if all required bindings are reachable.
isCompatibleScope(scope) {
for (const key in this.bindings) {
const binding = this.bindings[key];
@@ -57,6 +59,7 @@ export default class PathHoister {
return true;
}
+ // Look through all scopes and push compatible ones.
getCompatibleScopes() {
let scope = this.path.scope;
do {
@@ -66,6 +69,7 @@ export default class PathHoister {
break;
}
+ // deopt: These scopes are set in the visitor on const violations
if (this.breakOnScopePaths.indexOf(scope.path) >= 0) {
break;
}
@@ -73,7 +77,7 @@ export default class PathHoister {
}
getAttachmentPath() {
- const path = this._getAttachmentPath();
+ let path = this._getAttachmentPath();
if (!path) return;
let targetScope = path.scope;
@@ -94,8 +98,18 @@ export default class PathHoister {
// allow parameter references
if (binding.kind === "param") continue;
- // if this binding appears after our attachment point then don't hoist it
- if (this.getAttachmentParentForPath(binding.path).key > path.key) return;
+ // if this binding appears after our attachment point, then we move after it.
+ if (this.getAttachmentParentForPath(binding.path).key > path.key) {
+ this.attachAfter = true;
+ path = binding.path;
+
+ // We also move past any constant violations.
+ for (const violationPath of (binding.constantViolations: Array)) {
+ if (this.getAttachmentParentForPath(violationPath).key > path.key) {
+ path = violationPath;
+ }
+ }
+ }
}
}
@@ -106,11 +120,12 @@ export default class PathHoister {
const scopes = this.scopes;
const scope = scopes.pop();
+ // deopt: no compatible scopes
if (!scope) return;
if (scope.path.isFunction()) {
if (this.hasOwnParamBindings(scope)) {
- // should ignore this scope since it's ourselves
+ // deopt: should ignore this scope since it's ourselves
if (this.scope === scope) return;
// needs to be attached to the body
@@ -129,26 +144,30 @@ export default class PathHoister {
if (scope) return this.getAttachmentParentForPath(scope.path);
}
+ // Find an attachment for this path.
getAttachmentParentForPath(path) {
do {
- if (!path.parentPath ||
- (Array.isArray(path.container) && path.isStatement()) ||
- (
- path.isVariableDeclarator() &&
- path.parentPath.node !== null &&
- path.parentPath.node.declarations.length > 1
- )
- )
+ if (
+ // Beginning of the scope
+ !path.parentPath ||
+ // Has siblings and is a statement
+ (Array.isArray(path.container) && path.isStatement()) ||
+ // Is part of multiple var declarations
+ (path.isVariableDeclarator() &&
+ path.parentPath.node !== null &&
+ path.parentPath.node.declarations.length > 1))
return path;
} while ((path = path.parentPath));
}
+ // Returns true if a scope has param bindings.
hasOwnParamBindings(scope) {
for (const name in this.bindings) {
if (!scope.hasOwnBinding(name)) continue;
const binding = this.bindings[name];
- if (binding.kind === "param") return true;
+ // Ensure constant; without it we could place behind a reassignment
+ if (binding.kind === "param" && binding.constant) return true;
}
return false;
}
@@ -173,7 +192,8 @@ export default class PathHoister {
let uid = attachTo.scope.generateUidIdentifier("ref");
const declarator = t.variableDeclarator(uid, this.path.node);
- attachTo.insertBefore([
+ const insertFn = this.attachAfter ? "insertAfter" : "insertBefore";
+ attachTo[insertFn]([
attachTo.isVariableDeclarator() ? declarator : t.variableDeclaration("var", [declarator])
]);