Support for method-binding patterns

Author: qnighy

CHANGELOG.md

@@ -1,5 +1,8 @@
 ## Unreleased
 
+- Added
+  - Support for method-binding patterns e.g. `this.foo = this.foo.bind(this);`
+
 ## 0.1.8
 
 - Added

README.md

@@ -242,8 +242,8 @@ If you need to enforce specific styles, use Prettier or ESLint or whatever is yo
   - [x] Use `useCallback` if deemed necessary
   - [x] Auto-expand direct callback call (like `this.props.onClick()`) to indirect call
   - [x] Rename methods if necessary
-  - [ ] Skip method-binding expressions (e.g. `onClick={this.onClick.bind(this)}`)
-  - [ ] Skip method-binding statements (e.g. `this.onClick = this.onClick.bind(this)`)
+  - [x] Skip method-binding expressions (e.g. `onClick={this.onClick.bind(this)}`)
+  - [x] Skip method-binding statements (e.g. `this.onClick = this.onClick.bind(this)`)
 - [ ] Support for `this.state`
   - [x] Decompose `this.state` into `useState` variables
   - [x] Rename states if necessary

src/analysis.ts

@@ -2,7 +2,7 @@ import type { NodePath } from "@babel/core";
 import type { Scope } from "@babel/traverse";
 import type { ClassDeclaration, ClassMethod, Identifier, JSXIdentifier, TSType, TSTypeParameterDeclaration } from "@babel/types";
 import { AnalysisError } from "./analysis/error.js";
-import { analyzeClassFields } from "./analysis/class_fields.js";
+import { BindThisSite, analyzeClassFields } from "./analysis/class_fields.js";
 import { analyzeState, StateObjAnalysis } from "./analysis/state.js";
 import { getAndDelete } from "./utils.js";
 import { analyzeProps, needAlias, PropsObjAnalysis } from "./analysis/prop.js";
@@ -45,14 +45,15 @@ export type AnalysisResult = {
   props: PropsObjAnalysis;
   userDefined: UserDefinedAnalysis;
   effects: EffectAnalysis;
+  bindThisSites: BindThisSite[];
 };
 
 export function analyzeClass(
   path: NodePath<ClassDeclaration>,
   preanalysis: PreAnalysisResult
 ): AnalysisResult {
   const locals = new LocalManager(path);
-  const { instanceFields: sites, staticFields } = analyzeClassFields(path);
+  const { instanceFields: sites, staticFields, bindThisSites } = analyzeClassFields(path);
 
   const propsObjAnalysis = getAndDelete(sites, "props") ?? { sites: [] };
   const defaultPropsObjAnalysis = getAndDelete(staticFields, "defaultProps") ?? { sites: [] };
@@ -146,6 +147,7 @@ export function analyzeClass(
     props,
     userDefined,
     effects,
+    bindThisSites,
   };
 }
 

src/analysis/class_fields.ts

