diff --git a/README.md b/README.md index 3a06ef705d..8a10fa342b 100644 --- a/README.md +++ b/README.md @@ -131,6 +131,7 @@ Enable the rules that you would like to use. * [react/no-unescaped-entities](docs/rules/no-unescaped-entities.md): Detect unescaped HTML entities, which might represent malformed tags * [react/no-unknown-property](docs/rules/no-unknown-property.md): Prevent usage of unknown DOM property (fixable) * [react/no-unsafe](docs/rules/no-unsafe.md): Prevent usage of unsafe lifecycle methods +* [react/no-unstable-nested-components](docs/rules/no-unstable-nested-components.md): Prevent creating unstable components inside components * [react/no-unused-prop-types](docs/rules/no-unused-prop-types.md): Prevent definitions of unused prop types * [react/no-unused-state](docs/rules/no-unused-state.md): Prevent definition of unused state fields * [react/no-will-update-set-state](docs/rules/no-will-update-set-state.md): Prevent usage of setState in componentWillUpdate diff --git a/docs/rules/no-unstable-nested-components.md b/docs/rules/no-unstable-nested-components.md new file mode 100644 index 0000000000..b1a0e250ba --- /dev/null +++ b/docs/rules/no-unstable-nested-components.md @@ -0,0 +1,79 @@ +# Prevent creating unstable components inside components (react/no-unstable-nested-components) + +Creating components inside components without memoization leads to unstable components. The nested component and all its children are recreated during each re-render. Given stateful children of the nested component will lose their state on each re-render. + +React reconcilation performs element type comparison with [reference equality](https://github.com/facebook/react/blob/v16.13.1/packages/react-reconciler/src/ReactChildFiber.js#L407). 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). + +## Rule Details + +The following patterns are considered warnings: + +```jsx +function Component() { + function UnstableNestedComponent() { + return
; + } + + return ( +
+ +
+ ); +} +``` + +```jsx +class Component extends React.Component { + render() { + function UnstableNestedComponent() { + return
; + } + + return ( +
+ +
+ ); + } +} +``` + +The following patterns are **not** considered warnings: + +```jsx +function OutsideDefinedComponent(props) { + return
; +} + +function Component() { + return ( +
+ +
+ ); +} +``` + +```jsx +function Component() { + const MemoizedNestedComponent = React.useCallback(() =>
, []); + + return ( +
+ +
+ ); +} +``` + +## Rule Options + +```js +... +"react/no-unstable-nested-components": "off" | "warn" | "error" +... +``` + +## When Not To Use It + +If you are not interested in preventing bugs related to re-creation of the nested components or do not care about optimization of virtual DOM. diff --git a/index.js b/index.js index bc0c30a43d..e40300d7d5 100644 --- a/index.js +++ b/index.js @@ -74,6 +74,7 @@ const allRules = { 'no-unescaped-entities': require('./lib/rules/no-unescaped-entities'), 'no-unknown-property': require('./lib/rules/no-unknown-property'), 'no-unsafe': require('./lib/rules/no-unsafe'), + 'no-unstable-nested-components': require('./lib/rules/no-unstable-nested-components'), 'no-unused-prop-types': require('./lib/rules/no-unused-prop-types'), 'no-unused-state': require('./lib/rules/no-unused-state'), 'no-will-update-set-state': require('./lib/rules/no-will-update-set-state'), diff --git a/lib/rules/no-unstable-nested-components.js b/lib/rules/no-unstable-nested-components.js new file mode 100644 index 0000000000..baaf366700 --- /dev/null +++ b/lib/rules/no-unstable-nested-components.js @@ -0,0 +1,112 @@ +/** + * @fileoverview Prevent creating unstable components inside components + * @author Ari Perkkiƶ + */ + +'use strict'; + +const Components = require('../util/Components'); +const docsUrl = require('../util/docsUrl'); + +// ------------------------------------------------------------------------------ +// Constants +// ------------------------------------------------------------------------------ + +const ERROR_MESSAGE = 'Do not create unstable nested components. Declare them outside the component or memoize them.'; + +// ------------------------------------------------------------------------------ +// Rule Definition +// ------------------------------------------------------------------------------ + +module.exports = { + meta: { + docs: { + description: 'Prevent creating unstable components inside components', + category: 'Possible Errors', + recommended: false, + url: docsUrl('no-unstable-nested-components') + }, + schema: [] + }, + + create: Components.detect((context, components, utils) => { + /** + * Check whether given node is declared inside another component + * @param {ASTNode} node The AST node being checked + * @returns {Boolean} True if node has a parent component, false if not + */ + function isAnyParentComponent(node) { + if (!node.parent || node.parent.type === 'Program') { + return false; + } + + return components.get(node.parent) || isAnyParentComponent(node.parent); + } + + /** + * Check whether given node is inside class component's render block + * @param {ASTNode} node The AST node being checked + * @returns {Boolean} True if node is inside class component's render block, false if not + */ + function isInsideRenderMethod(node) { + const parentComponent = utils.getParentComponent(); + if (!parentComponent || parentComponent.type !== 'ClassDeclaration') { + return false; + } + + return ( + node.parent + && node.parent.type === 'MethodDefinition' + && node.parent.key + && node.parent.key.name === 'render' + ); + } + + /** + * Check whether current node is a stateless component declared inside class component. + * Util's component detection fails to detect stateless components inside class components. + * @returns {Boolean} True if current node a stateless component declared inside class component, false if not + */ + function isStatelessComponentInsideClassComponent() { + const parentComponent = utils.getParentComponent(); + const parentStatelessComponent = utils.getParentStatelessComponent(); + + return ( + parentComponent + && parentStatelessComponent + && parentComponent.type === 'ClassDeclaration' + && utils.getStatelessComponent(parentStatelessComponent) + ); + } + + /** + * Check whether given node is a unstable nested component + * @param {ASTNode} node The AST node being checked + */ + function validate(node) { + if (!components.get(node) && !isStatelessComponentInsideClassComponent()) { + return; + } + + // Prevent reporting nested class components twice + if (isInsideRenderMethod(node)) { + return; + } + + if (isAnyParentComponent(node)) { + context.report({node, message: ERROR_MESSAGE}); + } + } + + // -------------------------------------------------------------------------- + // Public + // -------------------------------------------------------------------------- + + return { + FunctionDeclaration(node) { validate(node); }, + ArrowFunctionExpression(node) { validate(node); }, + FunctionExpression(node) { validate(node); }, + ClassDeclaration(node) { validate(node); } + }; + }) +}; diff --git a/tests/lib/rules/no-unstable-nested-components.js b/tests/lib/rules/no-unstable-nested-components.js new file mode 100644 index 0000000000..18a7fd558e --- /dev/null +++ b/tests/lib/rules/no-unstable-nested-components.js @@ -0,0 +1,332 @@ +/** + * @fileoverview Prevent creating unstable components inside components + * @author Ari Perkkiƶ + */ + +'use strict'; + +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +const RuleTester = require('eslint').RuleTester; +const rule = require('../../../lib/rules/no-unstable-nested-components'); + +const parserOptions = { + ecmaVersion: 2018, + sourceType: 'module', + ecmaFeatures: { + jsx: true + } +}; + +const ERROR_MESSAGE = 'Do not create unstable nested components. Declare them outside the component or memoize them.'; + +// ------------------------------------------------------------------------------ +// Tests +// ------------------------------------------------------------------------------ + +const ruleTester = new RuleTester({parserOptions}); +ruleTester.run('no-unstable-nested-components', rule, { + valid: [ + { + code: ` + function OutsideDefinedFunctionComponent() { + return
; + } + + function Component() { + return ( +
+ +
+ ); + }` + }, + { + code: ` + const OutsideDefinedVariableComponent = () => { + return
; + } + + function Component() { + return ( +
+ +
+ ); + }` + }, + { + code: ` + class OutsideDefinedClassComponent extends React.Component { + render() { + return
; + } + } + + function Component() { + return ( +
+ +
+ ); + }` + }, + { + code: ` + function Component() { + const MemoizedNestedComponent = React.useCallback(() =>
, []); + + return ( +
+ +
+ ); + }` + }, + { + code: ` + function Component(props) { + // Should not interfere handler declarations + function onClick(event) { + props.onClick(event.target.value); + } + + return ( +
+
+ ); + } + ` + }, + { + code: ` + function Component() { + // This is fine, as the returned element has same reference + function getComponent() { + return
; + } + + return ( +
+ {getComponent()} +
+ ); + } + ` + } + ], + + invalid: [ + { + code: ` + function Component() { + function UnstableNestedFunctionComponent() { + return
; + } + + return ( +
+ +
+ ); + }`, + errors: [ + { + message: ERROR_MESSAGE + } + ] + }, + { + code: ` + function Component() { + const UnstableNestedVariableComponent = () => { + return
; + } + + return ( +
+ +
+ ); + }`, + errors: [ + { + message: ERROR_MESSAGE + } + ] + }, + { + code: ` + const Component = () => { + function UnstableNestedFunctionComponent() { + return
; + } + + return ( +
+ +
+ ); + }`, + errors: [ + { + message: ERROR_MESSAGE + } + ] + }, + { + code: ` + const Component = () => { + const UnstableNestedVariableComponent = () => { + return
; + } + + return ( +
+ +
+ ); + }`, + errors: [ + { + message: ERROR_MESSAGE + } + ] + }, + { + code: ` + function Component() { + class UnstableNestedClassComponent extends React.Component { + render() { + return
; + } + }; + + return ( +
+ +
+ ); + }`, + errors: [ + { + message: ERROR_MESSAGE + } + ] + }, + { + code: ` + class Component extends React.Component { + render() { + class UnstableNestedClassComponent extends React.Component { + render() { + return
; + } + }; + + return ( +
+ +
+ ); + } + }`, + errors: [ + { + message: ERROR_MESSAGE + } + ] + }, + { + code: ` + class Component extends React.Component { + render() { + function UnstableNestedFunctionComponent() { + return
; + } + + return ( +
+ +
+ ); + } + }`, + errors: [ + { + message: ERROR_MESSAGE + } + ] + }, + { + code: ` + class Component extends React.Component { + render() { + const UnstableNestedVariableComponent = () => { + return
; + } + + return ( +
+ +
+ ); + } + }`, + errors: [ + { + message: ERROR_MESSAGE + } + ] + }, + { + code: ` + function Component() { + function getComponent() { + function NestedUnstableFunctionComponent() { + return
; + }; + + return ; + }; + + return ( +
+ {getComponent()} +
+ ); + } + `, + errors: [ + { + message: ERROR_MESSAGE + } + ] + }, + { + code: ` + function Component() { + return ( +
+ {(() => { + function InlinedJSXComponent() { + return
; + }; + + return ; + })()} +
+ ); + } + `, + errors: [ + { + message: ERROR_MESSAGE + } + ] + } + ] +});