diff --git a/packages/babel-plugin-transform-react-constant-elements/src/index.js b/packages/babel-plugin-transform-react-constant-elements/src/index.js index a25fb67e6c6d..b38f2008d154 100644 --- a/packages/babel-plugin-transform-react-constant-elements/src/index.js +++ b/packages/babel-plugin-transform-react-constant-elements/src/index.js @@ -1,4 +1,4 @@ -export default function () { +export default function ({ types: t }) { const immutabilityVisitor = { enter(path, state) { const stop = () => { @@ -11,15 +11,39 @@ export default function () { return; } + // Elements with refs are not safe to hoist. if (path.isJSXIdentifier({ name: "ref" }) && path.parentPath.isJSXAttribute({ name: path.node })) { return stop(); } + // Ignore identifiers & JSX expressions. if (path.isJSXIdentifier() || path.isIdentifier() || path.isJSXMemberExpression()) { return; } - if (!path.isImmutable()) stop(); + if (!path.isImmutable()) { + // If it's not immutable, it may still be a pure expression, such as string concatenation. + // It is still safe to hoist that, so long as its result is immutable. + // If not, it is not safe to replace as mutable values (like objects) could be mutated after render. + // https://github.com/facebook/react/issues/3226 + if (path.isPure()) { + const expressionResult = path.evaluate(); + if (expressionResult.confident) { + // We know the result; check its mutability. + const { value } = expressionResult; + const isMutable = (value && typeof value === "object") || (typeof value === "function"); + if (!isMutable) { + // It evaluated to an immutable value, so we can hoist it. + return; + } + } else if (t.isIdentifier(expressionResult.deopt)) { + // It's safe to hoist here if the deopt reason is an identifier (e.g. func param). + // The hoister will take care of how high up it can be hoisted. + return; + } + } + stop(); + } } }; diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-deopt/actual.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-deopt/actual.js new file mode 100644 index 000000000000..4df51832c7ce --- /dev/null +++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-deopt/actual.js @@ -0,0 +1,5 @@ +// https://github.com/facebook/react/issues/3226 +// Not safe to reuse because it is mutable +function render() { + return
; +} diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-deopt/expected.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-deopt/expected.js new file mode 100644 index 000000000000..4df51832c7ce --- /dev/null +++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-deopt/expected.js @@ -0,0 +1,5 @@ +// https://github.com/facebook/react/issues/3226 +// Not safe to reuse because it is mutable +function render() { + return
; +} diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-2/actual.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-2/actual.js new file mode 100644 index 000000000000..0acb75cfaf6a --- /dev/null +++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-2/actual.js @@ -0,0 +1,5 @@ +function render(offset) { + return function () { + return
; + }; +} diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-2/expected.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-2/expected.js new file mode 100644 index 000000000000..65df627806b2 --- /dev/null +++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-2/expected.js @@ -0,0 +1,8 @@ +function render(offset) { + var _ref =
; + + return function () { + return _ref; + }; +} + diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-3/actual.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-3/actual.js new file mode 100644 index 000000000000..c6b89c77fd1a --- /dev/null +++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-3/actual.js @@ -0,0 +1,10 @@ +const OFFSET = 3; + +var Foo = React.createClass({ + render: function () { + return ( +
+ ); + } +}); + diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-3/expected.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-3/expected.js new file mode 100644 index 000000000000..e709176c4870 --- /dev/null +++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-3/expected.js @@ -0,0 +1,10 @@ +const OFFSET = 3; + +var _ref =
; + +var Foo = React.createClass({ + render: function () { + return _ref; + } +}); + diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression/actual.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression/actual.js new file mode 100644 index 000000000000..5131c839899b --- /dev/null +++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression/actual.js @@ -0,0 +1,11 @@ +var Foo = React.createClass({ + render: function () { + return ( +
+ ); + } +}); + diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression/expected.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression/expected.js new file mode 100644 index 000000000000..a7afcb1d6edf --- /dev/null +++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression/expected.js @@ -0,0 +1,8 @@ +var _ref =
; + +var Foo = React.createClass({ + render: function () { + return _ref; + } +}); +