From e0b45436013b92a5280e374274a855773e9bf558 Mon Sep 17 00:00:00 2001 From: Samuel Reed Date: Tue, 25 Jul 2017 13:34:21 -0500 Subject: [PATCH] feature: Support whitelisting mutable props for react-constant-elements (#5307) --- .../README.md | 33 ++++++++++++++++++- .../src/index.js | 27 ++++++++++++++- .../pure-expression-whitelist-deopt/actual.js | 20 +++++++++++ .../expected.js | 9 +++++ .../options.json | 7 ++++ .../actual.js | 20 +++++++++++ .../expected.js | 11 +++++++ .../options.json | 7 ++++ .../pure-expression-whitelist/actual.js | 18 ++++++++++ .../pure-expression-whitelist/expected.js | 9 +++++ .../pure-expression-whitelist/options.json | 7 ++++ 11 files changed, 166 insertions(+), 2 deletions(-) create mode 100644 packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-deopt/actual.js create mode 100644 packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-deopt/expected.js create mode 100644 packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-deopt/options.json create mode 100644 packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-member/actual.js create mode 100644 packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-member/expected.js create mode 100644 packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-member/options.json create mode 100644 packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist/actual.js create mode 100644 packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist/expected.js create mode 100644 packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist/options.json diff --git a/packages/babel-plugin-transform-react-constant-elements/README.md b/packages/babel-plugin-transform-react-constant-elements/README.md index 4754c94534..881f2d23bb 100644 --- a/packages/babel-plugin-transform-react-constant-elements/README.md +++ b/packages/babel-plugin-transform-react-constant-elements/README.md @@ -1,6 +1,9 @@ # babel-plugin-transform-react-constant-elements -> Treat React JSX elements as value types and hoist them to the highest scope +> Treat React JSX elements as value types and hoist them to the highest scope. + +This plugin can speed up reconciliation and reduce garbage collection pressure by hoisting +React elements to the highest possible scope, preventing multiple unnecessary reinstantiations. ## Example @@ -37,6 +40,14 @@ const Hr = () => {
this.node = node} /> ``` +- **Mutable Properties** + +> See https://github.com/facebook/react/issues/3226 for more on this + + ```js +
+ ``` + ## Installation ```sh @@ -55,6 +66,26 @@ npm install --save-dev babel-plugin-transform-react-constant-elements } ``` +## Options + +### `allowMutablePropsOnTags` + +`Array`, defaults to `[]` + +If you are using a particular library (like react-intl) that uses object properties, and you are sure +that the element won't modify its own props, you can whitelist the element so that objects are allowed. + +This will skip the `Mutable Properties` deopt. + +```json +{ + "plugins": [ + ["transform-react-constant-elements", {"allowMutablePropsOnTags": ["FormattedMessage"]}], + ] +} + +``` + ### Via CLI ```sh 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 dea8e97e7b..b6bfaee01d 100644 --- a/packages/babel-plugin-transform-react-constant-elements/src/index.js +++ b/packages/babel-plugin-transform-react-constant-elements/src/index.js @@ -41,10 +41,12 @@ export default function({ types: t }) { // We know the result; check its mutability. const { value } = expressionResult; const isMutable = - (value && typeof value === "object") || + (!state.mutablePropsAllowed && + (value && typeof value === "object")) || typeof value === "function"; if (!isMutable) { // It evaluated to an immutable value, so we can hoist it. + path.skip(); return; } } else if (t.isIdentifier(expressionResult.deopt)) { @@ -65,6 +67,29 @@ export default function({ types: t }) { HOISTED.add(path.node); const state = { isImmutable: true }; + + // This transform takes the option `allowMutablePropsOnTags`, which is an array + // of JSX tags to allow mutable props (such as objects, functions) on. Use sparingly + // and only on tags you know will never modify their own props. + if (this.opts.allowMutablePropsOnTags != null) { + if (!Array.isArray(this.opts.allowMutablePropsOnTags)) { + throw new Error( + ".allowMutablePropsOnTags must be an array, null, or undefined.", + ); + } + // Get the element's name. If it's a member expression, we use the last part of the path. + // So the option ["FormattedMessage"] would match "Intl.FormattedMessage". + let namePath = path.get("openingElement.name"); + while (namePath.isJSXMemberExpression()) { + namePath = namePath.get("property"); + } + + const elementName = namePath.node.name; + state.mutablePropsAllowed = + this.opts.allowMutablePropsOnTags.indexOf(elementName) > -1; + } + + // Traverse all props passed to this element for immutability. path.traverse(immutabilityVisitor, state); if (state.isImmutable) { diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-deopt/actual.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-deopt/actual.js new file mode 100644 index 0000000000..e470a4b105 --- /dev/null +++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-deopt/actual.js @@ -0,0 +1,20 @@ +// This is the same as the pure-expression-whitelist test, but 'FormattedMessage' is *not* whitelisted +// so it should not be hoisted. +var Foo = React.createClass({ + render: function () { + return ( + + ); + } +}); + diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-deopt/expected.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-deopt/expected.js new file mode 100644 index 0000000000..b893f67737 --- /dev/null +++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-deopt/expected.js @@ -0,0 +1,9 @@ +// This is the same as the pure-expression-whitelist test, but 'FormattedMessage' is *not* whitelisted +// so it should not be hoisted. +var Foo = React.createClass({ + render: function () { + return ; + } +}); diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-deopt/options.json b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-deopt/options.json new file mode 100644 index 0000000000..61a6bd63f4 --- /dev/null +++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-deopt/options.json @@ -0,0 +1,7 @@ +{ + + "plugins": [ + ["transform-react-constant-elements", {"allowMutablePropsOnTags": ["FormattedNumber"]}], + "syntax-jsx" + ] +} diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-member/actual.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-member/actual.js new file mode 100644 index 0000000000..702732d1be --- /dev/null +++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-member/actual.js @@ -0,0 +1,20 @@ +import Intl from 'react-intl'; + +var Foo = React.createClass({ + render: function () { + return ( + + ); + } +}); + diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-member/expected.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-member/expected.js new file mode 100644 index 0000000000..c461d150f4 --- /dev/null +++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-member/expected.js @@ -0,0 +1,11 @@ +import Intl from 'react-intl'; + +var _ref = ; + +var Foo = React.createClass({ + render: function () { + return _ref; + } +}); \ No newline at end of file diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-member/options.json b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-member/options.json new file mode 100644 index 0000000000..f690dd8c34 --- /dev/null +++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist-member/options.json @@ -0,0 +1,7 @@ +{ + + "plugins": [ + ["transform-react-constant-elements", {"allowMutablePropsOnTags": ["FormattedMessage"]}], + "syntax-jsx" + ] +} diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist/actual.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist/actual.js new file mode 100644 index 0000000000..9cf74bd310 --- /dev/null +++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist/actual.js @@ -0,0 +1,18 @@ +var Foo = React.createClass({ + render: function () { + return ( + + ); + } +}); + diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist/expected.js b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist/expected.js new file mode 100644 index 0000000000..13603d6061 --- /dev/null +++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist/expected.js @@ -0,0 +1,9 @@ +var _ref = ; + +var Foo = React.createClass({ + render: function () { + return _ref; + } +}); \ No newline at end of file diff --git a/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist/options.json b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist/options.json new file mode 100644 index 0000000000..f690dd8c34 --- /dev/null +++ b/packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/pure-expression-whitelist/options.json @@ -0,0 +1,7 @@ +{ + + "plugins": [ + ["transform-react-constant-elements", {"allowMutablePropsOnTags": ["FormattedMessage"]}], + "syntax-jsx" + ] +}