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-plugin-exhaustive-deps] hook wrongly marked as conditional (at exact number of conditionals in FC) #24279

Closed
SanderRonde opened this issue Apr 5, 2022 · 8 comments · Fixed by #24287

Comments

@SanderRonde
Copy link

When using an exact number of conditionals before and after a react hook, the React Hook "hook_name" is called conditionally. React Hooks must be called in the exact same order in every component render rule is wrongly flagged as being violated. This is a really weird bug and it's kind of hard to explain. Just take a look at the code and watch as ESLint flags the hook as somehow being conditional. While this may seem like a huge edge case, this actually triggered in our code base and caused all hooks in the component to be flagged as conditional.

React version: 18.0.0 (doesn't seem to matter)

Steps To Reproduce

  1. Check out this project, run yarn and run yarn eslint app/foo.tsx.
  2. Watch as the hook is incorrectly flagged as conditional.
  3. Removing or adding one of the conditionals in the return statement makes the bug go away. The same goes for removing one of the conditionals above the hook.

Link to code example: https://github.com/SanderRonde/eslint-hook-bug
Unfortunately I couldn't get it to work online (because of a lack of terminals)

The current behavior

Hook is incorrectly flagged as conditional

image

The expected behavior

Hook should not be conditional

@SanderRonde SanderRonde added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Apr 5, 2022
@gaearon gaearon added Type: Bug Difficulty: medium good first issue Component: ESLint Rules and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Apr 5, 2022
@scyron6
Copy link
Contributor

scyron6 commented Apr 5, 2022

I will work on this.

@gaearon
Copy link
Collaborator

gaearon commented Apr 5, 2022

Sure, thanks.

@gaearon
Copy link
Collaborator

gaearon commented Apr 27, 2022

Fixed in 18.1.0 4.5.0.

@KenyStev
Copy link

Apparently this one is still reproducing

@ChrisChiasson
Copy link

I am getting something that seems to be related to this; will try to narrow it down. FWIW, I downloaded the repo above and the example given by SanderRonde completes with no error now. In my case having a component with false && in front of it in the return statement is enough to trigger the bug. BTW, absolutely every single hook in the function after a certain boolean variable is assigned (but not acted upon with any kind of condition or return), lights up with this warning. It reminds me a lot of this type of JavaScript no-semicolons error https://stackoverflow.com/a/31013390/581848, where it actually results from a 'most vexing parse' (C++ terminology), but there are plenty of semicolons after the boolean so I don't see why it would think hooks on later lines would be conditional.

@ChrisChiasson
Copy link

I 'solved' it, but the solution is just based on intuition about the difficulties involved in writing static analyzers like esLint rather than any kind of 'fix' to the code. The change I made to fix it was from this (broken, see 3rd line):

  const runPrompts = !loading && usePromptsForCaseManagement;
  // blows up if next line uses runPrompts instead of the two conditions
  const showPrompts = runPrompts &&
    selectedTestId && testResultQuestionsPrompt && sectionIndex >= 1 &&
    sectionIndex !== localCaseNotes?.length;

to this (fixed, see 3rd line):

  const runPrompts = !loading && usePromptsForCaseManagement;
  // blows up if next line uses runPrompts instead of the two conditions
  const showPrompts = !loading && usePromptsForCaseManagement &&
    selectedTestId && testResultQuestionsPrompt && sectionIndex >= 1 &&
    sectionIndex !== localCaseNotes?.length;

@ChrisChiasson
Copy link

Similar to SanderRode, the esLint error won't show up unless the output returned from the function component is conditional (even if that condition is awlays false as SanderRode mentioned).

@dominiczy
Copy link

I'm having the same problem, it's triggered just by meaninglessly moving around condition in the JSX component (AFTER all the hooks are already executed). So is there a way to fix it at all?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants