[Feature Request][eslint-plugin-react-hooks] no-ref-checks, display error when using useRef's return value as condition

[johnrom]

https://github.com/reactjs/rfcs/issues/198 https://github.com/yannickcr/eslint-plugin-react/issues/3042

Not sure whether this belongs in eslint-plugin-react-hooks or eslint-plugin-react, but a contributor at the latter suggested this to be a more appropriate place. I've opened an issue instead of an RFC to discuss whether this is the appropriate place. https://github.com/yannickcr/eslint-plugin-react/issues/3042#issuecomment-899689312

I started migrating a codebase from class-based to function-based and came across some silliness. I had completely missed converting some of the ref checks:

- const thing = this.thing;
+ const thing = useRef(props.thing);

// I missed this
- if (!thing) {
+ if (!thing.current) {

TypeScript considers !useRef(props.thing) to be perfectly valid since it may be testing for non-existence of the value even if it is not boolean. However, due to the rules of hooks, this value will never be undefined -- any checks for it are unnecessary and could either be an innocuous useless check or indicate a serious bug / typo. Thus, I'm thinking it might make sense to make sure a useRef's return value is never used as a boolean or condition at the react linter level. Not sure if it's possible with ESLint, but if so it could be a very useful rule which would catch a lot of bugs.

[eps1lon]

// I missed this
- if (!thing) {
+ if (!thing.current) {

Since you're using TypeScript, have you tried the @typescript-eslint/no-unnecessary-condition rule from @typescript-eslint/plugin? This rule should catch the problems you're describing.

[johnrom]

I think I use the base recommended settings, so that sounds like a possible solution. The fact that the rule is not enabled by default (I assume, since it didn't apply) is a bit concerning, and it doesn't seem like it would be the safest to implement retroactively on every project. The rule itself would require a dev to remove safeguards from their project (or add a bunch of ignores) that may cover incorrect types and may introduce bugs if a dev is not careful to analyze all of their types, and could break integrations with third party code whose types are incorrect.

The rule I propose above doesn't have these drawbacks due to the rules of hooks which state that the useRef must be called on every render, and could be enabled by default for useRef without concern.

[stale[bot]]

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

[johnrom]

Not stale, I don't think the rule described above as a workaround is as powerful as the new rule proposed, especially since the rule is not commonly enabled by default.

[veloek]

I would really like this rule as well. Have you given it a shot trying to implement this yourself, @johnrom?

[johnrom]

I have not, unfortunately, and am not likely to as I am not familiar with writing eslint plugins. I still like the idea, though.

[man-trackunit]

Would love this! Stole a few hours of my life yesterday.

[tjx666]

The ESLint rule vue/no-ref-as-operand can be a real time-saver. Frequently, significant time is wasted on this common mistake.