react icon indicating copy to clipboard operation
react copied to clipboard

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

Open camhux opened this issue 3 years ago • 5 comments

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.

camhux avatar Apr 21 '21 23:04 camhux

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

cpojer avatar May 20 '21 00:05 cpojer

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.

andres-j avatar Jun 03 '21 15:06 andres-j

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:

destradaxcheckapp avatar Feb 15 '22 20:02 destradaxcheckapp

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".

GeoMarkou avatar Mar 02 '22 23:03 GeoMarkou

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

shindeajinkya avatar Oct 27 '22 07:10 shindeajinkya

@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);
}

GeoMarkou avatar Oct 29 '22 05:10 GeoMarkou

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.

mbelsky avatar Dec 02 '22 20:12 mbelsky