[compiler][wip] Fix for ValidatePreserveMemo edge case w multiple return in useMemo

Potential fix for #34750. With a useMemo with multiple returns we can sometimes end up thinking the FinishMemoize value wasn't memoized, even though it definitely is. But the same case works if you rewrite to have a let binding, assign to the let instead of returning, and then have a single return with the let — which is the exact shape we convert to when we inline the useMemo! The only difference is that when we inline, we end up with an extra LoadLocal. Adding an extra LoadLocal "fixes" the issue but this is definitely a hack.

However, along the way this helped me find that we also have a bug in PruneAlwaysInvalidatingScopes, where we were forgetting to set FinishMemoize instructions to pruned if their declaration always invalidates.

Putting up for feedback, not necessarily expecting to land as-is.

compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts

@@ -988,7 +988,7 @@ function getRuleForCategoryImpl(category: ErrorCategory): LintRule {
         severity: ErrorSeverity.Error,
         name: 'void-use-memo',
         description:
-          'Validates that useMemos always return a value. See [`useMemo()` docs](https://react.dev/reference/react/useMemo) for more information.',
+          'Validates that useMemos always return a value and that the result of the useMemo is used by the component/hook. See [`useMemo()` docs](https://react.dev/reference/react/useMemo) for more information.',
         preset: LintRulePreset.RecommendedLatest,
       };
     }

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

@@ -659,7 +659,7 @@ export const EnvironmentConfigSchema = z.object({
    * Invalid:
    *   useMemo(() => { ... }, [...]);
    */
-  validateNoVoidUseMemo: z.boolean().default(false),
+  validateNoVoidUseMemo: z.boolean().default(true),
 
   /**
    * Validates that Components/Hooks are always defined at module level. This prevents scope

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

@@ -988,7 +988,7 @@ export function createTemporaryPlace(
     identifier: makeTemporaryIdentifier(env.nextIdentifierId, loc),
     reactive: false,
     effect: Effect.Unknown,
-    loc: GeneratedSource,
+    loc,
   };
 }
 

compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts

@@ -11,7 +11,6 @@ import {
   CallExpression,
   Effect,
   Environment,
-  FinishMemoize,
   FunctionExpression,
   HIRFunction,
   IdentifierId,
@@ -25,7 +24,6 @@ import {
   Place,
   PropertyLoad,
   SpreadPattern,
-  StartMemoize,
   TInstruction,
   getHookKindForType,
   makeInstructionId,
@@ -184,36 +182,52 @@ function makeManualMemoizationMarkers(
   depsList: Array<ManualMemoDependency> | null,
   memoDecl: Place,
   manualMemoId: number,
-): [TInstruction<StartMemoize>, TInstruction<FinishMemoize>] {
+): [Array<Instruction>, Array<Instruction>] {
+  const temp = createTemporaryPlace(env, memoDecl.loc);
   return [
-    {
+    [
-      id: makeInstructionId(0),
+      {
-      lvalue: createTemporaryPlace(env, fnExpr.loc),
+        id: makeInstructionId(0),
-      value: {
+        lvalue: createTemporaryPlace(env, fnExpr.loc),
-        kind: 'StartMemoize',
+        value: {
-        manualMemoId,
+          kind: 'StartMemoize',
-        /*
+          manualMemoId,
-         * Use deps list from source instead of inferred deps
+          /*
-         * as dependencies
+           * Use deps list from source instead of inferred deps
-         */
+           * as dependencies
-        deps: depsList,
+           */
+          deps: depsList,
+          loc: fnExpr.loc,
+        },
+        effects: null,
         loc: fnExpr.loc,
       },
-      effects: null,
+    ],
-      loc: fnExpr.loc,
+    [
-    },
+      {
-    {
+        id: makeInstructionId(0),
-      id: makeInstructionId(0),
+        lvalue: {...temp},
-      lvalue: createTemporaryPlace(env, fnExpr.loc),
+        value: {
-      value: {
+          kind: 'LoadLocal',
-        kind: 'FinishMemoize',
+          place: {...memoDecl},
-        manualMemoId,
+          loc: memoDecl.loc,
-        decl: {...memoDecl},
+        },
-        loc: fnExpr.loc,
+        effects: null,
+        loc: memoDecl.loc,
       },
-      effects: null,
+      {
-      loc: fnExpr.loc,
+        id: makeInstructionId(0),
-    },
+        lvalue: createTemporaryPlace(env, memoDecl.loc),
+        value: {
+          kind: 'FinishMemoize',
+          manualMemoId,
+          decl: {...temp},
+          loc: memoDecl.loc,
+        },
+        effects: null,
+        loc: memoDecl.loc,
+      },
+    ],
   ];
 }
 
