jsx-eslint / eslint-plugin-react

React-specific linting rules for ESLint
MIT License
8.86k stars 2.76k forks source link

Rule proposal: Prefer memoized callback functions #3682

Open oliveryasuna opened 5 months ago

oliveryasuna commented 5 months ago

Consider the following:

type MyComponentProps = {
  onClick?: (event: React.MouseEvent<HTMLDivElement>) => void
};

const MyComponent = (props: MyComponentProps) => {
  return <div onClick={props.onClick}/>;
};

If props.onClick changes frequently and/or changes on every parent render, it will cause the child component to re-render. Therefore, one may want to memoized the callback:

const MyComponent = ((props: MyComponentProps) => {
  const handleClick = useCallback(
      ((event: React.MouseEvent<HTMLDivElement>) => {
        props.onClick?.(event);
      }),
      [props.onClick]
  );

  return (<div onClick={handleClick}/>);
});

If props.onClick changes on every parent render, e.g., it is defined inline (onClick={() => doSomething()}) without memoization, handleClick will change on every render as well, mirroring the pitfall of the first example. However, by memoizing handleClick, one has more control over re-renders if props.onClick is stable, i.e., it is not recreated on every parent render.

I propose a new rule that reports when a prop is used directly as a callback function. More generally, such a rule could allow developers to prefer one pattern over the other.

Note: I opened this issue is conjunction with https://github.com/jsx-eslint/eslint-plugin-react/issues/3683.

ljharb commented 5 months ago

@oliveryasuna While in general this is correct, for HTML elements that's not actually true - React is smarter than that. It's only functions passed as props to custom elements that need to be memoized.