Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Docs] no-unstable-nested-components: Warn about memoized, nested components #3444

Merged
merged 1 commit into from Oct 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -14,10 +14,12 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
### Changed
* [Docs] [`no-unknown-property`]: fix typo in link ([#3445][] @denkristoffer)
* [Perf] component detection: improve performance by optimizing getId ([#3451][] @golopot)
* [Docs] [`no-unstable-nested-components`]: Warn about memoized, nested components ([#3444][] @eps1lon)

[#3451]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3451
[#3448]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3448
[#3445]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3445
[#3444]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3444
[#3436]: https://github.com/jsx-eslint/eslint-plugin-react/issues/3436
[#3337]: https://github.com/jsx-eslint/eslint-plugin-react/issues/3337
[#1591]: https://github.com/jsx-eslint/eslint-plugin-react/pull/1591
Expand Down
26 changes: 16 additions & 10 deletions docs/rules/no-unstable-nested-components.md
Expand Up @@ -2,9 +2,9 @@

💼 This rule is enabled in the following [configs](https://github.com/jsx-eslint/eslint-plugin-react#shareable-configurations): `all`.

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.
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://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).
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).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[possible bugs are introduced](https://codepen.io/ariperkkio/pen/vYLodLB)

I'll update this demo not to include useCallback example at some point.


## Rule Details

Expand Down Expand Up @@ -76,6 +76,20 @@ function Component() {

```jsx
function Component() {
return <SomeComponent footer={<div />} />;
}
```

⚠️ WARNING ⚠️:

Creating nested but memoized components is currently not detected by this rule but should also be avoided.
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(() => <div />, []);

return (
Expand All @@ -86,14 +100,6 @@ function Component() {
}
```

```jsx
function Component() {
return (
<SomeComponent footer={<div />} />
)
}
```

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
Expand Down
4 changes: 4 additions & 0 deletions tests/lib/rules/no-unstable-nested-components.js
Expand Up @@ -78,6 +78,7 @@ ruleTester.run('no-unstable-nested-components', rule, {
`,
},
{
// false-negative.
code: `
function ParentComponent() {
const MemoizedNestedComponent = React.useCallback(() => <div />, []);
Expand All @@ -91,6 +92,7 @@ ruleTester.run('no-unstable-nested-components', rule, {
`,
},
{
// false-negative.
code: `
function ParentComponent() {
const MemoizedNestedComponent = React.useCallback(
Expand All @@ -107,6 +109,7 @@ ruleTester.run('no-unstable-nested-components', rule, {
`,
},
{
// false-negative.
code: `
function ParentComponent() {
const MemoizedNestedFunctionComponent = React.useCallback(
Expand All @@ -125,6 +128,7 @@ ruleTester.run('no-unstable-nested-components', rule, {
`,
},
{
// false-negative.
code: `
function ParentComponent() {
const MemoizedNestedFunctionComponent = React.useCallback(
Expand Down