chore(linting): Add react-hooks eslint rules

[robbie-c]

Problem

We don't have this lint rule enabled, which makes it very easy to introduce subtle bugs

Changes

Add the https://www.npmjs.com/package/eslint-plugin-react-hooks eslint plugin. It's made by the core react team to catch react hooks bugs.

It would be impossible to make code changes across the codebase, verify the correctness of each one, and get a reviewer for each one. Instead, I added a comment disabling the rule, and a FIXME comment so that someone who owns these files can take a closer look. This feels not-urgent, as presumably the code is already working in production. This approach means that we will get the warnings for new code without needing to spent excessive effort on old code.

How did you test this code?

Linting and tests pass

[mariusandra]

I've had the chat many times now on why we shouldn't enable this.

Last time: #10665 (comment)
Time before that: #6576 (review)
Many times in Slack

• It will force us to add all kea actions to the deps array
• In my experience this really hasn't helped catch bugs, and has been a source of frustrations
• We should be moving away from useEffects in general, or keeping them really really small.

[posthog-bot]

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

[robbie-c]

@mariusandra if this conversation has come up a few times - lets add a comment to the eslint file explaining why this is disabled. Just on your points:

We should be moving away from useEffects in general, or keeping them really really small.

It's useCallback and useMemo as well, which there are good reasons for using (referential stability of callbacks and avoiding unecessary re-rerenders being a big one).

It will force us to add all kea actions to the deps array

Yes this is true, I can see why this is annoying, but your IDE can help here. I just hit alt-enter in webstorm and it eslint-fixes the warning and adds everything to the dependency array that needs to be there. That way I don't need to think at all.

I don't feel strongly enough about the dependecy array to fight about it though :P

Probably the right answer is to keep the rules-of-hooks linting rule, as that only catches things that are definitely wrong rather than possibly wrong, disable the dependency array rule, and add a comment in the eslint file referencing this PR and the other two.

[posthog-bot]

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

[posthog-bot]

This PR was closed due to lack of activity. Feel free to reopen if it's still relevant.