Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: Support pure expressions in transform-react-constant-elements #4812

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -1,4 +1,4 @@
export default function () {
export default function ({ types: t }) {
const immutabilityVisitor = {
enter(path, state) {
const stop = () => {
Expand All @@ -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()) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pure template literals such as

<a className={`ok`}/>

is missed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yiminghe isnt it covered already by isPure here?

Copy link

@yiminghe yiminghe Oct 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the latest version seems to have template literals check: #5914

while the current stable version babel-traverse@6.26.0 does not @Andarist

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();
}
}
};

Expand Down
@@ -0,0 +1,5 @@
// https://github.com/facebook/react/issues/3226
// Not safe to reuse because it is mutable
function render() {
return <div style={{ width: 100 }} />;
}
@@ -0,0 +1,5 @@
// https://github.com/facebook/react/issues/3226
// Not safe to reuse because it is mutable
function render() {
return <div style={{ width: 100 }} />;
}
@@ -0,0 +1,5 @@
function render(offset) {
return function () {
return <div tabIndex={offset + 1} />;
};
}
@@ -0,0 +1,8 @@
function render(offset) {
var _ref = <div tabIndex={offset + 1} />;

return function () {
return _ref;
};
}

@@ -0,0 +1,10 @@
const OFFSET = 3;

var Foo = React.createClass({
render: function () {
return (
<div tabIndex={OFFSET + 1} />
);
}
});

@@ -0,0 +1,10 @@
const OFFSET = 3;

var _ref = <div tabIndex={OFFSET + 1} />;

var Foo = React.createClass({
render: function () {
return _ref;
}
});

@@ -0,0 +1,11 @@
var Foo = React.createClass({
render: function () {
return (
<div data-text={
"Some text, " +
"and some more too."
} />
);
}
});

@@ -0,0 +1,8 @@
var _ref = <div data-text={"Some text, " + "and some more too."} />;

var Foo = React.createClass({
render: function () {
return _ref;
}
});