Merge pull request #5 from Gelio/develop

Release v1.1.0

Author: Gelio

.npmignore

@@ -8,3 +8,4 @@ tsconfig.json
 tslint.json
 .prettierrc
 .editorconfig
+CONTRIBUTING.md

CHANGELOG.md

@@ -1,5 +1,21 @@
 # Changelog
 
+## v1.1.0 (2019-02-09)
+
+- Allow using hooks inside React component decorators, such as `React.memo` or `React.forwardRef`.
+
+  For example, the following code sample now **does not** violate the rule:
+
+  ```tsx
+  const MyComponent = React.memo(props => {
+    useEffect(() => {
+      console.log('Counter changed');
+    }, [props.value]);
+
+    return <div>Counter: {props.value}</div>;
+  });
+  ```
+
 ## v1.0.1 (2019-02-03)
 
 - Updated README

CONTRIBUTING.md

@@ -0,0 +1,29 @@
+# Contributing guidelines
+
+Anyone is welcome to contribute to this project. Filing issues with detected bugs or false positives
+is appreciated, as is opening pull requests with fixes to those.
+
+Before opening a pull request, make sure that all the tests pass by running
+
+```sh
+npm run test
+```
+
+It would be great if you added new test cases that cover the functionality you added/fixed in the
+pull request.
+
+## Information for developers
+
+Creating TSLint rules may seem complex in the beginning, but it is actually pretty easy once you
+understand the underlying concepts.
+
+Each Typescript file is parsed into an Abstract Syntax Tree (AST). It is a tree structure that
+contains nodes like statements, expressions, identifier that appear in the file being linted. The
+rule can then analyze the AST and report rule violations.
+
+Check the [TSLint documentation for developing TSLint rules](https://palantir.github.io/tslint/develop/custom-rules/)
+for more information on the subject.
+
+[AST Explorer](https://astexplorer.net/) is an extremely useful tool when working with and exploring ASTs. It
+supports different parsers, so when developing TSLint rules make sure to pick the _typescript_
+parser from the settings bar at the top.

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "tslint-react-hooks",
-  "version": "1.0.1",
+  "version": "1.1.0",
   "description": "TSLint rule that enforces the Rules of Hooks",
   "main": "tslint-react-hooks.json",
   "scripts": {

package.json

@@ -17,5 +17,5 @@ export function isBinaryConditionalExpression(
     return false;
   }
 
-  return binaryConditionalOperators.indexOf(node.operatorToken.kind) !== -1;
+  return binaryConditionalOperators.includes(node.operatorToken.kind);
 }

src/react-hooks-nesting-walker/is-binary-conditional-expression.ts

@@ -1,31 +1,17 @@
-import {
-  CallExpression,
-  isPropertyAccessExpression,
-  isIdentifier
-} from 'typescript';
+import { CallExpression } from 'typescript';
 
 import { isHookIdentifier } from './is-hook-identifier';
+import { isReactApiExpression } from './is-react-api-expression';
 
 /**
- * Tests is a `CallExpression` calls a React Hook
+ * Tests if a `CallExpression` calls a React Hook
  * @see https://github.com/facebook/react/blob/master/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js#L26
  */
 export function isHookCall({ expression }: CallExpression) {
-  if (isIdentifier(expression)) {
-    /**
-     * Test for direct `useHook` calls
-     */
-    return isHookIdentifier(expression);
-  } else if (isPropertyAccessExpression(expression)) {
-    /**
-     * Test for `React.useHook` calls
-     */
-    return (
-      isIdentifier(expression.expression) &&
-      expression.expression.text === 'React' &&
-      isHookIdentifier(expression.name)
-    );
-  }
-
-  return false;
+  return isHookAccessExpression(expression);
 }
+
+/**
+ * Tests for `useHook` or `React.useHook` calls
+ */
+const isHookAccessExpression = isReactApiExpression(isHookIdentifier);

src/react-hooks-nesting-walker/is-hook-call.ts

@@ -0,0 +1,35 @@
+import {
+  Identifier,
+  isPropertyAccessExpression,
+  isIdentifier,
+  Expression,
+} from 'typescript';
+
+export type Predicate<T> = (t: T) => boolean;
+
+/**
+ * Tests whether an `Expression` is an identifier that matches a predicate. Accepts also property
+ * access of that identifier from React's top-level API.
+ *
+ * @example
+ * const isForwardRef = isReactApiExpression((identifier) => identifier.text === 'forwardRef');
+ * // would match `isForwardRef` or `React.isForwardRef`
+ * const matches = isForwardRef(node);
+ *
+ * @param predicate Predicate that is run on the actual identifier.
+ */
+export const isReactApiExpression = (predicate: Predicate<Identifier>) => (
+  expression: Expression,
+) => {
+  if (isIdentifier(expression)) {
+    return predicate(expression);
+  } else if (isPropertyAccessExpression(expression)) {
+    return (
+      isIdentifier(expression.expression) &&
+      expression.expression.text === 'React' &&
+      predicate(expression.name)
+    );
+  }
+
+  return false;
+};

src/react-hooks-nesting-walker/is-react-api-expression.ts

@@ -0,0 +1,11 @@
+import { isReactApiExpression } from './is-react-api-expression';
+
+const reactComponentDecorators = ['forwardRef', 'memo'];
+
+/**
+ * Tests is an expression is a React top-level API component decorator (e.g. `React.forwardRef`,
+ * `React.memo`)
+ */
+export const isReactComponentDecorator = isReactApiExpression(identifier =>
+  reactComponentDecorators.includes(identifier.text),
+);

src/react-hooks-nesting-walker/is-react-component-decorator.ts

@@ -13,12 +13,14 @@ import {
   isIdentifier,
   isSourceFile,
   isClassDeclaration,
+  isCallExpression,
 } from 'typescript';
 
 import { isHookCall } from './is-hook-call';
 import { ERROR_MESSAGES } from './error-messages';
 import { isBinaryConditionalExpression } from './is-binary-conditional-expression';
 import { isComponentOrHookIdentifier } from './is-component-or-hook-identifier';
+import { isReactComponentDecorator } from './is-react-component-decorator';
 
 export class ReactHooksNestingWalker extends RuleWalker {
   public visitCallExpression(node: CallExpression) {
@@ -108,6 +110,16 @@ export class ReactHooksNestingWalker extends RuleWalker {
         return;
       }
 
+      /**
+       * Allow using hooks when the function is passed to `React.memo` or `React.forwardRef`
+       */
+      if (
+        isCallExpression(ancestor.parent) &&
+        isReactComponentDecorator(ancestor.parent.expression)
+      ) {
+        return;
+      }
+
       // Disallow using hooks inside other kinds of functions
       this.addFailureAtNode(hookNode, ERROR_MESSAGES.invalidFunctionExpression);
       return;

src/react-hooks-nesting-walker/react-hooks-nesting-walker.ts

@@ -0,0 +1,36 @@
+// Tests for React top level APIs that wrap components
+
+import React, { memo, forwardRef } from 'react';
+
+// Hooks should be allowed inside functions passed to React.memo and React.forwardRef
+
+const MemoizedComponent = React.memo(() => {
+  const ref = useRef();
+
+  return <div>Memoized</div>;
+});
+
+const RefForwardingComponent = React.forwardRef((props, ref) => {
+  useEffect(() => {
+    console.log('Forwarding refs');
+  });
+
+  return <div ref={ref}>Forwarding refs</div>;
+});
+
+
+// The same as above, but not using the `React` prefix
+
+const MemoizedComponent = memo(() => {
+  const ref = useRef();
+
+  return <div>Memoized</div>;
+});
+
+const RefForwardingComponent = forwardRef((props, ref) => {
+  useEffect(() => {
+    console.log('Forwarding refs');
+  });
+
+  return <div ref={ref}>Forwarding refs</div>;
+});

test/tslint-rule/react-top-level-api.ts.lint

@@ -8,7 +8,8 @@
     "noImplicitReturns": true,
     "noImplicitThis": true,
     "strict": true,
-    "target": "es2015"
+    "target": "es2015",
+    "lib": ["esnext"]
   },
   "include": ["src/**/*"],
   "exclude": ["node_modules", "dist"]