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

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

Closed
camhux opened this issue Apr 21, 2021 · 7 comments
Labels
Component: ESLint Rules Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug Type: Bug

Comments

@camhux
Copy link

camhux commented Apr 21, 2021

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 camhux added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Apr 21, 2021
@cpojer
Copy link
Contributor

cpojer commented May 20, 2021

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

@andres-j
Copy link

andres-j commented Jun 3, 2021

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
Copy link

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:

  • eslint@8.5.0
  • eslint-plugin-react-hooks@4.3.0
  • @babel/eslint-parser@7.16.5
  • react@16.14.0

@GeoMarkou
Copy link

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
Copy link

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

@GeoMarkou
Copy link

@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
Copy link

mbelsky commented Dec 2, 2022

There is a pr which increases the limit of "available" operators: #24287. So as a possible solution you can try to upgrade to v4.5.0 or higher.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: ESLint Rules Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug Type: Bug
Projects
None yet
Development

No branches or pull requests

8 participants