[compiler] Add new ValidateNoVoidUseMemo pass

Adds a new validation pass to validate against `useMemo`s that don't return anything. This usually indicates some kind of "useEffect"-like code that has side effects that need to be memoized to prevent overfiring, and is an anti-pattern.

A follow up validation could also look at the return value of `useMemo`s to see if they are being used.

Author: poteto

compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts

@@ -82,6 +82,7 @@ import {
 import {inferTypes} from '../TypeInference';
 import {
   validateContextVariableLValues,
+  validateNoVoidUseMemo,
   validateHooksUsage,
   validateMemoizedEffectDependencies,
   validateNoCapitalizedCalls,
@@ -167,6 +168,9 @@ function runWithEnvironment(
 
   validateContextVariableLValues(hir);
   validateUseMemo(hir).unwrap();
+  if (env.config.enableValidateNoVoidUseMemo) {
+    validateNoVoidUseMemo(hir).unwrap();
+  }
 
   if (
     env.isInferredMemoEnabled &&

compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts

@@ -631,6 +631,17 @@ export const EnvironmentConfigSchema = z.object({
    * ```
    */
   lowerContextAccess: ExternalFunctionSchema.nullable().default(null),
+
+  /**
+   * If enabled, will validate useMemos that don't return any values:
+   *
+   * Valid:
+   *   useMemo(() => foo, [foo]);
+   *   useMemo(() => { return foo }, [foo]);
+   * Invalid:
+   *   useMemo(() => { ... }, [...]);
+   */
+  enableValidateNoVoidUseMemo: z.boolean().default(false),
 });
 
 export type EnvironmentConfig = z.infer<typeof EnvironmentConfigSchema>;

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoVoidUseMemo.ts

@@ -0,0 +1,134 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import {CompilerError, ErrorSeverity} from '../CompilerError';
+import {
+  HIRFunction,
+  IdentifierId,
+  FunctionExpression,
+  SourceLocation,
+} from '../HIR';
+import {Result} from '../Utils/Result';
+
+// TODO: see https://github.com/facebook/react-forget/blob/main/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts#L132-L156
+
+/**
+ * Validates that useMemo has at least one explicit return statement.
+ *
+ * Valid cases:
+ * - useMemo(() => value)                      // implicit arrow function return
+ * - useMemo(() => { return value; })          // explicit return
+ * - useMemo(() => { return; })                // explicit undefined
+ * - useMemo(() => { if (cond) return val; })  // at least one return
+ *
+ * Invalid cases:
+ * - useMemo(() => { console.log(); })         // no return statement at all
+ */
+export function validateNoVoidUseMemo(
+  fn: HIRFunction,
+): Result<void, CompilerError> {
+  const errors = new CompilerError();
+  const useMemoHooks = new Map<
+    IdentifierId,
+    {name: string; loc: SourceLocation}
+  >();
+  const funcExprs = new Map<IdentifierId, FunctionExpression>();
+
+  for (const [, block] of fn.body.blocks) {
+    for (const instr of block.instructions) {
+      if (!instr.lvalue) {
+        continue;
+      }
+
+      if (instr.value.kind === 'LoadGlobal') {
+        if (
+          instr.value.binding.kind === 'Global' &&
+          instr.value.binding.name === 'useMemo'
+        ) {
+          useMemoHooks.set(instr.lvalue.identifier.id, {
+            name: instr.value.binding.name,
+            loc: instr.loc,
+          });
+        }
+      } else if (instr.value.kind === 'PropertyLoad') {
+        if (instr.value.property === 'useMemo') {
+          useMemoHooks.set(instr.lvalue.identifier.id, {
+            name: instr.value.property,
+            loc: instr.loc,
+          });
+        }
+      } else if (instr.value.kind === 'FunctionExpression') {
+        funcExprs.set(instr.lvalue.identifier.id, instr.value);
+      }
+    }
+  }
+
+  for (const [, block] of fn.body.blocks) {
+    for (const instr of block.instructions) {
+      if (instr.value.kind === 'CallExpression') {
+        const callee = instr.value.callee.identifier;
+        const useMemoHook = useMemoHooks.get(callee.id);
+
+        if (useMemoHook !== undefined && instr.value.args.length > 0) {
+          const firstArg = instr.value.args[0];
+          if (firstArg.kind !== 'Identifier') {
+            continue;
+          }
+
+          let funcToCheck = funcExprs.get(firstArg.identifier.id);
+
+          if (!funcToCheck) {
+            for (const [, searchBlock] of fn.body.blocks) {
+              for (const searchInstr of searchBlock.instructions) {
+                if (
+                  searchInstr.lvalue &&
+                  searchInstr.lvalue.identifier.id === firstArg.identifier.id &&
+                  searchInstr.value.kind === 'FunctionExpression'
+                ) {
+                  funcToCheck = searchInstr.value;
+                  break;
+                }
+              }
+              if (funcToCheck) break;
+            }
+          }
+
+          if (funcToCheck) {
+            const hasReturn = checkFunctionHasReturn(
+              funcToCheck.loweredFunc.func,
+            );
+
+            if (!hasReturn) {
+              errors.push({
+                severity: ErrorSeverity.InvalidReact,
+                reason: `React Compiler has skipped optimizing this component because ${useMemoHook.name} doesn't return a value. ${useMemoHook.name} should only be used for memoizing values, not running arbitrary side effects.`,
+                loc: useMemoHook.loc,
+                suggestions: null,
+                description: null,
+              });
+            }
+          }
+        }
+      }
+    }
+  }
+  return errors.asResult();
+}
+
+function checkFunctionHasReturn(func: HIRFunction): boolean {
+  for (const [, block] of func.body.blocks) {
+    if (block.terminal.kind === 'return') {
+      if (
+        block.terminal.returnVariant === 'Explicit' ||
+        block.terminal.returnVariant === 'Implicit'
+      ) {
+        return true;
+      }
+    }
+  }
+  return false;
+}

