switch to allowErrorBoundary

Author: henryqdineen

README.md

@@ -34,7 +34,7 @@ using only functions.
 
 Leaving the choice between class or function components up to the community is great, but generally within a codebase I want consistency: either we're writing class components and HoCs or we're using function components and hooks. Straddling the two adds unnecessary hurdles for sharing reusable logic.
 
-By default, class components that use `componentDidCatch` or `static getDerivedStateFromError` are enabled because there is currently no hook alternative for React. This is configurable via `allowComponentDidCatch` and `allowGetDerivedStateFromError`.
+By default, class components that use `componentDidCatch` or `static getDerivedStateFromError` are allowed because there is currently no hook alternative for React. This is configurable via `allowErrorBoundary`.
 
 This rule is intended to complement the [eslint-plugin-react](https://github.com/yannickcr/eslint-plugin-react) rule set.
 
@@ -102,7 +102,7 @@ module.exports = {
   rules: {
     "react-prefer-function-component/react-prefer-function-component": [
       "error",
-      { allowComponentDidCatch: false },
+      { allowErrorBoundary: false },
     ],
   },
 };
@@ -142,86 +142,75 @@ const Foo = ({ foo }) => <div>{foo}</div>;
 
 ```js
 ...
-"react-prefer-function-component": [<enabled>, { "allowComponentDidCatch": <allowComponentDidCatch>, "allowJsxUtilityClass": <allowJsxUtilityClass> }]
+"react-prefer-function-component": [<enabled>, { "allowErrorBoundary": <allowErrorBoundary>, "allowJsxUtilityClass": <allowJsxUtilityClass> }]
 ...
 ```
 
 - `enabled`: for enabling the rule. 0=off, 1=warn, 2=error. Defaults to 0.
-- `allowComponentDidCatch`: optional boolean. set to `false` if you want to also flag class components that use `componentDidCatch` (defaults to `true`).
-- `allowGetDerivedStateFromError`: optional boolean. set to `false` if you want to also flag class components that use `static getDerivedStateFromError` (defaults to `true`).
+- `allowErrorBoundary`: optional boolean. set to `false` if you want to also flag class components that use `componentDidCatch` or `static getDerivedStateFromError` (defaults to `true`).
 - `allowJsxUtilityClass`: optional boolean. set to `true` if you want to allow classes that contain JSX but aren't class components (defaults to `false`).
 
-### `allowComponentDidCatch`
+### `allowErrorBoundary`
 
-When `true` (the default) the rule will ignore components that use `componentDidCatch`
+When `true` (the default) the rule will ignore error boundary components that use `componentDidCatch` or `static getDerivedStateFromError`
 
 Examples of **correct** code for this rule:
 
 ```jsx
 import { Component } from "react";
 
-class Foo extends Component {
+class ErrorBoundary extends Component {
   componentDidCatch(error, errorInfo) {
     logErrorToMyService(error, errorInfo);
   }
 
   render() {
-    return <div>{this.props.foo}</div>;
+    return <div>{this.props.children}</div>;
   }
 }
 ```
 
-When `false` the rule will also flag components that use `componentDidCatch`
-
-Examples of **incorrect** code for this rule:
-
 ```jsx
 import { Component } from "react";
 
-class Foo extends Component {
-  componentDidCatch(error, errorInfo) {
-    logErrorToMyService(error, errorInfo);
+class ErrorBoundary extends Component {
+  constructor(props) {
+    super(props);
+    this.state = { hasError: false };
+  }
+
+  static getDerivedStateFromError(error) {
+    return { hasError: true };
   }
 
   render() {
-    return <div>{this.props.foo}</div>;
+    return <div>{this.state.hasError ? "Error" : this.props.children}</div>;
   }
 }
 ```
 
-### `allowGetDerivedStateFromError`
-
-When `true` (the default) the rule will ignore components that use `static getDerivedStateFromError`
+When `false` the rule will also flag error boundary components
 
-Examples of **correct** code for this rule:
+Examples of **incorrect** code for this rule:
 
 ```jsx
 import { Component } from "react";
 
-class Foo extends Component {
-  constructor(props) {
-    super(props);
-    this.state = { hasError: false };
-  }
-
-  static getDerivedStateFromError(error) {
-    return { hasError: true };
+class ErrorBoundary extends Component {
+  componentDidCatch(error, errorInfo) {
+    logErrorToMyService(error, errorInfo);
   }
 
   render() {
-    return <div>{this.state.hasError ? "Error" : this.props.foo}</div>;
+    return <div>{this.props.children}</div>;
   }
 }
 ```
 
-When `false` the rule will also flag components that use `static getDerivedStateFromError`
-
-Examples of **incorrect** code for this rule:
-
 ```jsx
 import { Component } from "react";
 
-class Foo extends Component {
+class ErrorBoundary extends Component {
   constructor(props) {
     super(props);
     this.state = { hasError: false };
@@ -232,7 +221,7 @@ class Foo extends Component {
   }
 
   render() {
-    return <div>{this.state.hasError ? "Error" : this.props.foo}</div>;
+    return <div>{this.state.hasError ? "Error" : this.props.children}</div>;
   }
 }
 ```

packages/eslint-plugin-react-prefer-function-component/src/prefer-function-component/index.test.ts

@@ -1,7 +1,6 @@
 import { RuleTester } from "eslint";
 import rule, {
-  ALLOW_COMPONENT_DID_CATCH,
-  ALLOW_GET_DERIVED_STATE_FROM_ERROR,
+  ALLOW_ERROR_BOUNDARY,
   ALLOW_JSX_UTILITY_CLASS,
   COMPONENT_SHOULD_BE_FUNCTION,
 } from ".";
@@ -180,7 +179,7 @@ const invalidForAllOptions = [
   `,
 ];
 
-const componentDidCatch = [
+const errorBoundaries = [
   // Extends from Component and uses componentDidCatch
   `\
     class Foo extends React.Component {
@@ -214,9 +213,6 @@ const componentDidCatch = [
       }
     };
   `,
-];
-
-const getDerivedStateFromError = [
   // Extends from Component and uses static getDerivedStateFromError
   `\
     class Foo extends React.Component {
@@ -279,24 +275,18 @@ ruleTester.run("prefer-function-component", rule, {
     ...validForAllOptions.flatMap((code) => [
       { code },
       { code, options: [{ [ALLOW_JSX_UTILITY_CLASS]: true }] },
-      { code, options: [{ [ALLOW_COMPONENT_DID_CATCH]: false }] },
-      { code, options: [{ [ALLOW_GET_DERIVED_STATE_FROM_ERROR]: false }] },
+      { code, options: [{ [ALLOW_ERROR_BOUNDARY]: false }] },
       {
         code,
         options: [
           {
             [ALLOW_JSX_UTILITY_CLASS]: true,
-            [ALLOW_COMPONENT_DID_CATCH]: false,
-            [ALLOW_GET_DERIVED_STATE_FROM_ERROR]: false,
+            [ALLOW_ERROR_BOUNDARY]: false,
           },
         ],
       },
     ]),
-    ...componentDidCatch.flatMap((code) => [
-      { code },
-      { code, options: [{ [ALLOW_JSX_UTILITY_CLASS]: true }] },
-    ]),
-    ...getDerivedStateFromError.flatMap((code) => [
+    ...errorBoundaries.flatMap((code) => [
       { code },
       { code, options: [{ [ALLOW_JSX_UTILITY_CLASS]: true }] },
     ]),
@@ -316,37 +306,24 @@ ruleTester.run("prefer-function-component", rule, {
       {
         code,
         errors: [{ messageId: COMPONENT_SHOULD_BE_FUNCTION }],
-        options: [{ [ALLOW_COMPONENT_DID_CATCH]: false }],
+        options: [{ [ALLOW_ERROR_BOUNDARY]: false }],
       },
       {
         code,
         errors: [{ messageId: COMPONENT_SHOULD_BE_FUNCTION }],
         options: [
           {
             [ALLOW_JSX_UTILITY_CLASS]: true,
-            [ALLOW_COMPONENT_DID_CATCH]: false,
+            [ALLOW_ERROR_BOUNDARY]: false,
           },
         ],
       },
     ]),
-    ...componentDidCatch.map((code) => ({
-      code,
-      options: [
-        {
-          [ALLOW_COMPONENT_DID_CATCH]: false,
-        },
-      ],
-      errors: [
-        {
-          messageId: COMPONENT_SHOULD_BE_FUNCTION,
-        },
-      ],
-    })),
-    ...getDerivedStateFromError.map((code) => ({
+    ...errorBoundaries.map((code) => ({
       code,
       options: [
         {
-          [ALLOW_GET_DERIVED_STATE_FROM_ERROR]: false,
+          [ALLOW_ERROR_BOUNDARY]: false,
         },
       ],
       errors: [
@@ -360,7 +337,7 @@ ruleTester.run("prefer-function-component", rule, {
       {
         code,
         errors: [{ messageId: COMPONENT_SHOULD_BE_FUNCTION }],
-        options: [{ [ALLOW_COMPONENT_DID_CATCH]: false }],
+        options: [{ [ALLOW_ERROR_BOUNDARY]: false }],
       },
     ]),
   ],

packages/eslint-plugin-react-prefer-function-component/src/prefer-function-component/index.ts

@@ -5,16 +5,13 @@ import type { Rule } from "eslint";
 import type { TSESTree } from "@typescript-eslint/types";
 
 export const COMPONENT_SHOULD_BE_FUNCTION = "componentShouldBeFunction";
-export const ALLOW_COMPONENT_DID_CATCH = "allowComponentDidCatch";
-export const ALLOW_GET_DERIVED_STATE_FROM_ERROR =
-  "allowGetDerivedStateFromError";
+export const ALLOW_ERROR_BOUNDARY = "allowErrorBoundary";
 export const ALLOW_JSX_UTILITY_CLASS = "allowJsxUtilityClass";
 
 type ClassNode = TSESTree.ClassDeclaration | TSESTree.ClassExpression;
 
 type RuleOptions = {
-  allowComponentDidCatch?: boolean;
-  allowGetDerivedStateFromError?: boolean;
+  allowErrorBoundary?: boolean;
   allowJsxUtilityClass?: boolean;
 };
 
@@ -36,11 +33,7 @@ const rule = {
       {
         type: "object",
         properties: {
-          [ALLOW_COMPONENT_DID_CATCH]: {
-            default: true,
-            type: "boolean",
-          },
-          [ALLOW_GET_DERIVED_STATE_FROM_ERROR]: {
+          [ALLOW_ERROR_BOUNDARY]: {
             default: true,
             type: "boolean",
           },
@@ -56,33 +49,24 @@ const rule = {
 
   create(context: Rule.RuleContext) {
     const options: RuleOptions = context.options[0] ?? {};
-    const allowComponentDidCatch = options.allowComponentDidCatch ?? true;
-    const allowGetDerivedStateFromError =
-      options.allowGetDerivedStateFromError ?? true;
+    const allowErrorBoundary = options.allowErrorBoundary ?? true;
     const allowJsxUtilityClass = options.allowJsxUtilityClass ?? false;
 
     function shouldPreferFunction(node: ClassNode): boolean {
-      const properties = node.body.body;
-      const hasComponentDidCatch = properties.some((property) => {
+      const isErrorBoundary = node.body.body.some((property) => {
         if ("key" in property && "name" in property.key) {
-          return property.key.name === "componentDidCatch";
+          return (
+            property.key.name === "componentDidCatch" ||
+            (property.static &&
+              property.key.name === "getDerivedStateFromError")
+          );
         }
-      });
-
-      if (hasComponentDidCatch && allowComponentDidCatch) {
         return false;
-      }
-
-      const hasGetDerivedStateFromError = properties.some((property) => {
-        if ("key" in property && "name" in property.key && property.static) {
-          return property.key.name === "getDerivedStateFromError";
-        }
       });
 
-      if (hasGetDerivedStateFromError && allowGetDerivedStateFromError) {
+      if (isErrorBoundary && allowErrorBoundary) {
         return false;
       }
-
       return true;
     }