react icon indicating copy to clipboard operation
react copied to clipboard

[eslint-plugin-react-hooks] New rule to enforce lazy initialization for `useState`

Open legendkong opened this issue 1 year ago • 6 comments

Context

It is common to have instances where React.useState() code is unnecessarily re-creating its initial state:

Example:

const Component = () => {
 const [state, setState] = useState(getInitialHundredItems());
}

React's useState hooks accepts an initialState argument e.g. React.useState(initialState). The initialState argument is the state used during the initial render. In subsequent renders, it is disregarded. If the initial state is the result of an expensive computation, we may provide a function instead, which will be executed only on the initial render.

In the above example, getInitialHundredItems is called on each re-render, but its result is only needed on initial render.

More details useState and Lazy initial state can be found here: link1, link2.

Description

To address the problem mentioned above, we can make use of an initializer function in React.useState(). This function will only be executed once (on initial render) and not on each re-render like the above code will.

Example of how one would use lazy initialization:

const Component = () => {
 const [state, setState] = useState(() => {
    return getInitialHundredItems(x);
  })
}

Rule

...
  /**
   * This rule is meant to detect the following anti-pattern:
   *
   * ```js
   * const Component = () => {
   *  const [state, setState] = useState(getInitialHundredItems());
   * }
   * ```
   *
   * The initialState argument is the state used during the initial render. In subsequent renders, it is disregarded.
   * This initial value can be the result of calling a function as in the above example.
   * But note that getInitialHundredItems is unconditionally and needlessly called on each render cycle.
   *
   * To avoid the problem mentioned above, instead of just calling a function that returns a value,
   * you can pass a function which returns the initial state.
   * This function will only be executed once (initial render) and not on each render like the above code will.
   * See [Lazy Initial State] for details.
   *
   * [Lazy Initial State]: https://reactjs.org/docs/hooks-reference.html#lazy-initial-state
   *
   * The above code can be changed to the below, with lazy initilization:
   *
   * ```js
   * const Component = () => {
   *  const [state, setState] = useState(() => {
   *    return getInitialHundredItems(x);
   *  })
   * }
   * ```
   */
  'prefer-react-use-state-lazy-initialization': {
    meta: {
      type: 'problem',
      fixable: 'code',
      schema: [], // no options
    },
    create(context) {
      const ALLOW_LIST = ['Boolean', 'String'];

      return {
        'CallExpression[callee.name="useState"]'(node) {
          if (node.arguments.length > 0) {
            const arg = node.arguments[0];
            // If arg is a call expression and is not in `allowList`
            // e.g `Boolean(x)` is allowed, but `expensiveFunc(x)` is not allowed
            if (arg.type == 'CallExpression' && ALLOW_LIST.indexOf(arg.callee.name) === -1) {
              context.report({
                node: arg,
                message:
                  'To prevent expensive re-computation, consider using lazy initial state for React.useState().',
              });
            }
          }
        },
      };
    },
  }
...

legendkong avatar Mar 30 '23 17:03 legendkong

This is a valid point

aliasgar55 avatar Apr 01 '23 05:04 aliasgar55

+1 up

vctqs1 avatar Apr 02 '23 09:04 vctqs1

Thanks for putting that rule together! I was looking for a rule like this and stumbled upon this issue. I played around with it a bit and noticed it doesn't identify certain cases, examples:

useState(a ? b : c());
useState(a() > b);
useState(a() || b);

I made an updated version here: https://github.com/patorjk/eslint-plugin-react-helpers/blob/main/lib/rules/prefer-use-state-lazy-initialization.js

patorjk avatar May 15 '23 22:05 patorjk

hey @patorjk, thanks for the updated version and for the pr. Looks good! But it seems like the facebook team hasn't been merging in eslint related PRs for a while now, so fingers crossed 🤞

legendkong avatar May 18 '23 09:05 legendkong

This would be so useful to have it as an official linting rule. Thanks for creating it @patorjk!

terrymun avatar Feb 11 '24 21:02 terrymun

Hello! I took another approach, utilizes the property of ast traversal such as ":exit", and code come out much clear. I just banged all call experience inside useState.

    create: (context) => {
        let useStateCallExpression = null;

        return {
            CallExpression(node) {
                if (node.callee.type === 'Identifier' && node.callee.name === 'useState') {
                    useStateCallExpression = node;
                    return;
                }

                if (useStateCallExpression) {
                    context.report({ node, messageId: 'useLazyInitialization' });
                }
            },

            'CallExpression:exit': function (node) {
                if (node === useStateCallExpression) {
                    useStateCallExpression = null;
                }
            },
        };
    },

SeanSilke avatar Mar 01 '24 18:03 SeanSilke

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

github-actions[bot] avatar May 30 '24 19:05 github-actions[bot]

Thanks for posting this! With React Compiler, we are working on optimizations to ensure that useState initializers only run once, so this will become automatic and not something that developers have to think about. I'll go ahead and close since this is already work-in-progress but via a different approach.

Thanks again for posting!

josephsavona avatar May 30 '24 20:05 josephsavona