compiler/packages/babel-plugin-react-compiler/src/Validation/index.ts

@@ -6,6 +6,7 @@
  */
 
 export {validateContextVariableLValues} from './ValidateContextVariableLValues';
+export {validateNoVoidUseMemo} from './ValidateNoVoidUseMemo';
 export {validateHooksUsage} from './ValidateHooksUsage';
 export {validateMemoizedEffectDependencies} from './ValidateMemoizedEffectDependencies';
 export {validateNoCapitalizedCalls} from './ValidateNoCapitalizedCalls';

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.expect.md

@@ -0,0 +1,33 @@
+
+## Input
+
+```javascript
+// @enableValidateNoVoidUseMemo
+function Component() {
+  const value = useMemo(() => {
+    console.log('computing');
+  }, []);
+  return <div>{value}</div>;
+}
+
+```
+
+
+## Error
+
+```
+Found 1 error:
+
+Error: React Compiler has skipped optimizing this component because useMemo doesn't return a value. useMemo should only be used for memoizing values, not running arbitrary side effects.
+
+error.useMemo-no-return-value.ts:3:16
+  1 | // @enableValidateNoVoidUseMemo
+  2 | function Component() {
+> 3 |   const value = useMemo(() => {
+    |                 ^^^^^^^ React Compiler has skipped optimizing this component because useMemo doesn't return a value. useMemo should only be used for memoizing values, not running arbitrary side effects.
+  4 |     console.log('computing');
+  5 |   }, []);
+  6 |   return <div>{value}</div>;
+```
+          
+      

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.useMemo-no-return-value.js

@@ -0,0 +1,7 @@
+// @enableValidateNoVoidUseMemo
+function Component() {
+  const value = useMemo(() => {
+    console.log('computing');
+  }, []);
+  return <div>{value}</div>;
+}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-arrow-implicit-return.expect.md

@@ -0,0 +1,39 @@
+
+## Input
+
+```javascript
+function Component() {
+  const value = useMemo(() => computeValue(), []);
+  return <div>{value}</div>;
+}
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+function Component() {
+  const $ = _c(2);
+  let t0;
+  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+    t0 = computeValue();
+    $[0] = t0;
+  } else {
+    t0 = $[0];
+  }
+  const value = t0;
+  let t1;
+  if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
+    t1 = <div>{value}</div>;
+    $[1] = t1;
+  } else {
+    t1 = $[1];
+  }
+  return t1;
+}
+
+```
+      
+### Eval output
+(kind: exception) Fixture not implemented

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-arrow-implicit-return.js

@@ -0,0 +1,4 @@
+function Component() {
+  const value = useMemo(() => computeValue(), []);
+  return <div>{value}</div>;
+}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-empty-return.expect.md

@@ -0,0 +1,33 @@
+
+## Input
+
+```javascript
+function Component() {
+  const value = useMemo(() => {
+    return;
+  }, []);
+  return <div>{value}</div>;
+}
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+function Component() {
+  const $ = _c(1);
+  let t0;
+  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+    t0 = <div>{undefined}</div>;
+    $[0] = t0;
+  } else {
+    t0 = $[0];
+  }
+  return t0;
+}
+
+```
+      
+### Eval output
+(kind: exception) Fixture not implemented

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-empty-return.js

@@ -0,0 +1,6 @@
+function Component() {
+  const value = useMemo(() => {
+    return;
+  }, []);
+  return <div>{value}</div>;
+}