eslint-plugin-react-hooks: lint rule to enforce that `useMemo` returns a value

[JoshuaKGoldberg]

Nothing prevents developers from calling useMemo without a return:

import { useMemo } from "react";

useMemo(() => {
    console.log("Yippee!");
}, []);

[TypeScript playground showing no errors]

Calling useMemo without a returned value is somewhat equivalent to calling useEffect without a returned value. Judging by the few times I've seen it happen, it's likely a case of a developer either confusing the two hooks or forgetting to return the memoized value.

Suggestion: can we add a lint rule to flag useMemo calls that don't definitely return a value? It could have a suggestion fixer to switch to useEffect.

[eps1lon]

I think we can fix this at a type level. We already flag map() with no return being assigned to ReactNode.

[JoshuaKGoldberg]

Please forgive my lack of familiarity with React-specific terminology here 😅...

fix this at a type level

What does "type level" mean? As in, will you use some Flow/TypeScript type system trickery, or ...?

We already flag map() with no return being assigned to ReactNode

What does this mean?

[eps1lon]

What does "type level" mean? As in, will you use some Flow/TypeScript type system trickery, or ...?

Sorry, yes Flow/TypeScript. Though I'm not familiar with Flow I can see how we might fix this with TypeScript since it has a specific type for no return statement: void.

In React 17, missing a return would not be caught with TypeScript. With React 18 we can catch missing returns now now:

const values = ['a', 'b']

//  Type 'void' is not assignable to type 'ReactNode'.(2322)
const element = <div>{
  values.map(value => {
    // no return :(
  })

}</div>;

Playground Link

We can do this with useMemo but we have to whitelist the types since we don't have negated types (T extends void ? never : T has no good error message).

A lint rule has the benefit of allowing a proper error message.

[xjamundx]

Literally ran into this today and hoped a rule like this existed. I can try to help write it if it's approved.

[nossbigg]

@eps1lon @JoshuaKGoldberg I've got a PR for this, please have a look, thanks! #34768

tldr: IMO we don't need typechecking? - we can just traverse the AST to check if there's a return 👍

[josephsavona]

@poteto Has implemented this validation in React Compiler, but it's currently disabled. I think the way to go here is to finish polishing that rule and enable. Thanks for the reminder!

[JoshuaKGoldberg]

don't need typechecking

Famous last words 😛 . Functions that return void often can't be detected only with the AST. You'd need types to understand cases like these two:

useMemo(() => console.log("gotcha!");

function thisTotallyReturnsSomething() {
  console.log("gotcha");
}

useMemo(() => thisTotallyReturnsSomething());

Related: https://typescript-eslint.io/rules/no-confusing-void-expression

[nossbigg]

@JoshuaKGoldberg ah yes I had missed out on the cases of implicit void returns, thanks for pointing it out.

But alas, given that there's an expectation of this rule to "work" in non-typed codebases (ie. no flow, no typescript), I guess there may not be enough information from the AST alone to solve for this case (for non-typed codebases).

Perhaps the (upcoming) React Compiler has a solution for this corner case? 🤔 (cc @josephsavona @poteto)

[josephsavona]

Yeah we handle implicit void returns :-)