reflective-bind - npm Package Compare versions

Comparing version 0.0.4-rc1 to 0.0.4-rc2

babel/index.js

@@ -12,14 +12,14 @@ /**
  *   - Simply replace `expression.bind(...bindArgs)` with
- *     `reflectiveBind(expression, ...bindArgs)`
+ *     `babelBind(expression, ...bindArgs)`
  *
  * Arrow Functions
- *   - Bail if function closes over any non-constant variables. Since the
- *     variable is non-constant, it is very difficult to determine what values
- *     to bind the variables to after hoisting. You'd have to find all the call
- *     sites, and bind right before the call.
+ *   - Bail if function closes over any variables that are assigned to after
+ *     the fn definition. If we transformed the arrow function in this case,
+ *     we would either get a reference error, or we would bind to the wrong
+ *     value.
  *   - Bail if the function closes over a variable that is bound after it.
- *     Since we replace the function call with a call to `babelBind` and pass 
- *     in the closed over variables, it would result in an reference error. 
+ *     Since we replace the function call with a call to `babelBind` and pass
+ *     in the closed over variables, it would result in an reference error.
  *   - Otherwise, hoist the arrow function to module level function and replace
- *     the arrow function with: `reflectiveBind(hoisted, this, ...constants)`,
+ *     the arrow function with: `babelBind(hoisted, this, ...constants)`,
  *     where `constants` is a list of all the constant variables that the
@@ -31,5 +31,10 @@ *     arrow function closed over (could be empty).
  *     rise of arrow functions, it is currently not worthwhile to support them.
+ *   - The difficulty with function expressions is that the `this` context can
+ *     be bound after function definition, but calling `babelBind` requires us
+ *     to pick a context to bind to.
  *   - If we want to support them, the best way is via a reflectivePartial
  *     implementation. You would follow the same logic as arrow functions, but
- *     you would hoist and replace with a call to reflectivePartial.
+ *     you would hoist and replace with a call to reflectivePartial. Either
+ *     that or only support functions that are immediately bound:
+ *     `function() {...}.bind(...)`.
  *   - Things to think through more: async and generator functions.
@@ -46,7 +51,8 @@ */
   let _hoistedSlug;
-  let _hoistPath;
-  let _needImport = false;
   let _bableBindIdentifer;
   let _babelBindImportDeclaration;
 
+  let _hoistPath;
+  let _needImport = false;
+
   const rootVisitor = {
@@ -109,2 +115,9 @@ // TODO: figure out how to not do the explicit recursive traversal.
 
+    JSXAttribute(path) {
+      // Don't transform ref callbacks
+      if (t.isJSXIdentifier(path.node.name) && path.node.name.name === "ref") {
+        path.skip();
+      }
+    },
+
     JSXExpressionContainer(path) {
@@ -149,2 +162,3 @@ const exprPath = path.get("expression");
       t.isMemberExpression(path.node.callee) &&
+      !path.node.callee.computed &&
       t.isIdentifier(path.node.callee.property, {name: "bind"})
@@ -163,2 +177,9 @@ ) {
   function processArrowFunctionExpression(path) {
+    // Don't hoist if the arrow function is top level (defined in the same
+    // scope as the hoist path) since that is where it's going to be hoisted
+    // to anyways.
+    if (path.parentPath.scope === _hoistPath.scope) {
+      return;
+    }
+
     const state = {
@@ -215,3 +236,3 @@ fnPath: path,
       // function we want to hoist.
-      if (!sameThisContext(state.fnPath, path)) {
+      if (!isDefinitelySameThisContext(state.fnPath, path)) {
         return;
@@ -424,2 +445,10 @@ }
 
+    // If the checkPath is inside a function, it might be called after
+    // otherPath has executed.
+    for (let i = 1; i < checkPathAncestorIdx; i++) {
+      if (t.isFunction(checkPathAncestry[i])) {
+        return false;
+      }
+    }
+
     // If both relationshps are part of a container list, the key property
@@ -490,7 +519,7 @@ // gives you the index in the container.
    * Returns true only if path has the same `this` context as parentPath.
-   * 
+   *
    * This means that the function-ancestor chain must only consist of arrow
    * functions.
    */
-  function sameThisContext(parentFnPath, path) {
+  function isDefinitelySameThisContext(parentFnPath, path) {
     let cur = path.getFunctionParent();
@@ -497,0 +526,0 @@ while (cur) {

CHANGELOG.md

@@ -5,11 +5,16 @@ # Reflective Bind Change Log
 
-## 0.0.4-rc1
+## Unreleased
 
-- Don't transform inline functions on `ref` prop.
-- Don't transform arrow functions defined at the top level.
+## 0.0.4-rc2
 
+* Don't transform inline functions on `ref` prop.
+* Don't transform arrow functions defined at the top level.
+* Don't transform functions referencing a variable that is re-assigned in a
+  different function.
+
 ## 0.0.3
 
-- Support non-constant reference in arrow function as long as there is no reassignment to the variable after the arrow function.
-- Don't transform inline functions on JSX html literals.
+* Support non-constant reference in arrow function as long as there is no
+  reassignment to the variable after the arrow function.
+* Don't transform inline functions on JSX html literals.
 
@@ -16,0 +21,0 @@ ## 0.0.2

package.json

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

README.md

@@ -6,6 +6,10 @@ [![Build Status](https://travis-ci.org/flexport/reflective-bind.svg?branch=master)](https://travis-ci.org/flexport/reflective-bind)
 
-With reflective-bind, you can freely use inline functions in render without worrying about wasteful re-rendering of React pure components. The best part is, it requires almost no code change 🙌
+With reflective-bind, you can freely use inline functions in render without worrying about wasteful re-rendering of React pure components. It works for both stateless functional components and class components. The best part is, it requires almost no code change 🙌
 
 [Check out our blog post](https://flexport.engineering/ending-the-debate-on-inline-functions-in-react-8c03fabd144) for more info on the motivation and the inner workings of reflective-bind.
 
+## Performance Benefits
+
+For one of Flexport's more complex forms, turning on the Babel transform reduced the wasted render time from 175ms to 18ms. Your mileage will vary based on the structure of your app and your use of pure components.
+
 ## Installation
@@ -19,2 +23,4 @@
 
+*NOTE: the design goal of the plugin is to preserve the semantics of your code. Your inline functions will still create new function instances each render. The transform simply enables the equality comparison of two function instances via reflection.*
+
 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):
@@ -239,12 +245,1 @@
 ```
-
-- Inline functions on html literals will not be transformed.
-
-```jsx
-function MyComponent(props) {
-  // This arrow function will not be transformed because it is used on an html
-  // literal.
-  const handleClick = () => {...};
-  return <div onClick={handleClick} />;
-}
-```

New alerts

Native code

Supply chain risk

Contains native code (e.g., compiled binaries or shared libraries). Including native code can obscure malicious behavior.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

48857

increased by18.25%

Number of package files

13

increased by8.33%

Lines of code

854

increased by3.02%

Worsened metrics

Number of lines in readme file

243

decreased by-2.02%

Number of medium supply chain risk alerts

1

increased byInfinity%

No dependency changes