reflective-bind - npm Package Compare versions

Comparing version 0.0.2 to 0.0.3-rc1

babel/index.js

@@ -248,5 +248,4 @@ /**
     state.canHoist =
-      binding.constant &&
       // Since we have determined that this identifier is bound outside the
-      // scope of the funciton, we must pass this identifier into the hoisted
+      // scope of the function, we must pass this identifier into the hoisted
       // function. This means that we will reference the binding immediately,
@@ -263,3 +262,9 @@ // instead of when the function is called. This will break for cases
       // We must detect this case, and prevent the hoisting from happening.
-      isBindingDefinitelyBeforePath(binding, state.fnPath);
+      isBindingDefinitelyBeforePath(binding, state.fnPath) &&
+      // For all constantViolations, we have to make sure they also come
+      // before the fnPath, otherwise we will bind to the wrong value.
+      (binding.constant ||
+        binding.constantViolations.every(p => {
+          return isPathDefinitelyBeforeOtherPath(p, state.fnPath);
+        }));
 
@@ -326,5 +331,11 @@ if (!state.canHoist) {
 
-  // Returns true iff we are 100% sure the binding is executed before the path.
-  // https://github.com/babel/babel/blob/75808a2d14a5872472eb12ee5135faca4950d57a/packages/babel-traverse/src/path/introspection.js#L216
   function isBindingDefinitelyBeforePath(binding, path) {
+    const isBindingFnDeclaration = t.isFunctionDeclaration(binding.path);
+    /* istanbul ignore if */
+    if (isBindingFnDeclaration && binding.identifier !== binding.path.node.id) {
+      throw new Error(
+        "binding identifier does not match the function declaration id"
+      );
+    }
+
     // If the binding is a function declaration (e.g. function foo() {}),
@@ -334,5 +345,2 @@ // binding.path.scope will get you the scope of the actual function, not
     // binding.path.parentPath.scope instead.
-    const isBindingFnDeclaration =
-      t.isFunctionDeclaration(binding.path) &&
-      binding.identifier === binding.path.node.id;
     const bscope = isBindingFnDeclaration
@@ -348,21 +356,36 @@ ? binding.path.parentPath.scope
 
