diff --git a/packages/eslint-plugin-react-hooks/README.md b/packages/eslint-plugin-react-hooks/README.md index e3b7d40431ea..fbb2c0a48893 100644 --- a/packages/eslint-plugin-react-hooks/README.md +++ b/packages/eslint-plugin-react-hooks/README.md @@ -41,6 +41,7 @@ If you want more fine-grained configuration, you can instead add a snippet like ], "rules": { // ... + "react-hooks/no-nested-components": "error", "react-hooks/rules-of-hooks": "error", "react-hooks/exhaustive-deps": "warn" } @@ -70,6 +71,93 @@ We suggest to use this option **very sparingly, if at all**. Generally saying, w Please refer to the [Rules of Hooks](https://reactjs.org/docs/hooks-rules.html) documentation and the [Hooks FAQ](https://reactjs.org/docs/hooks-faq.html#what-exactly-do-the-lint-rules-enforce) to learn more about this rule. -## License +## no-nested-components -MIT +Based of [`react/no-unstable-nested-components`](https://github.com/jsx-eslint/eslint-plugin-react/blob/8beb2aae3fbe36dd6f495b72cb20b27c043aff68/docs/rules/no-unstable-nested-components.md) but with a [detection mechanism consistent with Rules of Hooks](https://reactjs.org/docs/hooks-faq.html#what-exactly-do-the-lint-rules-enforce). + +Creating components inside components (nested components) will cause React to throw away the state of those nested components on each re-render of their parent. + +React reconciliation performs element type comparison with [reference equality](https://reactjs.org/docs/reconciliation.html#elements-of-different-types). The reference to the same element changes on each re-render when defining components inside the render block. This leads to complete recreation of the current node and all its children. As a result the virtual DOM has to do extra unnecessary work and [possible bugs are introduced](https://codepen.io/ariperkkio/pen/vYLodLB). + +### `no-nested-components` details + +The following patterns are considered warnings: + +```jsx +function Component() { + // nested component declaration + function UnstableNestedComponent() { + return
; + } + + return ( +
+ +
+ ); +} +``` + + +```jsx +function useComponent() { + // Nested component declaration in a hook. See https://reactjs.org/docs/hooks-faq.html#what-exactly-do-the-lint-rules-enforce for what's considered a Component and hook. + return function Component() { + return
+ } +} +``` + +```jsx +function Component() { + const config = React.useMemo({ + // Nested component declaration. See https://reactjs.org/docs/hooks-faq.html#what-exactly-do-the-lint-rules-enforce for what's considered a component. + ArrowDown(event) { + + } + }) + + return ( +
+ +
+ ); +} +``` + + +The following patterns are **not** considered warnings: + +```jsx +function OutsideDefinedComponent(props) { + return
; +} + +function Component() { + return ( +
+ +
+ ); +} +``` + +⚠️ WARNING ⚠️: + +Creating nested but memoized components should also be avoided since memoization is a performance concern not a semantic guarantee. +If the `useCallback` or `useMemo` hook has no dependency, you can safely move the component definition out of the render function. +If the hook does have dependencies, you should refactor the code so that you're able to move the component definition out of the render function. +If you want React to throw away the state of the nested component, use a [`key`](https://reactjs.org/docs/lists-and-keys.html#keys) instead. + +```jsx +function Component() { + // No ESLint warning but `MemoizedNestedComponent` should be moved outside of `Component`. + const MemoizedNestedComponent = React.useCallback(() =>
, []); + + return ( +
+ +
+ ); +} +``` diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleNoNestedComponents-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleNoNestedComponents-test.js new file mode 100644 index 000000000000..b47635f9b2a9 --- /dev/null +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRuleNoNestedComponents-test.js @@ -0,0 +1,1445 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +'use strict'; + +const ESLintTester = require('eslint').RuleTester; +const ReactHooksESLintPlugin = require('eslint-plugin-react-hooks'); +const ReactHooksESLintRule = + ReactHooksESLintPlugin.rules['no-nested-components']; + +/** + * A string template tag that removes padding from the left side of multi-line strings + * @param {Array} strings array of code strings (only one expected) + */ +function normalizeIndent(strings) { + const codeLines = strings[0].split('\n'); + const leftPadding = codeLines[1].match(/\s+/)[0]; + return codeLines.map(line => line.substr(leftPadding.length)).join('\n'); +} + +// *************************************************** +// For easier local testing, you can add to any case: +// { +// skip: true, +// --or-- +// only: true, +// ... +// } +// *************************************************** + +// Tests that are valid/invalid across all parsers +const tests = { + valid: [ + { + code: normalizeIndent` + function ParentComponent() { + return ( +
+ +
+ ); + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + return React.createElement( + "div", + null, + React.createElement(OutsideDefinedFunctionComponent, null) + ); + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + return ( + } + header={
} + /> + ); + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + return React.createElement(SomeComponent, { + footer: React.createElement(OutsideDefinedComponent, null), + header: React.createElement("div", null) + }); + } + `, + }, + { + //false-negative due to unknown display name: memoized component + code: normalizeIndent` + function ParentComponent() { + const MemoizedNestedComponent = React.useCallback(() =>
, []); + + return ( +
+ +
+ ); + } + `, + }, + { + // false-negative due to unknown display name: memoized component + code: normalizeIndent` + function ParentComponent() { + const MemoizedNestedComponent = React.useCallback( + () => React.createElement("div", null), + [] + ); + + return React.createElement( + "div", + null, + React.createElement(MemoizedNestedComponent, null) + ); + } + `, + }, + { + // false-negative due to unknown display name: memoized component + code: normalizeIndent` + function ParentComponent() { + const MemoizedNestedFunctionComponent = React.useCallback( + function () { + return
; + }, + [] + ); + + return ( +
+ +
+ ); + } + `, + }, + { + // false-negative due to unknown display name: memoized component + code: normalizeIndent` + function ParentComponent() { + const MemoizedNestedFunctionComponent = React.useCallback( + function () { + return React.createElement("div", null); + }, + [] + ); + + return React.createElement( + "div", + null, + React.createElement(MemoizedNestedFunctionComponent, null) + ); + } + `, + }, + { + code: normalizeIndent` + function ParentComponent(props) { + // Should not interfere handler declarations + function onClick(event) { + props.onClick(event.target.value); + } + + const onKeyPress = () => null; + + function getOnHover() { + return function onHover(event) { + props.onHover(event.target); + } + } + + return ( +
+
+ ); + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + function getComponent() { + return
; + } + + return ( +
+ {getComponent()} +
+ ); + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + function getComponent() { + return React.createElement("div", null); + } + + return React.createElement("div", null, getComponent()); + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + return ( + + {() =>
} + + ); + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + return ( +
} /> + ); + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + return ( + ( +
    + {items[index].map((item) => +
  • + {item} +
  • + )} +
+ )) + } + /> + ); + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + return React.createElement( + RenderPropComponent, + null, + () => React.createElement("div", null) + ); + } + `, + }, + { + code: normalizeIndent` + function ParentComponent(props) { + return ( +
    + {props.items.map(item => ( +
  • + {item.name} +
  • + ))} +
+ ); + } + `, + }, + { + code: normalizeIndent` + function ParentComponent(props) { + return ( + { + return ( +
  • + {item.name} +
  • + ); + })} + /> + ); + } + `, + }, + { + code: normalizeIndent` + function ParentComponent(props) { + return React.createElement( + "ul", + null, + props.items.map(() => + React.createElement( + "li", + { key: item.id }, + item.name + ) + ) + ) + } + `, + }, + { + code: normalizeIndent` + function createTestComponent(props) { + return ( +
    + ); + } + `, + }, + { + code: normalizeIndent` + function createTestComponent(props) { + return React.createElement("div", null); + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + return ( +
    } /> + ); + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + return React.createElement(ComponentWithProps, { + footer: () => React.createElement("div", null) + }); + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + return ( +
    }} /> + ) + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + return ( + + { + thing.match({ + renderLoading: () =>
    , + renderSuccess: () =>
    , + renderFailure: () =>
    , + }) + } + + ) + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + const thingElement = thing.match({ + renderLoading: () =>
    , + renderSuccess: () =>
    , + renderFailure: () =>
    , + }); + return ( + + {thingElement} + + ) + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + return ( + + { + thing.match({ + loading: () =>
    , + success: () =>
    , + failure: () =>
    , + }) + } + + ) + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + const thingElement = thing.match({ + loading: () =>
    , + success: () =>
    , + failure: () =>
    , + }); + return ( + + {thingElement} + + ) + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + return ( +
    } /> + ); + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + return React.createElement(ComponentForProps, { + renderFooter: () => React.createElement("div", null) + }); + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + useEffect(() => { + return () => null; + }); + + return
    ; + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + return ( + ( + + {items.map(item => ( +
  • {item}
  • + ))} +
    + )} /> + ) + } + `, + }, + { + code: normalizeIndent` + const ParentComponent = () => ( + + {list.map(item => ( +
  • {item}
  • + ))} + , + ]} + /> + ); + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + const rows = [ + { + name: 'A', + render: (props) => + }, + ]; + + return ; + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + return null }} />; + } + `, + }, + { + code: normalizeIndent` + const ParentComponent = createReactClass({ + displayName: "ParentComponent", + statics: { + getSnapshotBeforeUpdate: function () { + return null; + }, + }, + render() { + return
    ; + }, + }); + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + const rows = [ + { + name: 'A', + notPrefixedWithRender: (props) => + }, + ]; + + return
    ; + } + `, + }, + /* TODO These minor cases are currently falsely marked due to component detection + { + code: normalizeIndent` + function ParentComponent() { + const _renderHeader = () =>
    ; + return
    {_renderHeader()}
    ; + } + ` + }, + { + // https://github.com/emotion-js/emotion/blob/a89d4257b0246a1640a99f77838e5edad4ec4386/packages/jest/test/react-enzyme.test.js#L26-L28 + code: normalizeIndent` + const testCases = { + basic: { + render() { + const Component = () =>
    ; + return
    ; + } + } + } + ` + }, + */ + { + code: normalizeIndent` + function ParentComponent() { + const rows = [ + { + name: 'A', + notPrefixedWithRender: (props) => + }, + ]; + return
    ; + } + `, + }, + + { + code: normalizeIndent` + function ParentComponent() { + return ( + + { + thing.match({ + loading: () =>
    , + success: () =>
    , + failure: () =>
    , + }) + } + + ) + } + `, + }, + { + code: normalizeIndent` + function ParentComponent() { + const thingElement = thing.match({ + loading: () =>
    , + success: () =>
    , + failure: () =>
    , + }); + return ( + + {thingElement} + + ) + } + `, + }, + // false-negative React.Component#render + { + code: normalizeIndent` + class ParentComponent extends React.Component { + render() { + const List = (props) => { + const items = props.items + .map((item) => ( +
  • + {item.name} +
  • + )); + return
      {items}
    ; + }; + return ; + } + } + `, + }, + { + code: normalizeIndent` + function ComponentForProps(props) { + return React.createElement("div", null); + } + function ParentComponent() { + return React.createElement(ComponentForProps, { + notPrefixedWithRender: () => React.createElement("div", null) + }); + } + `, + }, + { + code: normalizeIndent` + function ComponentForProps(props) { + return
    ; + } + function ParentComponent() { + return ( +
    } /> + ); + } + `, + }, + { + code: normalizeIndent` + function ComponentWithProps(props) { + return React.createElement("div", null); + } + function ParentComponent() { + return React.createElement(ComponentWithProps, { + footer: () => React.createElement("div", null) + }); + } + `, + }, + { + code: normalizeIndent` + function ComponentWithProps(props) { + return
    ; + } + function ParentComponent() { + return ( +
    } /> + ); + } + `, + }, + { + code: normalizeIndent` + class ParentComponent extends React.Component { + render() { + const UnstableNestedClassComponent = () => { + return React.createElement("div", null); + } + return React.createElement( + "div", + null, + React.createElement(UnstableNestedClassComponent, null) + ); + } + } + `, + }, + // false-negative React.Component#render + { + code: normalizeIndent` + class ParentComponent extends React.Component { + render() { + const UnstableNestedVariableComponent = () => { + return
    ; + } + return ( +
    + +
    + ); + } + } + `, + }, + // false-negative React.Component#render + { + code: normalizeIndent` + class ParentComponent extends React.Component { + render() { + function UnstableNestedClassComponent() { + return React.createElement("div", null); + } + return React.createElement( + "div", + null, + React.createElement(UnstableNestedClassComponent, null) + ); + } + } + `, + }, + // false-negative React.Component#render + { + code: normalizeIndent` + class ParentComponent extends React.Component { + render() { + function UnstableNestedFunctionComponent() { + return
    ; + } + return ( +
    + +
    + ); + } + } + `, + }, + // false-negative React.Component#render + { + code: normalizeIndent` + class ParentComponent extends React.Component { + render() { + class UnstableNestedClassComponent extends React.Component { + render() { + return React.createElement("div", null); + } + } + return React.createElement( + "div", + null, + React.createElement(UnstableNestedClassComponent, null) + ); + } + } + `, + }, + // false-negative React.Component#render + { + code: normalizeIndent` + class ParentComponent extends React.Component { + render() { + class UnstableNestedClassComponent extends React.Component { + render() { + return
    ; + } + }; + return ( +
    + +
    + ); + } + } + `, + }, + // false-negative ClassComponent declaration + { + code: normalizeIndent` + function ParentComponent() { + class UnstableNestedClassComponent extends React.Component { + render() { + return React.createElement("div", null); + } + } + return React.createElement( + "div", + null, + React.createElement(UnstableNestedClassComponent, null) + ); + } + `, + errors: [ + {messageId: 'declarationDuringRender', data: {displayName: 'Unknown'}}, + ], + }, + // false-negative ClassComponent declaration + { + code: normalizeIndent` + function ParentComponent() { + class UnstableNestedClassComponent extends React.Component { + render() { + return
    ; + } + }; + return ( +
    + +
    + ); + } + `, + errors: [ + {messageId: 'declarationDuringRender', data: {displayName: 'Unknown'}}, + ], + }, + // intentional false-negative consistent with rules-of-hooks + { + code: normalizeIndent` + export default () => { + function UnstableNestedFunctionComponent() { + return React.createElement("div", null); + } + return React.createElement( + "div", + null, + React.createElement(UnstableNestedFunctionComponent, null) + ); + }; + `, + }, + // intentional false-negative consistent with rules-of-hooks + { + code: normalizeIndent` + export default () => { + function UnstableNestedFunctionComponent() { + return
    ; + } + return ( +
    + +
    + ); + } + `, + }, + { + code: normalizeIndent` + const Component = React.memo(React.forwardRef(() => { + React.useRef(); + })) + `, + }, + { + code: normalizeIndent` + function useCustomHook() { + const useNestedHook = () => { + React.useRef(); + } + useNestedHook(); + useNestedHook(); + } + `, + }, + ], + invalid: [ + { + code: normalizeIndent` + function ParentComponent() { + function UnstableNestedFunctionComponent() { + return
    ; + } + return ( +
    + +
    + ); + } + `, + errors: [ + { + messageId: 'declarationDuringRender', + data: {displayName: 'UnstableNestedFunctionComponent'}, + }, + ], + }, + { + code: normalizeIndent` + function ParentComponent() { + function UnstableNestedFunctionComponent() { + return React.createElement("div", null); + } + return React.createElement( + "div", + null, + React.createElement(UnstableNestedFunctionComponent, null) + ); + } + `, + errors: [ + { + messageId: 'declarationDuringRender', + data: {displayName: 'UnstableNestedFunctionComponent'}, + }, + ], + }, + { + code: normalizeIndent` + function ParentComponent() { + const UnstableNestedVariableComponent = () => { + return
    ; + } + return ( +
    + +
    + ); + } + `, + errors: [ + { + messageId: 'declarationDuringRender', + data: {displayName: 'UnstableNestedVariableComponent'}, + }, + ], + }, + { + code: normalizeIndent` + function ParentComponent() { + const UnstableNestedVariableComponent = () => { + return React.createElement("div", null); + } + return React.createElement( + "div", + null, + React.createElement(UnstableNestedVariableComponent, null) + ); + } + `, + errors: [ + { + messageId: 'declarationDuringRender', + data: {displayName: 'UnstableNestedVariableComponent'}, + }, + ], + }, + { + code: normalizeIndent` + const ParentComponent = () => { + function UnstableNestedFunctionComponent() { + return
    ; + } + return ( +
    + +
    + ); + } + `, + errors: [ + { + messageId: 'declarationDuringRender', + data: {displayName: 'UnstableNestedFunctionComponent'}, + }, + ], + }, + { + code: normalizeIndent` + const ParentComponent = () => { + function UnstableNestedFunctionComponent() { + return React.createElement("div", null); + } + return React.createElement( + "div", + null, + React.createElement(UnstableNestedFunctionComponent, null) + ); + } + `, + errors: [ + { + messageId: 'declarationDuringRender', + data: {displayName: 'UnstableNestedFunctionComponent'}, + }, + ], + }, + { + code: normalizeIndent` + const ParentComponent = () => { + const UnstableNestedVariableComponent = () => { + return
    ; + } + return ( +
    + +
    + ); + } + `, + errors: [ + { + messageId: 'declarationDuringRender', + data: {displayName: 'UnstableNestedVariableComponent'}, + }, + ], + }, + { + code: normalizeIndent` + const ParentComponent = () => { + const UnstableNestedVariableComponent = () => { + return React.createElement("div", null); + } + return React.createElement( + "div", + null, + React.createElement(UnstableNestedVariableComponent, null) + ); + } + `, + errors: [ + { + messageId: 'declarationDuringRender', + data: {displayName: 'UnstableNestedVariableComponent'}, + }, + ], + }, + { + code: normalizeIndent` + function ParentComponent() { + function getComponent() { + function NestedUnstableFunctionComponent() { + return
    ; + }; + return ; + } + return ( +
    + {getComponent()} +
    + ); + } + `, + errors: [ + { + messageId: 'declarationDuringRender', + data: {displayName: 'NestedUnstableFunctionComponent'}, + }, + ], + }, + { + code: normalizeIndent` + function ParentComponent() { + function getComponent() { + function NestedUnstableFunctionComponent() { + return React.createElement("div", null); + } + return React.createElement(NestedUnstableFunctionComponent, null); + } + return React.createElement("div", null, getComponent()); + } + `, + errors: [ + { + messageId: 'declarationDuringRender', + data: {displayName: 'NestedUnstableFunctionComponent'}, + }, + ], + }, + { + code: normalizeIndent` + function ComponentWithProps(props) { + return
    ; + } + function ParentComponent() { + return ( + ; + } + } /> + ); + } + `, + errors: [ + { + messageId: 'declarationDuringRender', + data: {displayName: 'SomeFooter'}, + }, + ], + }, + { + code: normalizeIndent` + function ComponentWithProps(props) { + return React.createElement("div", null); + } + function ParentComponent() { + return React.createElement(ComponentWithProps, { + footer: function SomeFooter() { + return React.createElement("div", null); + } + }); + } + `, + errors: [ + { + messageId: 'declarationDuringRender', + data: {displayName: 'SomeFooter'}, + }, + ], + }, + { + code: normalizeIndent` + function ParentComponent() { + return ( + + {() => { + function UnstableNestedComponent() { + return
    ; + } + return ( +
    + +
    + ); + }} + + ); + } + `, + errors: [ + { + messageId: 'declarationDuringRender', + data: {displayName: 'UnstableNestedComponent'}, + }, + ], + }, + { + code: normalizeIndent` + function RenderPropComponent(props) { + return props.render({}); + } + function ParentComponent() { + return React.createElement( + RenderPropComponent, + null, + () => { + function UnstableNestedComponent() { + return React.createElement("div", null); + } + return React.createElement( + "div", + null, + React.createElement(UnstableNestedComponent, null) + ); + } + ); + } + `, + errors: [ + { + messageId: 'declarationDuringRender', + data: {displayName: 'UnstableNestedComponent'}, + }, + ], + }, + { + code: normalizeIndent` + function ParentComponent() { + return ( +
    }} /> + ); + } + `, + errors: [ + {messageId: 'declarationDuringRender', data: {displayName: 'Header'}}, + ], + }, + { + code: normalizeIndent` + function ParentComponent() { + const UnstableNestedComponent = React.memo(() => { + return
    ; + }); + return ( +
    + +
    + ); + } + `, + errors: [ + {messageId: 'declarationDuringRender', data: {displayName: 'Unknown'}}, + ], + }, + { + code: normalizeIndent` + function ParentComponent() { + const UnstableNestedComponent = React.memo( + () => React.createElement("div", null), + ); + return React.createElement( + "div", + null, + React.createElement(UnstableNestedComponent, null) + ); + } + `, + errors: [ + {messageId: 'declarationDuringRender', data: {displayName: 'Unknown'}}, + ], + }, + { + code: normalizeIndent` + function ParentComponent() { + const UnstableNestedComponent = React.memo( + function () { + return
    ; + } + ); + return ( +
    + +
    + ); + } + `, + errors: [ + {messageId: 'declarationDuringRender', data: {displayName: 'Unknown'}}, + ], + }, + { + code: normalizeIndent` + function ParentComponent() { + const UnstableNestedComponent = React.memo( + function () { + return React.createElement("div", null); + } + ); + return React.createElement( + "div", + null, + React.createElement(UnstableNestedComponent, null) + ); + } + `, + errors: [ + {messageId: 'declarationDuringRender', data: {displayName: 'Unknown'}}, + ], + }, + { + code: normalizeIndent` + function ParentComponent() { + return ( +
    }} /> + ) + } + `, + errors: [ + {messageId: 'declarationDuringRender', data: {displayName: 'Header'}}, + ], + }, + { + code: normalizeIndent` + function ParentComponent(props) { + return React.createElement( + "ul", + null, + props.items.map(function Item() { + return React.createElement( + "li", + { key: item.id }, + item.name + ); + }) + ); + } + `, + errors: [ + {messageId: 'declarationDuringRender', data: {displayName: 'Item'}}, + ], + }, + { + code: normalizeIndent` + function ParentComponent(props) { + return ( +
      + {props.items.map(function Item(item) { + return ( +
    • + {item.name} +
    • + ); + })} +
    + ); + } + `, + errors: [ + {messageId: 'declarationDuringRender', data: {displayName: 'Item'}}, + ], + }, + { + code: normalizeIndent` + function ParentComponent() { + const UnstableNestedFunctionComponent = React.forwardRef(function UnstableNestedFunctionComponent(props, ref) { + return
    ; + }); + return ( +
    + +
    + ); + } + `, + errors: [ + { + messageId: 'declarationDuringRender', + data: {displayName: 'UnstableNestedFunctionComponent'}, + }, + ], + }, + { + code: normalizeIndent` + function ParentComponent() { + const MemoizedComponentn = React.useCallback(function Component() {}); + return ( + + ); + } + `, + errors: [ + { + messageId: 'declarationDuringRender', + data: {displayName: 'Component'}, + }, + ], + }, + ], +}; + +// Tests that are only valid/invalid across parsers supporting Flow +const testsFlow = { + valid: [], + invalid: [], +}; + +// Tests that are only valid/invalid across parsers supporting TypeScript +const testsTypescript = { + valid: [], + invalid: [], +}; + +// For easier local testing +if (!process.env.CI) { + let only = []; + let skipped = []; + [ + ...tests.valid, + ...tests.invalid, + ...testsFlow.valid, + ...testsFlow.invalid, + ...testsTypescript.valid, + ...testsTypescript.invalid, + ].forEach(t => { + if (t.skip) { + delete t.skip; + skipped.push(t); + } + if (t.only) { + delete t.only; + only.push(t); + } + }); + const predicate = t => { + if (only.length > 0) { + return only.indexOf(t) !== -1; + } + if (skipped.length > 0) { + return skipped.indexOf(t) === -1; + } + return true; + }; + tests.valid = tests.valid.filter(predicate); + tests.invalid = tests.invalid.filter(predicate); + testsFlow.valid = testsFlow.valid.filter(predicate); + testsFlow.invalid = testsFlow.invalid.filter(predicate); + testsTypescript.valid = testsTypescript.valid.filter(predicate); + testsTypescript.invalid = testsTypescript.invalid.filter(predicate); +} + +describe('react-hooks', () => { + const parserOptions = { + ecmaFeatures: { + jsx: true, + }, + ecmaVersion: 6, + sourceType: 'module', + }; + + const testsBabelEslint = { + valid: [...testsFlow.valid, ...tests.valid], + invalid: [...testsFlow.invalid, ...tests.invalid], + }; + + new ESLintTester({ + parser: require.resolve('babel-eslint'), + parserOptions, + }).run('parser: babel-eslint', ReactHooksESLintRule, testsBabelEslint); + + // new ESLintTester({ + // parser: require.resolve('@babel/eslint-parser'), + // parserOptions, + // }).run( + // 'parser: @babel/eslint-parser', + // ReactHooksESLintRule, + // testsBabelEslint + // ); + + // const testsTypescriptEslintParser = { + // valid: [...testsTypescript.valid, ...tests.valid], + // invalid: [...testsTypescript.invalid, ...tests.invalid], + // }; + + // new ESLintTester({ + // parser: require.resolve('@typescript-eslint/parser-v2'), + // parserOptions, + // }).run( + // 'parser: @typescript-eslint/parser@2.x', + // ReactHooksESLintRule, + // testsTypescriptEslintParser + // ); + + // new ESLintTester({ + // parser: require.resolve('@typescript-eslint/parser-v3'), + // parserOptions, + // }).run( + // 'parser: @typescript-eslint/parser@3.x', + // ReactHooksESLintRule, + // testsTypescriptEslintParser + // ); + + // new ESLintTester({ + // parser: require.resolve('@typescript-eslint/parser-v5'), + // parserOptions, + // }).run('parser: @typescript-eslint/parser@^5.0.0-0', ReactHooksESLintRule, { + // valid: testsTypescriptEslintParser.valid, + // invalid: testsTypescriptEslintParser.invalid, + // }); +}); diff --git a/packages/eslint-plugin-react-hooks/src/NoNestedComponents.js b/packages/eslint-plugin-react-hooks/src/NoNestedComponents.js new file mode 100644 index 000000000000..fce2e5de806d --- /dev/null +++ b/packages/eslint-plugin-react-hooks/src/NoNestedComponents.js @@ -0,0 +1,266 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/* eslint-disable no-for-of-loops/no-for-of-loops */ + +'use strict'; + +/** + * Catch all identifiers that begin with "use" followed by an uppercase Latin + * character to exclude identifiers like "user". + */ + +function isHookName(s) { + return /^use[A-Z0-9]/.test(s); +} + +/** + * We consider hooks to be a hook name identifier or a member expression + * containing a hook name. + */ + +function isHook(node) { + if (node.type === 'Identifier') { + return isHookName(node.name); + } else if ( + node.type === 'MemberExpression' && + !node.computed && + isHook(node.property) + ) { + const obj = node.object; + const isPascalCaseNameSpace = /^[A-Z].*/; + return obj.type === 'Identifier' && isPascalCaseNameSpace.test(obj.name); + } else { + return false; + } +} + +/** + * Checks if the node is a React component name. React component names must + * always start with an uppercase letter. + */ + +function isComponentName(node) { + return node.type === 'Identifier' && /^[A-Z]/.test(node.name); +} + +function isReactFunction(node, functionName) { + return ( + node.name === functionName || + (node.type === 'MemberExpression' && + node.object.name === 'React' && + node.property.name === functionName) + ); +} + +/** + * Checks if the node is a callback argument of forwardRef. This render function + * should follow the rules of hooks. + */ + +function isForwardRefCallback(node) { + return !!( + node.parent && + node.parent.callee && + isReactFunction(node.parent.callee, 'forwardRef') + ); +} + +/** + * Checks if the node is a callback argument of React.memo. This anonymous + * functional component should follow the rules of hooks. + */ + +function isMemoCallback(node) { + return !!( + node.parent && + node.parent.callee && + isReactFunction(node.parent.callee, 'memo') + ); +} + +function isInsideComponentOrHook(node) { + while (node) { + const functionName = getFunctionName(node); + if (functionName) { + if (isComponentName(functionName) || isHook(functionName)) { + return true; + } + } + if (isForwardRefCallback(node) || isMemoCallback(node)) { + return true; + } + node = node.parent; + } + return false; +} + +function isComponent(node) { + const functionName = getFunctionName(node); + if (functionName) { + if (isComponentName(functionName)) { + return true; + } + } + + if (isForwardRefCallback(node) || isMemoCallback(node)) { + return true; + } + return false; +} + +function isNestedComponentDeclaration(node) { + return ( + isComponent(node) && + isInsideComponentOrHook(node.parent) && + !isForwardRefCallback(node.parent) && + !isMemoCallback(node.parent) + ); +} + +export default { + meta: { + type: 'problem', + docs: { + description: 'Ensures all component definitions ensure persistent state.', + recommended: true, + url: + 'https://github.com/facebook/react/tree/main/packages/eslint-plugin-react-hooks#no-nested-components', + }, + messages: { + declarationDuringRender: + `Component "{{ displayName }}" is declared during render. ` + + "You should move this declaration outside of render to ensure this component's state is persisted across re-renders of its parent. " + + 'If this component is memoized, you should still refactor the component to be able to move it outside of render. ' + + 'If you want to reset its state use a key instead (see https://reactjs.org/docs/lists-and-keys.html#keys).', + }, + }, + create(context) { + return { + FunctionDeclaration(node) { + // function MyComponent() { const onClick = useEvent(...) } + if (isNestedComponentDeclaration(node)) { + context.report({ + node: node, + messageId: 'declarationDuringRender', + data: { + displayName: getDisplayName(node), + }, + }); + } + }, + FunctionExpression(node) { + // const MyComponent = function MyComponent() { const onClick = useEvent(...) } + if (isNestedComponentDeclaration(node)) { + context.report({ + node: node, + messageId: 'declarationDuringRender', + data: { + displayName: getDisplayName(node), + }, + }); + } + }, + ArrowFunctionExpression(node) { + // const MyComponent = () => { const onClick = useEvent(...) } + if (isNestedComponentDeclaration(node)) { + context.report({ + node: node, + messageId: 'declarationDuringRender', + data: { + displayName: getDisplayName(node), + }, + }); + } + }, + }; + }, +}; + +function getDisplayName(node) { + if ( + node.type === 'ArrowFunctionExpression' || + node.type === 'FunctionDeclaration' || + node.type === 'FunctionExpression' + ) { + const functionName = getFunctionName(node); + if (functionName !== undefined && functionName.name) { + return functionName.name; + } + } + return 'Unknown'; +} + +/** + * Gets the static name of a function AST node. For function declarations it is + * easy. For anonymous function expressions it is much harder. If you search for + * `IsAnonymousFunctionDefinition()` in the ECMAScript spec you'll find places + * where JS gives anonymous function expressions names. We roughly detect the + * same AST nodes with some exceptions to better fit our use case. + */ + +function getFunctionName(node) { + if ( + node.type === 'FunctionDeclaration' || + (node.type === 'FunctionExpression' && node.id) + ) { + // function useHook() {} + // const whatever = function useHook() {}; + // + // Function declaration or function expression names win over any + // assignment statements or other renames. + return node.id; + } else if ( + node.type === 'FunctionExpression' || + node.type === 'ArrowFunctionExpression' + ) { + if ( + node.parent.type === 'VariableDeclarator' && + node.parent.init === node + ) { + // const useHook = () => {}; + return node.parent.id; + } else if ( + node.parent.type === 'AssignmentExpression' && + node.parent.right === node && + node.parent.operator === '=' + ) { + // useHook = () => {}; + return node.parent.left; + } else if ( + node.parent.type === 'Property' && + node.parent.value === node && + !node.parent.computed + ) { + // {useHook: () => {}} + // {useHook() {}} + return node.parent.key; + + // NOTE: We could also support `ClassProperty` and `MethodDefinition` + // here to be pedantic. However, hooks in a class are an anti-pattern. So + // we don't allow it to error early. + // + // class {useHook = () => {}} + // class {useHook() {}} + } else if ( + node.parent.type === 'AssignmentPattern' && + node.parent.right === node && + !node.parent.computed + ) { + // const {useHook = () => {}} = {}; + // ({useHook = () => {}} = {}); + // + // Kinda clowny, but we'd said we'd follow spec convention for + // `IsAnonymousFunctionDefinition()` usage. + return node.parent.left; + } else { + return undefined; + } + } else { + return undefined; + } +} diff --git a/packages/eslint-plugin-react-hooks/src/index.js b/packages/eslint-plugin-react-hooks/src/index.js index 88e1ae71a2a7..62d7338f68b6 100644 --- a/packages/eslint-plugin-react-hooks/src/index.js +++ b/packages/eslint-plugin-react-hooks/src/index.js @@ -7,6 +7,7 @@ 'use strict'; +import NoNestedComponents from './NoNestedComponents'; import RulesOfHooks from './RulesOfHooks'; import ExhaustiveDeps from './ExhaustiveDeps'; @@ -14,6 +15,7 @@ export const configs = { recommended: { plugins: ['react-hooks'], rules: { + 'react-hooks/no-nested-components': 'error', 'react-hooks/rules-of-hooks': 'error', 'react-hooks/exhaustive-deps': 'warn', }, @@ -21,6 +23,7 @@ export const configs = { }; export const rules = { + 'no-nested-components': NoNestedComponents, 'rules-of-hooks': RulesOfHooks, 'exhaustive-deps': ExhaustiveDeps, };