@@ -16,6 +16,8 @@ export type ClassFieldsAnalysis = {
   instanceFields: Map<string, ClassFieldAnalysis>;
   /** Access to static fields (`C.foo`, where `C` is the class), indexed by their names. */
   staticFields: Map<string, ClassFieldAnalysis>;
+  /** Appearances of `this` as in `this.foo.bind(this)` */
+  bindThisSites: BindThisSite[];
 };
 
 /**
@@ -105,6 +107,24 @@ export type FieldInit = {
   methodPath: NodePath<ClassMethod | ClassPrivateMethod>;
 };
 
+/**
+ * Appearance of `this` as in `this.foo.bind(this)`
+ */
+export type BindThisSite = {
+  /**
+   * true if the bind call has more arguments e.g. `this.foo.bind(this, 42)`
+   */
+  bindsMore: boolean;
+  /** `this` as in the argument to `Function.prototype.bind` */
+  thisArgPath: NodePath<ThisExpression>;
+  /** The whole bind expression */
+  binderPath: NodePath<CallExpression>;
+  /** The member expression that `this` is being bound to e.g. `this.foo` */
+  bindeePath: NodePath<MemberExpression>;
+  /** true if this is part of self-binding: `this.foo = this.foo.bind(this);` */
+  isSelfBindingInitialization: boolean;
+};
+
 /**
  * Collect declarations and uses of the following:
  *
@@ -301,13 +321,33 @@ export function analyzeClassFields(path: NodePath<ClassDeclaration>): ClassField
   }
 
   // 2nd pass: look for uses within items
+  const bindThisSites: BindThisSite[] = [];
   function traverseItem(owner: string | undefined, path: NodePath) {
     traverseThis(path, (thisPath) => {
       // Ensure this is part of `this.foo`
       const thisMemberPath = thisPath.parentPath;
       if (!thisMemberPath.isMemberExpression({
         object: thisPath.node
       })) {
+        // Check for bind arguments: `this.foo.bind(this)`
+        if (
+          thisMemberPath.isCallExpression()
+          && thisMemberPath.node.arguments[0] === thisPath.node
+          && thisMemberPath.node.callee.type === "MemberExpression"
+          && memberRefName(thisMemberPath.node.callee) === "bind"
+          && thisMemberPath.node.callee.object.type === "MemberExpression"
+          && thisMemberPath.node.callee.object.object.type === "ThisExpression"
+        ) {
+          bindThisSites.push({
+            bindsMore: thisMemberPath.node.arguments.length > 1,
+            thisArgPath: thisPath,
+            binderPath: thisMemberPath,
+            bindeePath: (thisMemberPath.get("callee") as NodePath<MemberExpression>).get("object") as NodePath<MemberExpression>,
+            // Checked later
+            isSelfBindingInitialization: false,
+          });
+          return;
+        }
         throw new AnalysisError(`Stray this`);
       }
 
@@ -345,6 +385,41 @@ export function analyzeClassFields(path: NodePath<ClassDeclaration>): ClassField
     traverseItem(body.owner, body.path);
   }
 
+  // Special handling for self-binding initialization (`this.foo = this.foo.bind(this)`)
+  for (const [name, field] of instanceFields) {
+    field.sites = field.sites.filter((site) => {
+      if (
+        site.type === "decl"
+        && site.init?.type === "init_value"
+      ) {
+        const valuePath = site.init.valuePath;
+        const bindThisSite = bindThisSites.find((binder) => binder.binderPath === valuePath)
+        if (
+          bindThisSite
+          && !bindThisSite.bindsMore
+          && memberRefName(bindThisSite.bindeePath.node) === name
+        ) {
+          bindThisSite.isSelfBindingInitialization = true;
+          // Skip the self-binding initialization (lhs)
+          return false;
+        }
+      }
+      return true;
+    });
+  }
+  for (const [, field] of instanceFields) {
+    field.sites = field.sites.filter((site) => {
+      if (site.type === "expr") {
+        const bindThisSite = bindThisSites.find((binder) => binder.bindeePath === site.path)
+        if (bindThisSite?.isSelfBindingInitialization) {
+          // Skip the self-binding initialization (rhs)
+          return false;
+        }
+      }
+      return true;
+    });
+  }
+
   // Post validation
   for (const [name, field] of instanceFields) {
     if (field.sites.length === 0) {
@@ -373,7 +448,7 @@ export function analyzeClassFields(path: NodePath<ClassDeclaration>): ClassField
     }
   }
 
-  return { instanceFields, staticFields };
+  return { instanceFields, staticFields, bindThisSites };
 }
 
 function traverseThis(path: NodePath, visit: (path: NodePath<ThisExpression>) => void) {

src/index.test.ts

@@ -760,6 +760,53 @@ describe("react-declassify", () => {
       `;
       expect(transform(input)).toBe(output);
     });
+
+    it("transforms binding initialization", () => {
+      const input = dedent`\
+        class C extends React.Component {
+          constructor(props) {
+            super(props);
+            this.foo = this.foo.bind(this);
+          }
+
+          render() {
+            return (
+              <Component2
+                foo={this.foo}
+                bar={this.bar.bind(this)}
+              />
+            );
+          }
+
+          foo() {
+            console.log("foo");
+          }
+
+          bar() {
+            console.log("bar");
+          }
+        }
+      `;
+      const output = dedent`\
+        const C = () => {
+          const foo = React.useCallback(function foo() {
+            console.log("foo");
+          }, []);
+
+          const bar = React.useCallback(function bar() {
+            console.log("bar");
+          }, []);
+
+          return (
+            <Component2
+              foo={foo}
+              bar={bar}
+            />
+          );
+        };
+      `;
+      expect(transform(input)).toBe(output);
+    });
   });
 
   describe("State transformation", () => {

src/index.ts

@@ -180,6 +180,13 @@ function transformClass(analysis: AnalysisResult, options: { ts: boolean }, babe
       }
     }
   }
+  for (const bindThisSite of analysis.bindThisSites) {
+    if (bindThisSite.bindsMore) {
+      bindThisSite.thisArgPath.replaceWith(t.nullLiteral());
+    } else {
+      bindThisSite.binderPath.replaceWith(bindThisSite.bindeePath.node);
+    }
+  }
   // Preamble is a set of statements to be added before the original render body.
   const preamble: Statement[] = [];
   const propsWithAlias = Array.from(analysis.props.props).filter(([, prop]) => needAlias(prop));