-    const bindingAncestry = binding.path.getAncestry();
-    const pathAncestry = path.getAncestry();
-    const [bindingAncestorIdx, pathAncestorIdx] = commonAncestorIndices(
-      bindingAncestry,
-      pathAncestry
+    return isPathDefinitelyBeforeOtherPath(binding.path, path);
+  }
+
+  // Returns true iff we are 100% sure checkPath is executed before otherPath.
+  // https://github.com/babel/babel/blob/75808a2d14a5872472eb12ee5135faca4950d57a/packages/babel-traverse/src/path/introspection.js#L216
+  function isPathDefinitelyBeforeOtherPath(checkPath, otherPath) {
+    const checkPathAncestry = checkPath.getAncestry();
+    const otherPathAncestry = otherPath.getAncestry();
+    const [checkPathAncestorIdx, otherPathAncestorIdx] = commonAncestorIndices(
+      checkPathAncestry,
+      otherPathAncestry
     );
-    if (bindingAncestorIdx === 0) {
-      // This means that the path is part of the binding. An example of this is
-      // a recursive function:
+
+    if (checkPathAncestorIdx === 0) {
+      // This means that checkPath is a direct ancestor of otherPath. An
+      // example of this is a recursive function:
       // const a = () => a();
-      // This means the binding is not created before the path is executed.
+      // This means checkPath is not executed before the otherPath is executed.
       return false;
+    } else if (otherPathAncestorIdx === 0) {
+      // This means that checkPath is a direct ancestor of otherPath.
+      // In our current use case of this function, this means that the
+      // arrow function we want to hoist (otherPath) is a direct ancestor
+      // of the declaration / reassignment (checkPath). This means that we are
+      // either declaring, or reassigning, an identifier inside the function,
+      // which means we can't hoist the arrow function.
+      return false;
     }
-    /* istanbul ignore if */
-    if (bindingAncestorIdx < 0 || pathAncestorIdx <= 0) {
-      // This means that there is no common ancestor, or path is the ancestor
-      // of binding. Neither case is valid for us.
+
+    /* instanbul ignore if */
+    if (checkPathAncestorIdx < 0 || otherPathAncestorIdx <= 0) {
+      // This means that there is no common ancestor, which should never happen.
       throw new Error(
-        `Invalid ancestor indices [${bindingAncestorIdx}, ${pathAncestorIdx}]`
+        `Invalid ancestor indices [${checkPathAncestorIdx}, ${otherPathAncestorIdx}]`
       );
@@ -373,8 +396,10 @@ }
     // one is executed first.
-    const bindingRelationship = bindingAncestry[bindingAncestorIdx - 1];
-    const pathRelationship = pathAncestry[pathAncestorIdx - 1];
+    const checkPathRelationship = checkPathAncestry[checkPathAncestorIdx - 1];
+    const otherPathRelationship = otherPathAncestry[otherPathAncestorIdx - 1];
     /* istanbul ignore if */
-    if (!bindingRelationship || !pathRelationship) {
+    if (!checkPathRelationship || !otherPathRelationship) {
       // This should never happen.
-      throw new Error("Invalid binding or path relationship!");
+      throw new Error(
+        "Invalid checkPathRelationship or otherPathRelationship!"
+      );
     }
@@ -384,5 +409,5 @@
     // gives you the index in the container.
-    if (bindingRelationship.listKey && pathRelationship.listKey) {
+    if (checkPathRelationship.listKey && otherPathRelationship.listKey) {
       /* istanbul ignore if */
-      if (bindingRelationship.container !== pathRelationship.container) {
+      if (checkPathRelationship.container !== otherPathRelationship.container) {
         // This should never happen.
@@ -393,31 +418,26 @@ throw new Error("Relationships not in the same container!");
       return (
-        isBindingFnDeclaration || bindingRelationship.key < pathRelationship.key
+        t.isFunctionDeclaration(checkPath) ||
+        checkPathRelationship.key < otherPathRelationship.key
       );
     }
 
-    // Otherwise, use the visitor order to determine which relationshio is
+    // Otherwise, use the visitor order to determine which relationship is
     // executed first.
-    const commonAncestorType = bindingAncestry[bindingAncestorIdx].type;
+    const commonAncestorType = checkPathAncestry[checkPathAncestorIdx].type;
     const visitorKeys = t.VISITOR_KEYS[commonAncestorType];
-    const bindingPosition = visitorKeys.indexOf(
-      bindingRelationship.listKey || bindingRelationship.key
+    const checkPathPosition = visitorKeys.indexOf(
+      checkPathRelationship.listKey || checkPathRelationship.key
     );
-    const pathPosition = visitorKeys.indexOf(
-      pathRelationship.listKey || pathRelationship.key
+    const otherPathPosition = visitorKeys.indexOf(
+      otherPathRelationship.listKey || otherPathRelationship.key
     );
     /* istanbul ignore if */
-    if (bindingPosition < 0) {
-      throw new Error(`Invalid bindingPosition ${bindingPosition}`);
+    if (checkPathPosition < 0) {
+      throw new Error(`Invalid checkPathRelationship ${checkPathPosition}`);
     }
     /* istanbul ignore if */
-    if (pathPosition < 0) {
-      throw new Error(`Invalid pathPosition ${pathPosition}`);
+    if (otherPathPosition < 0) {
+      throw new Error(`Invalid otherPathPosition ${otherPathPosition}`);
     }
-    /* istanbul ignore if */
-    if (bindingPosition >= pathPosition) {
-      throw new Error(
-        "Binding does not occur before path in visitor key order!"
-      );
-    }
-    return true;
+    return checkPathPosition < otherPathPosition;
   }
@@ -492,3 +512,3 @@
   function nodesDefinitelyEqual(node1, node2) {
-    /* istanbul ignore else  */
+    /* istanbul ignore else */
     if (node1.type !== node2.type) {
@@ -547,3 +567,3 @@ return false;
   function addToHoistPath(node) {
-    /* istanbul ignore else  */
+    /* istanbul ignore else */
     if (_hoistPath.node.body && _hoistPath.node.body.length) {
@@ -550,0 +570,0 @@ node.leadingComments = _hoistPath.node.body[0].leadingComments;

CHANGELOG.md

@@ -1,2 +0,2 @@
-# Mutation Sentinel Change Log
+# Reflective Bind Change Log
 
@@ -6,1 +6,9 @@ All notable changes to this project will be documented in this file.
 ## Unreleased
+
+## 0.0.3
+
+- Support non-constant reference in arrow function as long as there is no reassignment to the variable after the arrow function.
+
+## 0.0.2
+
+Initial release

package.json

 {
   "name": "reflective-bind",
-  "version": "0.0.2",
+  "version": "0.0.3-rc1",
   "description": "Eliminate wasteful re-rendering in React components caused by inline functions",
@@ -5,0 +5,0 @@ "author": "Dounan Shi",

README.md

@@ -6,7 +6,7 @@ [![Build Status](https://travis-ci.org/flexport/reflective-bind.svg?branch=master)](https://travis-ci.org/flexport/reflective-bind)
 
-The `reflective-bind/babel` plugin enables you freely use inline arrow functions in the render method of React components without worrying about deoptimizing pure components.
+In React, using inline functions (arrow functions and `Function.prototype.bind`) in render will [cause pure components to wastefully re-render]((https://flexport.engineering/optimizing-react-rendering-part-1-9634469dca02)). As a result, many React developers encourage you to [never use inline functions](https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-no-bind.md) in render. However, others think that [avoiding them is premature optimization](https://cdb.reacttraining.com/react-inline-functions-and-performance-bdff784f5578).
 
-## Motivation
+With reflective-bind, you can freely use inline functions in render without worrying about wasteful re-rendering of pure components.
 
-Using inline functions (arrow functions and `Function.prototype.bind`) in render will [deoptimize pure child components]((https://flexport.engineering/optimizing-react-rendering-part-1-9634469dca02)). As a result, many React developers encourage you to [never use inline functions](https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-no-bind.md) in your render function. However, others think that [avoiding them is premature optimization](https://cdb.reacttraining.com/react-inline-functions-and-performance-bdff784f5578). With reflective bind you can use inline functions and have optimized pure components.
+The best part is, it requires almost no code change 🙌
 
@@ -21,3 +21,3 @@ ## Installation
 
-Add it to the top of your plugin list in `.babelrc` (just has to come before other plugins that transform arrow functions and `bind` calls):
+Add it to the top of your plugin list in `.babelrc` (it must be run before other plugins that transform arrow functions and `bind` calls):
 
@@ -31,3 +31,3 @@ ```
 
-And implement `shouldComponentUpdate` in your component:
+And call reflective bind’s `shouldComponentUpdate` helper function in your component:
 
@@ -59,3 +59,3 @@ ```js
 
-If for some reason you want the babel plugin to skip processing a specific file, add the following to the file.
+If you do not want the babel plugin to process a specific file, add the following line to your file:
 
@@ -71,3 +71,3 @@ ```js
 
-The plugin simply transforms inline functions into calls to `reflectiveBind`, and the `shouldComponentUpdate` helper function uses `reflectiveEqual` in the shallow comparison equality check.
+The plugin simply transforms inline functions into calls to `reflectiveBind`. This then allows the `shouldComponentUpdate` helper function to use `reflectiveEqual` in the shallow comparison equality check.
 
@@ -128,4 +128,6 @@ ## Using reflectiveBind manually
 
-The following examples of inline functions can all be transformed into calls to `reflectiveBind`:
+The following are examples of some inline functions that will be transformed into calls to `reflectiveBind` by the babel plugin:
 
+- Inline arrow functions:
+
 ```js
@@ -138,5 +140,6 @@ function MyComponent(props) {
 
+- `Function.prototype.bind`:
+
 ```js
 function MyComponent(props) {
-  // Supports Function.prototype.bind
   const handleClick = props.callback.bind(undefined, "yay");
@@ -147,5 +150,6 @@ return <PureChild onClick={handleClick} />
 
+- Multiple assignments / reassignments:
+
 ```js
 function MyComponent(props) {
-  // Supports multiple assignments / reassignments
   let handleClick = () => {...};
@@ -163,5 +167,6 @@
 
+- Ternary expressions:
+
 ```js
 function MyComponent(props) {
-  // Supports ternary expressions
   const handleClick = props.condition
@@ -175,24 +180,20 @@ ? () => {...}
 
+- For maximum optimization, avoid accessing nested attributes in your arrow function. Prefer to pull the nested value out to a const and close over it in your arrow function.
+
 ```js
-class MyComponent extends React.Component {
-  render() {
-    // For class components, referencing `this.props.___` and `this.state.___`
-    // from within your arrow function is supported, but we recommend you to
-    // extract these references out to a const, especially if you are
-    // accessing deeply nested attributes (e.g. `this.props.user.name.first`).
-    
-    // PureChild will re-render whenever `user` changes.
-    const decentHandler = () => alert(this.props.user.name.first);
-    
-    // PureChild re-render ONLY when the first name changes.
-    const firstName = this.props.user.name.first;
-    const betterHandler = () => alert(firstName);
-    
-    return (
-      <div>
-        <PureChild onClick={decentHandler} />
-        <PureChild onClick={betterHandler} />
-      </div>
-    );
-  }
+function MyComponent(props) {
+  
+  // PureChild will re-render whenever `props` changes (bad)
+  const badHandleClick = () =>  alert(props.user.name.first);
+  
+  const firstName = props.user.name.first;
+  // Now, PureChild will only re-render when firstName changes (good)
+  const goodHandleClick = () => alert(firstName);
+  
+  return (
+    <div>
+      <PureChild onClick={badHandleClick} />
+      <PureChild onClick={goodHandleClick} />
+    </div>
+  );
 }
@@ -212,4 +213,4 @@ ```
   const badHandleClick = () => {
-    // Referencing `foo` will deopt since it is reassigned after
-    // this arrow function.
+    // Referencing `foo`, which is reassigned after this arrow function, will
+    // prevent this arrow function from being transformed.
     alert(foo);
@@ -228,8 +229,9 @@ };
 function MyComponent(props) {
-  // This arrow function won't be optimized because `fn` is not referenced
-  // in the JSX.
+  // This arrow function won't be transformed because `fn` is not referenced
+  // directly in the JSX.
   const fn = () => {...};
   const badHandleClick = fn;
                     
-  // This will be optimized since `goodHandleClick` is referenced in the JSX.
+  // This arrow function will be transformed since `goodHandleClick` is
+  // referenced directly in the JSX.
   const goodHandleClick = () => {...};
@@ -249,30 +251,1 @@
 ```
-
-- For maximum optimization, avoid accessing nested attributes in your arrow function. Prefer to pull the values out to a const and close over it in your arrow function.
-
-```js
-function MyComponent(props) {
-  
-  const badHandleClick = () => {
-    // Referencing nested attributes inside the arrow function will cause
-    // PureChild to re-render whenever the outermost object changes. In this
-    // case, `props` will change every render, which will cause PureChild to
-    // always re-render.
-    alert(props.user.name.first);
-  };
-  
-  const firstName = props.user.name.first;
-  const goodHandleClick = () => {
-    // To avoid referencing nested attributes inside the arrow function,
-    // simply extract it out to a const, and reference the const.
-    alert(firstName);
-  };
-  
-  return (
-    <div>
-      <PureChild onClick={badHandleClick} />
-      <PureChild onClick={goodHandleClick} />
-    </div>
-  );
-}
-```

No alert changes

Improved metrics

Total package byte prevSize

40539

increased by0.57%

Lines of code

814

increased by2.01%

Worsened metrics

Number of lines in readme file

239

decreased by-10.15%

No dependency changes