Skip to content

Commit

Permalink
fix: Don't hoist JSX elements referencing mutable variables (#13054)
Browse files Browse the repository at this point in the history
issue-13051

Co-authored-by: Clint Goodman <clintgoodman@workfront.com>
  • Loading branch information
cgood92 and Clint Goodman committed Mar 25, 2021
1 parent 0067fd9 commit 266e077
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 10 deletions.
Expand Up @@ -64,13 +64,17 @@ export default declare((api, options) => {
// Ignore identifiers & JSX expressions.
if (
path.isJSXIdentifier() ||
path.isIdentifier() ||
path.isJSXMemberExpression() ||
path.isJSXNamespacedName()
) {
return;
}

if (path.isIdentifier()) {
const binding = path.scope.getBinding(path.node.name);
if (binding && binding.constant) return;
}

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.
Expand Down
@@ -1,5 +1,3 @@
var _span;

let foo = 'hello';

const mutate = () => {
Expand All @@ -8,5 +6,5 @@ const mutate = () => {

export const Component = () => {
if (Math.random() > 0.5) mutate();
return _span || (_span = <span>{foo}</span>);
return <span>{foo}</span>;
};
@@ -1,7 +1,5 @@
var _span;

let foo = 'hello';
export const Component = () => {
foo = 'goodbye';
return _span || (_span = <span>{foo}</span>);
return <span>{foo}</span>;
};
@@ -0,0 +1,10 @@
function render() {
const nodes = [];

for(let i = 0; i < 5; i++) {
nodes.push(<div>{i}</div>)
}

return nodes;
}

@@ -0,0 +1,10 @@
function render() {
const nodes = [];

for (let i = 0; i < 5; i++) {
nodes.push(<div>{i}</div>);
}

return nodes;
}

@@ -1,7 +1,5 @@
var _div;

var Foo = React.createClass({
render: function render() {
return _div || (_div = <div foo={notDeclared}></div>);
return <div foo={notDeclared}></div>;
}
});

0 comments on commit 266e077

Please sign in to comment.