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..3134ec43e5 --- /dev/null +++ b/docs/rules/no-unstable-nested-components.md @@ -0,0 +1,135 @@ +# 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 + +function SomeComponent({ footer: Footer }) { + return ( +
+
+
+ ); +} + +function Component() { + 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 ( +
+ +
+ ); +} +``` + +By default component creation is allowed inside component props only if prop name starts with `render`. See `allowAsProps` option for disabling this limitation completely. + +```jsx +function SomeComponent(props) { + return
{props.renderFooter()}
; +} + +function Component() { + return ( +
+
} /> +
+ ); +} +``` + +## Rule Options + +```js +... +"react/no-unstable-nested-components": [ + "off" | "warn" | "error", + { "allowAsProps": true | false } +] +... +``` + +You can allow component creation inside component props by setting `allowAsProps` option to true. When using this option make sure you are **calling** the props in the receiving component and not using them as elements. + +The following patterns are **not** considered warnings: + +```jsx +function SomeComponent(props) { + return
{props.footer()}
; +} + +function Component() { + return ( +
+
} /> +
+ ); +} +``` + +## 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..0ed086f1bb --- /dev/null +++ b/lib/rules/no-unstable-nested-components.js @@ -0,0 +1,227 @@ +/** + * @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_WITHOUT_NAME = 'Declare this component outside parent component or memoize it.'; +const COMPONENT_AS_PROPS_INFO = ' If you want to allow component creation in props, set allowAsProps option to true.'; +const RENDER_REGEXP = /^render/; + +function generateErrorMessageWithParentName(parentName) { + return `Declare this component outside parent component "${parentName}" or memoize it.`; +} + +// ------------------------------------------------------------------------------ +// 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: [{ + type: 'object', + properties: { + customValidators: { + type: 'array', + items: { + type: 'string' + } + }, + allowAsProps: { + type: 'boolean' + } + }, + additionalProperties: false + }] + }, + + create: Components.detect((context, components, utils) => { + const allowAsProps = context.options.some((option) => option && option.allowAsProps); + + /** + * Get the closest parent component of the given node if any exist + * @param {ASTNode} node The AST node + * @returns {ASTNode} Node of the closest parent component, if any. + */ + function getClosestParentComponentNode(node) { + if (!node || !node.parent || node.parent.type === 'Program') { + return; + } + + if (components.get(node.parent)) { + return node.parent; + } + + return getClosestParentComponentNode(node.parent); + } + + /** + * Resolve the component name of given node + * @param {ASTNode} node The AST node of the component + * @returns {String} Name of the component, if any + */ + function resolveComponentName(node) { + const parentName = node.id && node.id.name; + if (parentName) return parentName; + + return ( + node.type === 'ArrowFunctionExpression' + && node.parent + && node.parent.id + && node.parent.id.name + ); + } + + /** + * Check whether given node is declared inside class component's render block + * ```jsx + * class Component extends React.Component { + * render() { + * class NestedClassComponent extends React.Component { + * ... + * ``` + * @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.type === 'MethodDefinition' + && node.parent.key + && node.parent.key.name === 'render' + ); + } + + /** + * Check whether current node is a function component declared inside class component. + * Util's component detection fails to detect function components inside class components. + * ```jsx + * class Component extends React.Component { + * render() { + * const NestedComponent = () =>
; + * ... + * ``` + * @param {ASTNode} node The AST node being checked + * @returns {Boolean} True if current node a function component declared inside class component, false if not + */ + function isFunctionComponentInsideClassComponent(node) { + const parentComponent = utils.getParentComponent(); + const parentStatelessComponent = utils.getParentStatelessComponent(); + + return ( + parentComponent + && parentStatelessComponent + && parentComponent.type === 'ClassDeclaration' + && utils.getStatelessComponent(parentStatelessComponent) + && utils.isReturningJSX(node) + ); + } + + /** + * Check whether given node is declared inside a component prop. + * Props prefixed with `render` are allowed. All prop names can be allowed with `allowAsProps` option. + * ```jsx + *
} /> + * ``` + * @param {ASTNode} node The AST node being checked + * @returns {Boolean} True if node is a component declared inside prop, false if not + */ + function isComponentInProp(node) { + if (allowAsProps || node.parent.type !== 'JSXExpressionContainer') { + return false; + } + + // Check whether component is a render prop, e.g. {() =>
} + const propNode = node.parent.parent; + if (propNode && propNode.type === 'JSXElement') { + return false; + } + + // Check whether prop name starts with render, e.g.
} /> + if ( + propNode + && propNode.type === 'JSXAttribute' + && propNode.name + && propNode.name.type === 'JSXIdentifier' + && RENDER_REGEXP.test(propNode.name.name)) { + return false; + } + + return utils.isReturningJSX(node); + } + + /** + * Check whether given node is a unstable nested component + * @param {ASTNode} node The AST node being checked + */ + function validate(node) { + if (!node || !node.parent) { + return; + } + + const isDeclaredInsideProps = isComponentInProp(node); + if ( + !components.get(node) + && !isFunctionComponentInsideClassComponent(node) + && !isDeclaredInsideProps) { + return; + } + + // Prevent reporting nested class components twice + if (isInsideRenderMethod(node)) { + return; + } + + const parentComponent = getClosestParentComponentNode(node); + if (parentComponent) { + const parentName = resolveComponentName(parentComponent); + + // Exclude lowercase parents, e.g. function createTestComponent() + // React-dom prevents creating lowercase components + if (parentName && parentName[0] === parentName[0].toLowerCase()) { + return; + } + + let message = parentName + ? generateErrorMessageWithParentName(parentName) + : ERROR_MESSAGE_WITHOUT_NAME; + + // Add information about allowAsProps option when component is declared inside prop + if (isDeclaredInsideProps && !allowAsProps) { + message += COMPONENT_AS_PROPS_INFO; + } + + context.report({node, 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..c7e0d0c1cc --- /dev/null +++ b/tests/lib/rules/no-unstable-nested-components.js @@ -0,0 +1,585 @@ +/** + * @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 = 'Declare this component outside parent component "ParentComponent" or memoize it.'; +const ERROR_MESSAGE_WITHOUT_NAME = 'Declare this component outside parent component or memoize it.'; +const ERROR_MESSAGE_COMPONENT_AS_PROPS = `${ERROR_MESSAGE} If you want to allow component creation in props, set allowAsProps option to true.`; + +// ------------------------------------------------------------------------------ +// Tests +// ------------------------------------------------------------------------------ + +const ruleTester = new RuleTester({parserOptions}); +ruleTester.run('no-unstable-nested-components', rule, { + valid: [ + { + code: ` + function OutsideDefinedFunctionComponent() { + return
; + } + + function ParentComponent() { + return ( +
+ +
+ ); + } + ` + }, + { + code: ` + const OutsideDefinedVariableComponent = () => { + return
; + } + + function ParentComponent() { + return ( +
+ +
+ ); + } + ` + }, + { + code: ` + class OutsideDefinedClassComponent extends React.Component { + render() { + return
; + } + } + + function ParentComponent() { + return ( +
+ +
+ ); + } + ` + }, + { + code: ` + function OutsideDefinedComponentForProps() { + return
; + } + + function ComponentWithProps(props) { + return
; + } + + function ParentComponent() { + return ( + } + header={
} + /> + ); + } + ` + }, + { + code: ` + function ParentComponent() { + const MemoizedNestedVariableComponent = React.useCallback(() =>
, []); + + return ( +
+ +
+ ); + } + ` + }, + { + code: ` + function ParentComponent() { + const MemoizedNestedFunctionComponent = React.useCallback( + function () { + return
; + }, + [] + ); + + return ( +
+ +
+ ); + } + ` + }, + { + code: ` + function ParentComponent(props) { + // Should not interfere handler declarations + function onClick(event) { + props.onClick(event.target.value); + } + + function getOnHover() { + return function onHover(event) { + props.onHover(event.target); + } + } + + return ( +
+
+ ); + } + ` + }, + { + code: ` + function ParentComponent() { + function getComponent() { + return
; + } + + return ( +
+ {getComponent()} +
+ ); + } + ` + }, + { + code: ` + function RenderPropComponent(props) { + return props.render({}); + } + + function ParentComponent() { + return ( + + {() =>
} + + ); + } + ` + }, + { + code: ` + function ParentComponent(props) { + return ( +
    + {props.items.map(item => ( +
  • + {item.name} +
  • + ))} +
+ ); + } + ` + }, + { + code: ` + function ParentComponent(props) { + return ( +
    + {props.items.map(function Item(item) { + return ( +
  • + {item.name} +
  • + ); + })} +
+ ); + } + ` + }, + { + code: ` + function createTestComponent(props) { + return ( +
+ ); + } + ` + }, + { + code: ` + function ComponentWithProps(props) { + return
{props.footer()}
; + } + + function ParentComponent() { + return ( +
} /> + ); + } + `, + options: [{ + allowAsProps: true + }] + }, + { + code: ` + function ComponentForProps(props) { + return
; + } + + function ParentComponent() { + return ( +
} /> + ); + } + ` + } + ], + + invalid: [ + { + code: ` + function ParentComponent() { + function UnstableNestedFunctionComponent() { + return
; + } + + return ( +
+ +
+ ); + } + `, + errors: [ + { + message: ERROR_MESSAGE + } + ] + }, + { + code: ` + function ParentComponent() { + const UnstableNestedVariableComponent = () => { + return
; + } + + return ( +
+ +
+ ); + } + `, + errors: [ + { + message: ERROR_MESSAGE + } + ] + }, + { + code: ` + const ParentComponent = () => { + function UnstableNestedFunctionComponent() { + return
; + } + + return ( +
+ +
+ ); + } + `, + errors: [ + { + message: ERROR_MESSAGE + } + ] + }, + { + code: ` + export default () => { + function UnstableNestedFunctionComponent() { + return
; + } + + return ( +
+ +
+ ); + } + `, + errors: [ + { + message: ERROR_MESSAGE_WITHOUT_NAME + } + ] + }, + { + code: ` + const ParentComponent = () => { + const UnstableNestedVariableComponent = () => { + return
; + } + + return ( +
+ +
+ ); + } + `, + errors: [ + { + message: ERROR_MESSAGE + } + ] + }, + { + code: ` + function ParentComponent() { + class UnstableNestedClassComponent extends React.Component { + render() { + return
; + } + }; + + return ( +
+ +
+ ); + } + `, + errors: [ + { + message: ERROR_MESSAGE + } + ] + }, + { + code: ` + class ParentComponent extends React.Component { + render() { + class UnstableNestedClassComponent extends React.Component { + render() { + return
; + } + }; + + return ( +
+ +
+ ); + } + } + `, + errors: [ + { + message: ERROR_MESSAGE + } + ] + }, + { + code: ` + class ParentComponent extends React.Component { + render() { + function UnstableNestedFunctionComponent() { + return
; + } + + return ( +
+ +
+ ); + } + } + `, + errors: [ + { + message: ERROR_MESSAGE + } + ] + }, + { + code: ` + class ParentComponent extends React.Component { + render() { + const UnstableNestedVariableComponent = () => { + return
; + } + + return ( +
+ +
+ ); + } + } + `, + errors: [ + { + message: ERROR_MESSAGE + } + ] + }, + { + code: ` + function ParentComponent() { + function getComponent() { + function NestedUnstableFunctionComponent() { + return
; + }; + + return ; + }; + + return ( +
+ {getComponent()} +
+ ); + } + `, + errors: [ + { + message: ERROR_MESSAGE + } + ] + }, + { + code: ` + function ParentComponent() { + return ( +
+ {(() => { + function InlinedJSXComponent() { + return
; + }; + + return ; + })()} +
+ ); + } + `, + errors: [ + { + message: ERROR_MESSAGE + } + ] + }, + { + code: ` + function ComponentWithProps(props) { + return
; + } + + function ParentComponent() { + return ( + ; + } + } /> + ); + } + `, + errors: [ + { + message: ERROR_MESSAGE_COMPONENT_AS_PROPS + } + ] + }, + { + code: ` + function ComponentWithProps(props) { + return
; + } + + function ParentComponent() { + return ( +
} /> + ); + } + `, + errors: [ + { + message: ERROR_MESSAGE_COMPONENT_AS_PROPS + } + ] + }, + { + code: ` + function RenderPropComponent(props) { + return props.render({}); + } + + function ParentComponent() { + return ( + + {() => { + function UnstableNestedComponent() { + return
; + } + + return ( +
+ +
+ ); + }} + + ); + } + `, + errors: [ + { + message: ERROR_MESSAGE + } + ] + }, + { + code: ` + function ComponentForProps(props) { + return
; + } + + function ParentComponent() { + return ( +
} /> + ); + } + `, + errors: [ + { + message: ERROR_MESSAGE_COMPONENT_AS_PROPS + } + ] + } + ] +});