facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
227.92k stars 46.51k forks source link

Bug: ESLint react-hooks/rules-of-hooks false positives when codepath counts exceed Number.MAX_SAFE_INTEGER #21328

Open camhux opened 3 years ago

camhux commented 3 years ago

React version: 16.14.0 eslint-plugin-react-hooks version: 4.2.0 eslint version: 7.24.0 @typescript-eslint/parser version: 4.17.0 @babel/eslint-parser version: 7.13.4

A false positive from rules-of-hooks, specifically the "called conditionally" report, cropped up in a codebase I work on this week. It was a very strange scenario where modifying portions of arbitrary logical expressions/operators would change which hooks were reported, or make the lint start passing altogether (when nothing about the structure of hooks had changed).

I drilled into it and diagnosed it as an overflow in the lint rule's path counting logic. Please find an isolated reproduction and brief writeup in this repository: https://github.com/camhux/eslint-react-hook-false-positive

Steps To Reproduce

  1. Clone https://github.com/camhux/eslint-react-hook-false-positive.
  2. Run yarn repro.

Link to code example: https://github.com/camhux/eslint-react-hook-false-positive/blob/main/repro.tsx

The current behavior

The useEffect hook on repro.tsx:7 is flagged by rules-of-hooks as being called conditionally.

The expected behavior

No errors are raised by the rules-of-hooks rule for repro.tsx.

cpojer commented 3 years ago

I'm running into the exact same issue, also with optional chaining in a large component.

andres-j commented 3 years ago

Having this same issue, a large functional component passing query data to child components. Many ternary operators/null checks to handle cases of missing data. False positives appear when making changes exactly as @camhux describes.

destradaxcheckapp commented 2 years ago

I'm experiencing the same issue. Managed to reproduce it with this test component:

import React, { useEffect, useState } from 'react';

const MyComponent = () => {
  const [a1] = useState({});
  const [a2] = useState(!a1?.a && !a1?.a && !a1?.a);
  const [a3] = useState({});

  useEffect(() => {
    console.log(a3, a2);
  }, [a3, a2]);

  return (
    <div>
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
      {false && <input value={a1?.a} />}
    </div>
  );
};

export default MyComponent;

ESLint reports:

Line 5:16:  React Hook "useState" is called conditionally. React Hooks must be called in the exact same order in every component render   react-hooks/rules-of-hooks
  Line 7:3:   React Hook "useEffect" is called conditionally. React Hooks must be called in the exact same order in every component render  react-hooks/rules-of-hooks

This is my .eslintrc.json:

{
  "extends": ["react-app", "react-app/jest"],
  "rules": {
    "no-debugger": "warn",
    "no-console": ["warn", { "allow": ["warn", "error", "info"] }],
    "no-undef": "off",
    "no-unused-expressions": "off",
    "import/no-anonymous-default-export": "off",
    "react-hooks/exhaustive-deps": [
      "warn",
      {
        "additionalHooks": "(useMemoRef|useCallbackRef|useUpdateEffect)"
      }
    ]
  }
}

Versions I'm using:

GeoMarkou commented 2 years ago

It's nice to finally know what the cause of this was. I knew it had something to do with optional chaining and conditional operators but couldn't figure it out. There's not really an easy workaround either since the only solution I can see is is "use less conditions".

shindeajinkya commented 1 year ago

has anyone found any solution to this? facing the same issue @GeoMarkou @destradaxcheckapp

GeoMarkou commented 1 year ago

@shindeajinkya

I noticed that for some reason ternary operations seem to count as way more “points” against the maximum number of code paths than expected. So you can solve this by converting them into if/else statements instead.

Before:

const displayedOption = (IsLoading || options.length === 1) ? options[0] : options.find(o => o.id === selectedOption);

After:

let displayedOption = options[0];
if (!IsLoading && options.length > 1) {
    displayedOption = options.find(o => o.id === selectedOption);
}
mbelsky commented 1 year ago

There is a pr which increases the limit of "available" operators: https://github.com/facebook/react/pull/24287. So as a possible solution you can try to upgrade to v4.5.0 or higher.