@@ -409,10 +423,7 @@ export function dropManualMemoization(
    *   LoadLocal fnArg
    * - (if validation is enabled) collect manual memoization markers
    */
-  const queuedInserts: Map<
+  const queuedInserts: Map<InstructionId, Array<Instruction>> = new Map();
-    InstructionId,
-    TInstruction<StartMemoize> | TInstruction<FinishMemoize>
-  > = new Map();
   for (const [_, block] of func.body.blocks) {
     for (let i = 0; i < block.instructions.length; i++) {
       const instr = block.instructions[i]!;
@@ -438,40 +449,6 @@ export function dropManualMemoization(
             continue;
           }
 
-          /**
-           * Bailout on void return useMemos. This is an anti-pattern where code might be using
-           * useMemo like useEffect: running arbirtary side-effects synced to changes in specific
-           * values.
-           */
-          if (
-            func.env.config.validateNoVoidUseMemo &&
-            manualMemo.kind === 'useMemo'
-          ) {
-            const funcToCheck = sidemap.functions.get(
-              fnPlace.identifier.id,
-            )?.value;
-            if (funcToCheck !== undefined && funcToCheck.loweredFunc.func) {
-              if (!hasNonVoidReturn(funcToCheck.loweredFunc.func)) {
-                errors.pushDiagnostic(
-                  CompilerDiagnostic.create({
-                    category: ErrorCategory.VoidUseMemo,
-                    reason: 'useMemo() callbacks must return a value',
-                    description: `This ${
-                      manualMemo.loadInstr.value.kind === 'PropertyLoad'
-                        ? 'React.useMemo()'
-                        : 'useMemo()'
-                    } callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects`,
-                    suggestions: null,
-                  }).withDetails({
-                    kind: 'error',
-                    loc: instr.value.loc,
-                    message: 'useMemo() callbacks must return a value',
-                  }),
-                );
-              }
-            }
-          }
-
           instr.value = getManualMemoizationReplacement(
             fnPlace,
             instr.value.loc,
@@ -557,11 +534,11 @@ export function dropManualMemoization(
       let nextInstructions: Array<Instruction> | null = null;
       for (let i = 0; i < block.instructions.length; i++) {
         const instr = block.instructions[i];
-        const insertInstr = queuedInserts.get(instr.id);
+        const insertInstructions = queuedInserts.get(instr.id);
-        if (insertInstr != null) {
+        if (insertInstructions != null) {
           nextInstructions = nextInstructions ?? block.instructions.slice(0, i);
           nextInstructions.push(instr);
-          nextInstructions.push(insertInstr);
+          nextInstructions.push(...insertInstructions);
         } else if (nextInstructions != null) {
           nextInstructions.push(instr);
         }
@@ -629,17 +606,3 @@ function findOptionalPlaces(fn: HIRFunction): Set<IdentifierId> {
   }
   return optionals;
 }
-
-function hasNonVoidReturn(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/ReactiveScopes/PruneAlwaysInvalidatingScopes.ts

@@ -77,6 +77,15 @@ class Transform extends ReactiveFunctionTransform<boolean> {
         }
         break;
       }
+      case 'FinishMemoize': {
+        if (
+          !withinScope &&
+          this.alwaysInvalidatingValues.has(value.decl.identifier)
+        ) {
+          value.pruned = true;
+        }
+        break;
+      }
     }
     return {kind: 'keep'};
   }

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

@@ -24,6 +24,7 @@ import {Result} from '../Utils/Result';
 
 export function validateUseMemo(fn: HIRFunction): Result<void, CompilerError> {
   const errors = new CompilerError();
+  const voidMemoErrors = new CompilerError();
   const useMemos = new Set<IdentifierId>();
   const react = new Set<IdentifierId>();
   const functions = new Map<IdentifierId, FunctionExpression>();
@@ -125,7 +126,22 @@ export function validateUseMemo(fn: HIRFunction): Result<void, CompilerError> {
           validateNoContextVariableAssignment(body.loweredFunc.func, errors);
 
           if (fn.env.config.validateNoVoidUseMemo) {
-            unusedUseMemos.set(lvalue.identifier.id, callee.loc);
+            if (!hasNonVoidReturn(body.loweredFunc.func)) {
+              voidMemoErrors.pushDiagnostic(
+                CompilerDiagnostic.create({
+                  category: ErrorCategory.VoidUseMemo,
+                  reason: 'useMemo() callbacks must return a value',
+                  description: `This useMemo() callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects`,
+                  suggestions: null,
+                }).withDetails({
+                  kind: 'error',
+                  loc: body.loc,
+                  message: 'useMemo() callbacks must return a value',
+                }),
+              );
+            } else {
+              unusedUseMemos.set(lvalue.identifier.id, callee.loc);
+            }
           }
           break;
         }
@@ -146,10 +162,10 @@ export function validateUseMemo(fn: HIRFunction): Result<void, CompilerError> {
      * Even a DCE-based version could be bypassed with `noop(useMemo(...))`.
      */
     for (const loc of unusedUseMemos.values()) {
-      errors.pushDiagnostic(
+      voidMemoErrors.pushDiagnostic(
         CompilerDiagnostic.create({
           category: ErrorCategory.VoidUseMemo,
-          reason: 'Unused useMemo()',
+          reason: 'useMemo() result is unused',
           description: `This useMemo() value is unused. useMemo() is for computing and caching values, not for arbitrary side effects`,
           suggestions: null,
         }).withDetails({
@@ -160,6 +176,7 @@ export function validateUseMemo(fn: HIRFunction): Result<void, CompilerError> {
       );
     }
   }
+  fn.env.logErrors(voidMemoErrors.asResult());
   return errors.asResult();
 }
 
@@ -192,3 +209,17 @@ function validateNoContextVariableAssignment(
     }
   }
 }
+
+function hasNonVoidReturn(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/__tests__/fixtures/compiler/error.bug-invariant-expected-consistent-destructuring.expect.md

@@ -29,7 +29,7 @@ Found 1 error:
 
 Invariant: Expected consistent kind for destructuring
 
-Other places were `Reassign` but 'mutate? #t8$46[7:9]{reactive}' is const.
+Other places were `Reassign` but 'mutate? #t8$47[7:9]{reactive}' is const.
 
 error.bug-invariant-expected-consistent-destructuring.ts:9:9
    7 |

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-unused-usememo.expect.md

@@ -1,35 +0,0 @@
-
-## Input
-
-```javascript
-// @validateNoVoidUseMemo
-function Component() {
-  useMemo(() => {
-    return [];
-  }, []);
-  return <div />;
-}
-
-```
-
-
-## Error
-
-```
-Found 1 error:
-
-Error: Unused useMemo()
-
-This useMemo() value is unused. useMemo() is for computing and caching values, not for arbitrary side effects.
-
-error.invalid-unused-usememo.ts:3:2
-  1 | // @validateNoVoidUseMemo
-  2 | function Component() {
-> 3 |   useMemo(() => {
-    |   ^^^^^^^ useMemo() result is unused
-  4 |     return [];
-  5 |   }, []);
-  6 |   return <div />;
-```
-          
-      

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

@@ -1,64 +0,0 @@
-
-## Input
-
-```javascript
-// @validateNoVoidUseMemo
-function Component() {
-  const value = useMemo(() => {
-    console.log('computing');
-  }, []);
-  const value2 = React.useMemo(() => {
-    console.log('computing');
-  }, []);
-  return (
-    <div>
-      {value}
-      {value2}
-    </div>
-  );
-}
-
-```
-
-
-## Error
-
-```
-Found 2 errors:
-
-Error: useMemo() callbacks must return a value
-
-This useMemo() callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects.
-
-error.useMemo-no-return-value.ts:3:16
-  1 | // @validateNoVoidUseMemo
-  2 | function Component() {
-> 3 |   const value = useMemo(() => {
-    |                 ^^^^^^^^^^^^^^^
-> 4 |     console.log('computing');
-    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-> 5 |   }, []);
-    | ^^^^^^^^^ useMemo() callbacks must return a value
-  6 |   const value2 = React.useMemo(() => {
-  7 |     console.log('computing');
-  8 |   }, []);
-
-Error: useMemo() callbacks must return a value
-
-This React.useMemo() callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects.
-
-error.useMemo-no-return-value.ts:6:17
-   4 |     console.log('computing');
-   5 |   }, []);
->  6 |   const value2 = React.useMemo(() => {
-     |                  ^^^^^^^^^^^^^^^^^^^^^
->  7 |     console.log('computing');
-     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
->  8 |   }, []);
-     | ^^^^^^^^^ useMemo() callbacks must return a value
-   9 |   return (
-  10 |     <div>
-  11 |       {value}
-```
-          
-      

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-unused-usememo.expect.md

@@ -0,0 +1,41 @@
+
+## Input
+
+```javascript
+// @validateNoVoidUseMemo @loggerTestOnly
+function Component() {
+  useMemo(() => {
+    return [];
+  }, []);
+  return <div />;
+}
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime"; // @validateNoVoidUseMemo @loggerTestOnly
+function Component() {
+  const $ = _c(1);
+  let t0;
+  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+    t0 = <div />;
+    $[0] = t0;
+  } else {
+    t0 = $[0];
+  }
+  return t0;
+}
+
+```
+
+## Logs
+
+```
+{"kind":"CompileError","detail":{"options":{"category":"VoidUseMemo","reason":"useMemo() result is unused","description":"This useMemo() value is unused. useMemo() is for computing and caching values, not for arbitrary side effects","suggestions":null,"details":[{"kind":"error","loc":{"start":{"line":3,"column":2,"index":67},"end":{"line":3,"column":9,"index":74},"filename":"invalid-unused-usememo.ts","identifierName":"useMemo"},"message":"useMemo() result is unused"}]}},"fnLoc":null}
+{"kind":"CompileSuccess","fnLoc":{"start":{"line":2,"column":0,"index":42},"end":{"line":7,"column":1,"index":127},"filename":"invalid-unused-usememo.ts"},"fnName":"Component","memoSlots":1,"memoBlocks":1,"memoValues":1,"prunedMemoBlocks":0,"prunedMemoValues":0}
+```
+      
+### Eval output
+(kind: exception) Fixture not implemented

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/invalid-unused-usememo.js

@@ -1,4 +1,4 @@
-// @validateNoVoidUseMemo
+// @validateNoVoidUseMemo @loggerTestOnly
 function Component() {
   useMemo(() => {
     return [];

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

@@ -0,0 +1,59 @@
+
+## Input
+
+```javascript
+// @validateNoVoidUseMemo @loggerTestOnly
+function Component() {
+  const value = useMemo(() => {
+    console.log('computing');
+  }, []);
+  const value2 = React.useMemo(() => {
+    console.log('computing');
+  }, []);
+  return (
+    <div>
+      {value}
+      {value2}
+    </div>
+  );
+}
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime"; // @validateNoVoidUseMemo @loggerTestOnly
+function Component() {
+  const $ = _c(1);
+
+  console.log("computing");
+
+  console.log("computing");
+  let t0;
+  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+    t0 = (
+      <div>
+        {undefined}
+        {undefined}
+      </div>
+    );
+    $[0] = t0;
+  } else {
+    t0 = $[0];
+  }
+  return t0;
+}
+
+```
+
+## Logs
+
+```
+{"kind":"CompileError","detail":{"options":{"category":"VoidUseMemo","reason":"useMemo() callbacks must return a value","description":"This useMemo() callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects","suggestions":null,"details":[{"kind":"error","loc":{"start":{"line":3,"column":24,"index":89},"end":{"line":5,"column":3,"index":130},"filename":"invalid-useMemo-no-return-value.ts"},"message":"useMemo() callbacks must return a value"}]}},"fnLoc":null}
+{"kind":"CompileError","detail":{"options":{"category":"VoidUseMemo","reason":"useMemo() callbacks must return a value","description":"This useMemo() callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects","suggestions":null,"details":[{"kind":"error","loc":{"start":{"line":6,"column":31,"index":168},"end":{"line":8,"column":3,"index":209},"filename":"invalid-useMemo-no-return-value.ts"},"message":"useMemo() callbacks must return a value"}]}},"fnLoc":null}
+{"kind":"CompileSuccess","fnLoc":{"start":{"line":2,"column":0,"index":42},"end":{"line":15,"column":1,"index":283},"filename":"invalid-useMemo-no-return-value.ts"},"fnName":"Component","memoSlots":1,"memoBlocks":1,"memoValues":1,"prunedMemoBlocks":0,"prunedMemoValues":0}
+```
+      
+### Eval output
+(kind: exception) Fixture not implemented

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

@@ -1,4 +1,4 @@
-// @validateNoVoidUseMemo
+// @validateNoVoidUseMemo @loggerTestOnly
 function Component() {
   const value = useMemo(() => {
     console.log('computing');

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

@@ -0,0 +1,33 @@
+
+## Input
+
+```javascript
+// @loggerTestOnly
+function component(a) {
+  let x = useMemo(() => {
+    mutate(a);
+  }, []);
+  return x;
+}
+
+```
+
+## Code
+
+```javascript
+// @loggerTestOnly
+function component(a) {
+  mutate(a);
+}
+
+```
+
+## Logs
+
+```
+{"kind":"CompileError","detail":{"options":{"category":"VoidUseMemo","reason":"useMemo() callbacks must return a value","description":"This useMemo() callback doesn't return a value. useMemo() is for computing and caching values, not for arbitrary side effects","suggestions":null,"details":[{"kind":"error","loc":{"start":{"line":3,"column":18,"index":61},"end":{"line":5,"column":3,"index":87},"filename":"invalid-useMemo-return-empty.ts"},"message":"useMemo() callbacks must return a value"}]}},"fnLoc":null}
+{"kind":"CompileSuccess","fnLoc":{"start":{"line":2,"column":0,"index":19},"end":{"line":7,"column":1,"index":107},"filename":"invalid-useMemo-return-empty.ts"},"fnName":"component","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":1,"prunedMemoValues":0}
+```
+      
+### Eval output
+(kind: exception) Fixture not implemented

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

@@ -1,3 +1,4 @@
+// @loggerTestOnly
 function component(a) {
   let x = useMemo(() => {
     mutate(a);

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useMemo-dropped-infer-always-invalidating.expect.md

@@ -1,49 +0,0 @@
-
-## Input
-
-```javascript
-// @validatePreserveExistingMemoizationGuarantees
-
-import {useMemo} from 'react';
-import {useHook} from 'shared-runtime';
-
-// useMemo values may not be memoized in Forget output if we
-// infer that their deps always invalidate.
-// This is technically a false positive as the useMemo in source
-// was effectively a no-op
-function useFoo(props) {
-  const x = [];
-  useHook();
-  x.push(props);
-
-  return useMemo(() => [x], [x]);
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: useFoo,
-  params: [{}],
-};
-
-```
-
-
-## Error
-
-```
-Found 1 error:
-
-Compilation Skipped: Existing memoization could not be preserved
-
-React Compiler has skipped optimizing this component because the existing manual memoization could not be preserved. This value was memoized in source but not in compilation output.
-
-error.false-positive-useMemo-dropped-infer-always-invalidating.ts:15:9
-  13 |   x.push(props);
-  14 |
-> 15 |   return useMemo(() => [x], [x]);
-     |          ^^^^^^^^^^^^^^^^^^^^^^^ Could not preserve existing memoization
-  16 | }
-  17 |
-  18 | export const FIXTURE_ENTRYPOINT = {
-```
-          
-      

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/error.false-positive-useMemo-dropped-infer-always-invalidating.ts

@@ -1,21 +0,0 @@
-// @validatePreserveExistingMemoizationGuarantees
-
-import {useMemo} from 'react';
-import {useHook} from 'shared-runtime';
-
-// useMemo values may not be memoized in Forget output if we
-// infer that their deps always invalidate.
-// This is technically a false positive as the useMemo in source
-// was effectively a no-op
-function useFoo(props) {
-  const x = [];
-  useHook();
-  x.push(props);
-
-  return useMemo(() => [x], [x]);
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: useFoo,
-  params: [{}],
-};

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/repro-useMemo-counts-as-preserved-if-it-was-always-invalidating.expect.md

@@ -0,0 +1,56 @@
+
+## Input
+
+```javascript
+// @validatePreserveExistingMemoizationGuarantees
+
+import {useMemo} from 'react';
+import {useHook} from 'shared-runtime';
+
+// If we can prove that a useMemo was ineffective because it would always invalidate,
+// then we shouldn't throw a "couldn't preserve existing memoization" error
+// TODO: consider reporting a separate error to the user for this case, if you're going
+// to memoize manually, then you probably want to know that it's a no-op
+function useFoo(props) {
+  const x = [];
+  useHook();
+  x.push(props);
+
+  return useMemo(() => [x], [x]);
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: useFoo,
+  params: [{}],
+};
+
+```
+
+## Code
+
+```javascript
+// @validatePreserveExistingMemoizationGuarantees
+
+import { useMemo } from "react";
+import { useHook } from "shared-runtime";
+
+// If we can prove that a useMemo was ineffective because it would always invalidate,
+// then we shouldn't throw a "couldn't preserve existing memoization" error
+// TODO: consider reporting a separate error to the user for this case, if you're going
+// to memoize manually, then you probably want to know that it's a no-op
+function useFoo(props) {
+  const x = [];
+  useHook();
+  x.push(props);
+  return [x];
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: useFoo,
+  params: [{}],
+};
+
+```
+      
+### Eval output
+(kind: ok) [[{}]]

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/preserve-memo-validation/repro-useMemo-counts-as-preserved-if-it-was-always-invalidating.ts

@@ -0,0 +1,21 @@
+// @validatePreserveExistingMemoizationGuarantees
+
+import {useMemo} from 'react';
+import {useHook} from 'shared-runtime';
+
+// If we can prove that a useMemo was ineffective because it would always invalidate,
+// then we shouldn't throw a "couldn't preserve existing memoization" error
+// TODO: consider reporting a separate error to the user for this case, if you're going
+// to memoize manually, then you probably want to know that it's a no-op
+function useFoo(props) {
+  const x = [];
+  useHook();
+  x.push(props);
+
+  return useMemo(() => [x], [x]);
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: useFoo,
+  params: [{}],
+};

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-inline-unnamed-useMemo-with-multiple-returns.expect.md

@@ -0,0 +1,98 @@
+
+## Input
+
+```javascript
+import {useMemo} from 'react';
+import {identity, useIdentity} from 'shared-runtime';
+
+// Adapted from https://github.com/facebook/react/issues/34750
+function useLocalCampaignBySlug(slug: string) {
+  const campaigns = useIdentity({a: {slug: 'a', name: 'campaign'}});
+  // The useMemo result is never assigned to a local so we did not previously ensure
+  // that there was a variable declaration for it when promoting the result temporary
+  return useMemo(() => {
+    for (const id of Object.keys(campaigns)) {
+      const campaign = campaigns[id];
+      if (campaign.slug === slug) {
+        return identity(campaign);
+      }
+    }
+    return null;
+  }, [campaigns, slug]);
+}
+
+function Component() {
+  const campaign = useLocalCampaignBySlug('a');
+  return <div>{campaign.name}</div>;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{}],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+import { useMemo } from "react";
+import { identity, useIdentity } from "shared-runtime";
+
+// Adapted from https://github.com/facebook/react/issues/34750
+function useLocalCampaignBySlug(slug) {
+  const $ = _c(4);
+  let t0;
+  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+    t0 = { a: { slug: "a", name: "campaign" } };
+    $[0] = t0;
+  } else {
+    t0 = $[0];
+  }
+  const campaigns = useIdentity(t0);
+  let t1;
+  if ($[1] !== campaigns || $[2] !== slug) {
+    bb0: {
+      for (const id of Object.keys(campaigns)) {
+        const campaign = campaigns[id];
+        if (campaign.slug === slug) {
+          t1 = identity(campaign);
+          break bb0;
+        }
+      }
+
+      t1 = null;
+    }
+    $[1] = campaigns;
+    $[2] = slug;
+    $[3] = t1;
+  } else {
+    t1 = $[3];
+  }
+  return t1;
+}
+
+function Component() {
+  const $ = _c(2);
+  const campaign = useLocalCampaignBySlug("a");
+  let t0;
+  if ($[0] !== campaign.name) {
+    t0 = <div>{campaign.name}</div>;
+    $[0] = campaign.name;
+    $[1] = t0;
+  } else {
+    t0 = $[1];
+  }
+  return t0;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{}],
+};
+
+```
+      
+### Eval output
+(kind: ok) <div>campaign</div>

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-inline-unnamed-useMemo-with-multiple-returns.js

@@ -0,0 +1,28 @@
+import {useMemo} from 'react';
+import {identity, useIdentity} from 'shared-runtime';
+
+// Adapted from https://github.com/facebook/react/issues/34750
+function useLocalCampaignBySlug(slug: string) {
+  const campaigns = useIdentity({a: {slug: 'a', name: 'campaign'}});
+  // The useMemo result is never assigned to a local so we did not previously ensure
+  // that there was a variable declaration for it when promoting the result temporary
+  return useMemo(() => {
+    for (const id of Object.keys(campaigns)) {
+      const campaign = campaigns[id];
+      if (campaign.slug === slug) {
+        return identity(campaign);
+      }
+    }
+    return null;
+  }, [campaigns, slug]);
+}
+
+function Component() {
+  const campaign = useLocalCampaignBySlug('a');
+  return <div>{campaign.name}</div>;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{}],
+};

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro.expect.md

@@ -2,6 +2,7 @@
 ## Input
 
 ```javascript
+// @validateNoVoidUseMemo:false
 function Component(props) {
   const item = props.item;
   const thumbnails = [];
@@ -22,7 +23,7 @@ function Component(props) {
 ## Code
 
 ```javascript
-import { c as _c } from "react/compiler-runtime";
+import { c as _c } from "react/compiler-runtime"; // @validateNoVoidUseMemo:false
 function Component(props) {
   const $ = _c(6);
   const item = props.item;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro.js

@@ -1,3 +1,4 @@
+// @validateNoVoidUseMemo:false
 function Component(props) {
   const item = props.item;
   const thumbnails = [];

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-nested-ifs.expect.md

@@ -6,6 +6,7 @@ function Component(props) {
   const x = useMemo(() => {
     if (props.cond) {
       if (props.cond) {
+        return props.value;
       }
     }
   }, [props.cond]);
@@ -24,10 +25,18 @@ export const FIXTURE_ENTRYPOINT = {
 
 ```javascript
 function Component(props) {
-  if (props.cond) {
+  let t0;
+  bb0: {
     if (props.cond) {
+      if (props.cond) {
+        t0 = props.value;
+        break bb0;
+      }
     }
+    t0 = undefined;
   }
+  const x = t0;
+  return x;
 }
 
 export const FIXTURE_ENTRYPOINT = {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/useMemo-nested-ifs.js

@@ -2,6 +2,7 @@ function Component(props) {
   const x = useMemo(() => {
     if (props.cond) {
       if (props.cond) {
+        return props.value;
       }
     }
   }, [props.cond]);

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

@@ -1,22 +0,0 @@
-
-## Input
-
-```javascript
-function component(a) {
-  let x = useMemo(() => {
-    mutate(a);
-  }, []);
-  return x;
-}
-
-```
-
-## Code
-
-```javascript
-function component(a) {
-  mutate(a);
-}
-
-```
-      

compiler/packages/eslint-plugin-react-compiler/__tests__/PluginTest-test.ts

@@ -120,7 +120,15 @@ testRule('plugin-recommended', TestRecommendedRules, {
             
         return <Child x={state} />;
         }`,
-      errors: [makeTestCaseError('Unused useMemo()')],
+      errors: [
+        makeTestCaseError('useMemo() callbacks must return a value'),
+        makeTestCaseError(
+          'Calling setState from useMemo may trigger an infinite loop',
+        ),
+        makeTestCaseError(
+          'Calling setState from useMemo may trigger an infinite loop',
+        ),
+      ],
     },
     {
       name: 'Pipeline errors are reported',