React: Erroneous useEffect conditional call error

[synzen]

Do you want to request a feature or report a bug?
bug

What is the current behavior?
React gives an error about useEffect being called conditionally when it actually isn't. Modifying a local object variable within a loop seems to be triggering it.

React Hook "useEffect" is called conditionally. React Hooks must be called in the exact same order in every component render. Did you accidentally call a React Hook after an early return? react-hooks/rules-of-hooks

Demo

https://codesandbox.io/s/epic-meadow-eji2l

What is the expected behavior?
It should not give any errors about conditional calls of useEffect since it's not being called conditionally

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
v16.8.6 / Chrome / Windows, unknown

[jahed]

From my understanding, the linter error is correct, but the message isn't helpful.

The loop before useEffect can potentially be iterated any number of times, and within it, another hook might be used (which you shouldn't to anyway) which will cause React to lose track of which hook maps to which call in the next render call. Or the loop might return early.

In your specific scenario, it's over a fixed number (zero) so it's a false positive, but the general rule still applies: don't use hooks after conditionals and iterations.

This isn't in the Rules of Hooks page though, it says to only call hooks in the "top level" which I assume means before any branching paths, but it might mean something else.

If you need to, then pull out part after the iteration (the effect and returned element) into its own component so that React can track them separately. Alternatively, if it makes sense, move the iteration into the effect.

[synzen]

If this is intended behavior, then that's a bit silly. Giving an _error_ (not a warning) about a hook being called conditionally only because it's after a loop that has the potential to contain a hook rather than actually containing one, despite there being no change in the order of hook calls (was very frustrating to debug upon first encountering this).

In any case I don't believe that is the case. I should've put more emphasis on it but it's not the actual _loop_ causing the error to show up, but the modifying a local JS object is triggering it. If you replace the object in the sandbox with an array, and instead push items into the array within the loop, the error disappears. Here's another sandbox:

https://codesandbox.io/s/festive-ride-p2mis

You can also replace the problematic object variable with a string, and concatenate to the string in the loop - the error also disappears in that case. So something about modifying an object is causing it.

[jahed]

You're right. Looks like rules-of-hooks is able to determine improper use of hooks within loops and conditionals, so doesn't make sense for it to be flagged afterwards.

It's not the object modification triggering it. Looks like it's specifically the else inside a loop.

https://codesandbox.io/s/late-sky-lopeh

Moving the loop into its own function prevents the error. So it may be an issue with the code path calculations here:

https://github.com/facebook/react/blob/master/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js#L405

There's a "Special case when we think there might be an early return.", and there's a few "possiblies" and "might", so the detection likely needs improving. Don't know how myself as I haven't dealt with ESLint rules and ASTs before.

[FabianoLothor]

Any help for here? Same issue.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

[stale[bot]]

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!

[gaearon]

I believe the latest ESLint plugin version fixes it, not deployed on CodeSandbox yet though.