diff --git a/README.md b/README.md index 57830de..72c0936 100644 --- a/README.md +++ b/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` are enabled because there is currently no hook alternative for React. This option is configurable via `allowComponentDidCatch`. +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. @@ -42,7 +42,7 @@ This rule is intended to complement the [eslint-plugin-react](https://github.com > What about `ErrorBoundary` class components? Does this lint rule support those? -Yes it does. [Error Boundaries](https://reactjs.org/docs/error-boundaries.html) are implemented by defining `componentDidCatch`. Because there is currently no hook equivalent, class components that implement `componentDidCatch` are allowed by default. +Yes it does. [Error Boundaries](https://reactjs.org/docs/error-boundaries.html) are implemented by defining `static getDerivedStateFromError` or `componentDidCatch`. Because there is currently no hook equivalent, class components that implement `componentDidCatch` or `static getDerivedStateFromError` are allowed by default. This option is configurable. @@ -102,7 +102,7 @@ module.exports = { rules: { "react-prefer-function-component/react-prefer-function-component": [ "error", - { allowComponentDidCatch: false }, + { allowErrorBoundary: false }, ], }, }; @@ -142,48 +142,86 @@ const Foo = ({ foo }) =>
{foo}
; ```js ... -"react-prefer-function-component": [, { "allowComponentDidCatch": , "allowJsxUtilityClass": }] +"react-prefer-function-component": [, { "allowErrorBoundary": , "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`). +- `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
{this.props.foo}
; + return
{this.props.children}
; + } +} +``` + +```jsx +import { Component } from "react"; + +class ErrorBoundary extends Component { + constructor(props) { + super(props); + this.state = { hasError: false }; + } + + static getDerivedStateFromError(error) { + return { hasError: true }; + } + + render() { + return
{this.state.hasError ? "Error" : this.props.children}
; } } ``` -When `false` the rule will also flag components that use `componentDidCatch` +When `false` the rule will also flag error boundary components Examples of **incorrect** 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
{this.props.foo}
; + return
{this.props.children}
; + } +} +``` + +```jsx +import { Component } from "react"; + +class ErrorBoundary extends Component { + constructor(props) { + super(props); + this.state = { hasError: false }; + } + + static getDerivedStateFromError(error) { + return { hasError: true }; + } + + render() { + return
{this.state.hasError ? "Error" : this.props.children}
; } } ``` diff --git a/packages/eslint-plugin-react-prefer-function-component/src/prefer-function-component/index.test.ts b/packages/eslint-plugin-react-prefer-function-component/src/prefer-function-component/index.test.ts index 837aa2c..a2e6756 100644 --- a/packages/eslint-plugin-react-prefer-function-component/src/prefer-function-component/index.test.ts +++ b/packages/eslint-plugin-react-prefer-function-component/src/prefer-function-component/index.test.ts @@ -1,6 +1,6 @@ import { RuleTester } from "eslint"; import rule, { - ALLOW_COMPONENT_DID_CATCH, + ALLOW_ERROR_BOUNDARY, ALLOW_JSX_UTILITY_CLASS, COMPONENT_SHOULD_BE_FUNCTION, } from "."; @@ -179,7 +179,7 @@ const invalidForAllOptions = [ `, ]; -const componentDidCatch = [ +const errorBoundaries = [ // Extends from Component and uses componentDidCatch `\ class Foo extends React.Component { @@ -213,6 +213,51 @@ const componentDidCatch = [ } }; `, + // Extends from Component and uses static getDerivedStateFromError + `\ + class Foo extends React.Component { + constructor(props) { + super(props); + this.state = { hasError: false }; + } + static getDerivedStateFromError(error) { + return { hasError: true }; + } + render() { + return
{this.state.hasError ? "Error" : this.props.foo}
; + } + } + `, + // Extends from Component and uses static getDerivedStateFromError + `\ + class Foo extends React.PureComponent { + constructor(props) { + super(props); + this.state = { hasError: false }; + } + static getDerivedStateFromError(error) { + return { hasError: true }; + } + render() { + return
{this.state.hasError ? "Error" : this.props.foo}
; + } + } + `, + // Extends from Component in an expression context. + `\ + const Foo = class extends React.Component { + constructor(props) { + super(props); + this.state = { hasError: false }; + } + static getDerivedStateFromError(error) { + return { hasError: true }; + } + render() { + return
{this.state.hasError ? "Error" : this.props.foo}
; + } + }; + `, ]; const jsxUtilityClass = [ @@ -230,18 +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_ERROR_BOUNDARY]: false }] }, { code, options: [ { [ALLOW_JSX_UTILITY_CLASS]: true, - [ALLOW_COMPONENT_DID_CATCH]: false, + [ALLOW_ERROR_BOUNDARY]: false, }, ], }, ]), - ...componentDidCatch.flatMap((code) => [ + ...errorBoundaries.flatMap((code) => [ { code }, { code, options: [{ [ALLOW_JSX_UTILITY_CLASS]: true }] }, ]), @@ -261,7 +306,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 }], }, { code, @@ -269,16 +314,16 @@ ruleTester.run("prefer-function-component", rule, { options: [ { [ALLOW_JSX_UTILITY_CLASS]: true, - [ALLOW_COMPONENT_DID_CATCH]: false, + [ALLOW_ERROR_BOUNDARY]: false, }, ], }, ]), - ...componentDidCatch.map((code) => ({ + ...errorBoundaries.map((code) => ({ code, options: [ { - [ALLOW_COMPONENT_DID_CATCH]: false, + [ALLOW_ERROR_BOUNDARY]: false, }, ], errors: [ @@ -292,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 }], }, ]), ], diff --git a/packages/eslint-plugin-react-prefer-function-component/src/prefer-function-component/index.ts b/packages/eslint-plugin-react-prefer-function-component/src/prefer-function-component/index.ts index 1cb3a19..f342a5b 100644 --- a/packages/eslint-plugin-react-prefer-function-component/src/prefer-function-component/index.ts +++ b/packages/eslint-plugin-react-prefer-function-component/src/prefer-function-component/index.ts @@ -5,13 +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_ERROR_BOUNDARY = "allowErrorBoundary"; export const ALLOW_JSX_UTILITY_CLASS = "allowJsxUtilityClass"; type ClassNode = TSESTree.ClassDeclaration | TSESTree.ClassExpression; type RuleOptions = { - allowComponentDidCatch?: boolean; + allowErrorBoundary?: boolean; allowJsxUtilityClass?: boolean; }; @@ -33,7 +33,7 @@ const rule = { { type: "object", properties: { - [ALLOW_COMPONENT_DID_CATCH]: { + [ALLOW_ERROR_BOUNDARY]: { default: true, type: "boolean", }, @@ -49,18 +49,22 @@ const rule = { create(context: Rule.RuleContext) { const options: RuleOptions = context.options[0] ?? {}; - const allowComponentDidCatch = options.allowComponentDidCatch ?? 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") + ); } + return false; }); - if (hasComponentDidCatch && allowComponentDidCatch) { + if (isErrorBoundary && allowErrorBoundary) { return false; } return true;