[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.
2025-11-01 09:12:30 +00:00 · 2025-07-25 12:56:28 -04:00
parent 1623111407
commit c8abac65cd
14 changed files with 392 additions and 0 deletions
@@ -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 &&
@@ -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>;
@@ -0,0 +1,156 @@
+/**
+ * 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,
+  Environment,
+  Instruction,
+  getHookKindForType,
+} from '../HIR';
+import {Result} from '../Utils/Result';
+
+type TemporariesSidemap = {
+  useMemoHooks: Map<IdentifierId, {name: string; loc: SourceLocation}>;
+  funcExprs: Map<IdentifierId, FunctionExpression>;
+  react: Set<IdentifierId>;
+};
+
+/**
+ * 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 sidemap: TemporariesSidemap = {
+    useMemoHooks: new Map(),
+    funcExprs: new Map(),
+    react: new Set(),
+  };
+
+  for (const [, block] of fn.body.blocks) {
+    for (const instr of block.instructions) {
+      collectTemporaries(instr, fn.env, sidemap);
+    }
+  }
+
+  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 = sidemap.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 = sidemap.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 = checkFunctionHasNonVoidReturn(
+              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 checkFunctionHasNonVoidReturn(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;
+}
+
+function collectTemporaries(
+  instr: Instruction,
+  env: Environment,
+  sidemap: TemporariesSidemap,
+): void {
+  const {value, lvalue} = instr;
+  switch (value.kind) {
+    case 'FunctionExpression': {
+      sidemap.funcExprs.set(lvalue.identifier.id, value);
+      break;
+    }
+    case 'LoadGlobal': {
+      const global = env.getGlobalDeclaration(value.binding, value.loc);
+      const hookKind = global !== null ? getHookKindForType(env, global) : null;
+      if (hookKind === 'useMemo') {
+        sidemap.useMemoHooks.set(lvalue.identifier.id, {
+          name: value.binding.name,
+          loc: instr.loc,
+        });
+      } else if (value.binding.name === 'React') {
+        sidemap.react.add(lvalue.identifier.id);
+      }
+      break;
+    }
+    case 'PropertyLoad': {
+      if (sidemap.react.has(value.object.identifier.id)) {
+        if (value.property === 'useMemo') {
+          sidemap.useMemoHooks.set(lvalue.identifier.id, {
+            name: value.property,
+            loc: instr.loc,
+          });
+        }
+      }
+      break;
+    }
+  }
+}
@@ -6,6 +6,7 @@
 */

 export {validateContextVariableLValues} from './ValidateContextVariableLValues';
+export {validateNoVoidUseMemo} from './ValidateNoVoidUseMemo';
 export {validateHooksUsage} from './ValidateHooksUsage';
 export {validateMemoizedEffectDependencies} from './ValidateMemoizedEffectDependencies';
 export {validateNoCapitalizedCalls} from './ValidateNoCapitalizedCalls';
@@ -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>;
+```
+          
+      
@@ -0,0 +1,7 @@
+// @enableValidateNoVoidUseMemo
+function Component() {
+  const value = useMemo(() => {
+    console.log('computing');
+  }, []);
+  return <div>{value}</div>;
+}
@@ -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
@@ -0,0 +1,4 @@
+function Component() {
+  const value = useMemo(() => computeValue(), []);
+  return <div>{value}</div>;
+}
@@ -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
@@ -0,0 +1,6 @@
+function Component() {
+  const value = useMemo(() => {
+    return;
+  }, []);
+  return <div>{value}</div>;
+}
@@ -0,0 +1,33 @@
+
+## Input
+
+```javascript
+function Component() {
+  const value = useMemo(() => {
+    return null;
+  }, []);
+  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>{null}</div>;
+    $[0] = t0;
+  } else {
+    t0 = $[0];
+  }
+  return t0;
+}
+
+```
+      
+### Eval output
+(kind: exception) Fixture not implemented
@@ -0,0 +1,6 @@
+function Component() {
+  const value = useMemo(() => {
+    return null;
+  }, []);
+  return <div>{value}</div>;
+}
@@ -0,0 +1,50 @@
+
+## Input
+
+```javascript
+function Component({items}) {
+  const value = useMemo(() => {
+    for (let item of items) {
+      if (item.match) return item;
+    }
+    return null;
+  }, [items]);
+  return <div>{value}</div>;
+}
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+function Component(t0) {
+  const $ = _c(2);
+  const { items } = t0;
+  let t1;
+  bb0: {
+    for (const item of items) {
+      if (item.match) {
+        t1 = item;
+        break bb0;
+      }
+    }
+
+    t1 = null;
+  }
+  const value = t1;
+  let t2;
+  if ($[0] !== value) {
+    t2 = <div>{value}</div>;
+    $[0] = value;
+    $[1] = t2;
+  } else {
+    t2 = $[1];
+  }
+  return t2;
+}
+
+```
+      
+### Eval output
+(kind: exception) Fixture not implemented
@@ -0,0 +1,9 @@
+function Component({items}) {
+  const value = useMemo(() => {
+    for (let item of items) {
+      if (item.match) return item;
+    }
+    return null;
+  }, [items]);
+  return <div>{value}</div>;
+}