[commit] Improve error for hoisting violations

The previous error for hoisting violations pointed only to the variable declaration, but didn't show where the value was accessed before that declaration. We now track where each hoisted variable is first accessed and report two errors, one for the reference and one for the declaration. When we improve our diagnostic infra to support reporting errors at multiple locations we can merge these into a single conceptual error.

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

@@ -72,7 +72,7 @@ export function lower(
   env: Environment,
   // Bindings captured from the outer function, in case lower() is called recursively (for lambdas)
   bindings: Bindings | null = null,
-  capturedRefs: Array<t.Identifier> = [],
+  capturedRefs: Map<t.Identifier, SourceLocation> = new Map(),
 ): Result<HIRFunction, CompilerError> {
   const builder = new HIRBuilder(env, {
     bindings,
@@ -80,13 +80,13 @@ export function lower(
   });
   const context: HIRFunction['context'] = [];
 
-  for (const ref of capturedRefs ?? []) {
+  for (const [ref, loc] of capturedRefs ?? []) {
     context.push({
       kind: 'Identifier',
       identifier: builder.resolveBinding(ref),
       effect: Effect.Unknown,
       reactive: false,
-      loc: ref.loc ?? GeneratedSource,
+      loc,
     });
   }
 
@@ -3439,10 +3439,12 @@ function lowerFunction(
    * This isn't a problem in practice because use Babel's scope analysis to
    * identify the correct references.
    */
-  const lowering = lower(expr, builder.environment, builder.bindings, [
-    ...builder.context,
-    ...capturedContext,
-  ]);
+  const lowering = lower(
+    expr,
+    builder.environment,
+    builder.bindings,
+    new Map([...builder.context, ...capturedContext]),
+  );
   let loweredFunc: HIRFunction;
   if (lowering.isErr()) {
     lowering
@@ -4160,6 +4162,11 @@ function captureScopes({from, to}: {from: Scope; to: Scope}): Set<Scope> {
   return scopes;
 }
 
+/**
+ * Returns a mapping of "context" identifiers — references to free variables that
+ * will become part of the function expression's `context` array — along with the
+ * source location of their first reference within the function.
+ */
 function gatherCapturedContext(
   fn: NodePath<
     | t.FunctionExpression
@@ -4168,8 +4175,8 @@ function gatherCapturedContext(
     | t.ObjectMethod
   >,
   componentScope: Scope,
-): Array<t.Identifier> {
-  const capturedIds = new Set<t.Identifier>();
+): Map<t.Identifier, SourceLocation> {
+  const capturedIds = new Map<t.Identifier, SourceLocation>();
 
   /*
    * Capture all the scopes from the parent of this function up to and including
@@ -4212,8 +4219,15 @@ function gatherCapturedContext(
 
     // Add the base identifier binding as a dependency.
     const binding = baseIdentifier.scope.getBinding(baseIdentifier.node.name);
-    if (binding !== undefined && pureScopes.has(binding.scope)) {
-      capturedIds.add(binding.identifier);
+    if (
+      binding !== undefined &&
+      pureScopes.has(binding.scope) &&
+      !capturedIds.has(binding.identifier)
+    ) {
+      capturedIds.set(
+        binding.identifier,
+        path.node.loc ?? binding.identifier.loc ?? GeneratedSource,
+      );
     }
   }
 
@@ -4250,7 +4264,7 @@ function gatherCapturedContext(
     },
   });
 
-  return [...capturedIds.keys()];
+  return capturedIds;
 }
 
 function notNull<T>(value: T | null): value is T {

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

@@ -1388,6 +1388,16 @@ export enum ValueReason {
    */
   JsxCaptured = 'jsx-captured',
 
+  /**
+   * Argument to a hook
+   */
+  HookCaptured = 'hook-captured',
+
+  /**
+   * Return value of a hook
+   */
+  HookReturn = 'hook-return',
+
   /**
    * Passed to an effect
    */

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

@@ -106,7 +106,7 @@ export default class HIRBuilder {
   #current: WipBlock;
   #entry: BlockId;
   #scopes: Array<Scope> = [];
-  #context: Array<t.Identifier>;
+  #context: Map<t.Identifier, SourceLocation>;
   #bindings: Bindings;
   #env: Environment;
   #exceptionHandlerStack: Array<BlockId> = [];
@@ -121,7 +121,7 @@ export default class HIRBuilder {
     return this.#env.nextIdentifierId;
   }
 
-  get context(): Array<t.Identifier> {
+  get context(): Map<t.Identifier, SourceLocation> {
     return this.#context;
   }
 
@@ -137,13 +137,13 @@ export default class HIRBuilder {
     env: Environment,
     options?: {
       bindings?: Bindings | null;
-      context?: Array<t.Identifier>;
+      context?: Map<t.Identifier, SourceLocation>;
       entryBlockKind?: BlockKind;
     },
   ) {
     this.#env = env;
     this.#bindings = options?.bindings ?? new Map();
-    this.#context = options?.context ?? [];
+    this.#context = options?.context ?? new Map();
     this.#entry = makeBlockId(env.nextBlockId);
     this.#current = newBlock(this.#entry, options?.entryBlockKind ?? 'block');
   }

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

@@ -1302,6 +1302,34 @@ export const DefaultNonmutatingHook = addHook(
     calleeEffect: Effect.Read,
     hookKind: 'Custom',
     returnValueKind: ValueKind.Frozen,
+    aliasing: {
+      receiver: makeIdentifierId(0),
+      params: [],
+      rest: makeIdentifierId(1),
+      returns: makeIdentifierId(2),
+      temporaries: [],
+      effects: [
+        // Freeze the arguments
+        {
+          kind: 'Freeze',
+          value: signatureArgument(1),
+          reason: ValueReason.HookCaptured,
+        },
+        // Returns a frozen value
+        {
+          kind: 'Create',
+          into: signatureArgument(2),
+          value: ValueKind.Frozen,
+          reason: ValueReason.HookReturn,
+        },
+        // May alias any arguments into the return
+        {
+          kind: 'Alias',
+          from: signatureArgument(1),
+          into: signatureArgument(2),
+        },
+      ],
+    },
   },
   'DefaultNonmutatingHook',
 );

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

@@ -42,8 +42,16 @@ export default function analyseFunctions(func: HIRFunction): void {
            * Reset mutable range for outer inferReferenceEffects
            */
           for (const operand of instr.value.loweredFunc.func.context) {
-            operand.identifier.mutableRange.start = makeInstructionId(0);
-            operand.identifier.mutableRange.end = makeInstructionId(0);
+            /**
+             * NOTE: inferReactiveScopeVariables makes identifiers in the scope
+             * point to the *same* mutableRange instance. Resetting start/end
+             * here is insufficient, because a later mutation of the range
+             * for any one identifier could affect the range for other identifiers.
+             */
+            operand.identifier.mutableRange = {
+              start: makeInstructionId(0),
+              end: makeInstructionId(0),
+            };
             operand.identifier.scope = null;
           }
           break;

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

@@ -341,6 +341,10 @@ export function getWriteErrorReason(abstractValue: AbstractValue): string {
     return "Mutating a value returned from 'useReducer()', which should not be mutated. Use the dispatch function to update instead";
   } else if (abstractValue.reason.has(ValueReason.Effect)) {
     return 'Updating a value used previously in an effect function or as an effect dependency is not allowed. Consider moving the mutation before calling useEffect()';
+  } else if (abstractValue.reason.has(ValueReason.HookCaptured)) {
+    return 'Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook';
+  } else if (abstractValue.reason.has(ValueReason.HookReturn)) {
+    return 'Updating a value returned from a hook is not allowed. Consider moving the mutation into the hook where the value is constructed';
   } else {
     return 'This mutates a variable that React considers immutable';
   }

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

@@ -38,6 +38,7 @@ import {
 import {
   eachInstructionValueLValue,
   eachInstructionValueOperand,
+  eachTerminalOperand,
   eachTerminalSuccessor,
 } from '../HIR/visitors';
 import {Ok, Result} from '../Utils/Result';
@@ -221,8 +222,19 @@ export function inferMutationAliasingEffects(
   return Ok(undefined);
 }
 
-function findHoistedContextDeclarations(fn: HIRFunction): Set<DeclarationId> {
-  const hoisted = new Set<DeclarationId>();
+function findHoistedContextDeclarations(
+  fn: HIRFunction,
+): Map<DeclarationId, Place | null> {
+  const hoisted = new Map<DeclarationId, Place | null>();
+  function visit(place: Place): void {
+    if (
+      hoisted.has(place.identifier.declarationId) &&
+      hoisted.get(place.identifier.declarationId) == null
+    ) {
+      // If this is the first load of the value, store the location
+      hoisted.set(place.identifier.declarationId, place);
+    }
+  }
   for (const block of fn.body.blocks.values()) {
     for (const instr of block.instructions) {
       if (instr.value.kind === 'DeclareContext') {
@@ -232,10 +244,17 @@ function findHoistedContextDeclarations(fn: HIRFunction): Set<DeclarationId> {
           kind == InstructionKind.HoistedFunction ||
           kind == InstructionKind.HoistedLet
         ) {
-          hoisted.add(instr.value.lvalue.place.identifier.declarationId);
+          hoisted.set(instr.value.lvalue.place.identifier.declarationId, null);
+        }
+      } else {
+        for (const operand of eachInstructionValueOperand(instr.value)) {
+          visit(operand);
         }
       }
     }
+    for (const operand of eachTerminalOperand(block.terminal)) {
+      visit(operand);
+    }
   }
   return hoisted;
 }
@@ -248,12 +267,12 @@ class Context {
   catchHandlers: Map<BlockId, Place> = new Map();
   isFuctionExpression: boolean;
   fn: HIRFunction;
-  hoistedContextDeclarations: Set<DeclarationId>;
+  hoistedContextDeclarations: Map<DeclarationId, Place | null>;
 
   constructor(
     isFunctionExpression: boolean,
     fn: HIRFunction,
-    hoistedContextDeclarations: Set<DeclarationId>,
+    hoistedContextDeclarations: Map<DeclarationId, Place | null>,
   ) {
     this.isFuctionExpression = isFunctionExpression;
     this.fn = fn;
@@ -901,27 +920,69 @@ function applyEffect(
           console.log(prettyFormat(state.debugAbstractValue(value)));
         }
 
-        const reason = getWriteErrorReason({
-          kind: value.kind,
-          reason: value.reason,
-          context: new Set(),
-        });
-        effects.push({
-          kind:
-            value.kind === ValueKind.Frozen ? 'MutateFrozen' : 'MutateGlobal',
-          place: effect.value,
-          error: {
-            severity: ErrorSeverity.InvalidReact,
-            reason,
-            description:
-              effect.value.identifier.name !== null &&
-              effect.value.identifier.name.kind === 'named'
-                ? `Found mutation of \`${effect.value.identifier.name.value}\``
-                : null,
-            loc: effect.value.loc,
-            suggestions: null,
-          },
-        });
+        if (
+          mutationKind === 'mutate-frozen' &&
+          context.hoistedContextDeclarations.has(
+            effect.value.identifier.declarationId,
+          )
+        ) {
+          const description =
+            effect.value.identifier.name !== null &&
+            effect.value.identifier.name.kind === 'named'
+              ? `Variable \`${effect.value.identifier.name.value}\` is accessed before it is declared`
+              : null;
+          const hoistedAccess = context.hoistedContextDeclarations.get(
+            effect.value.identifier.declarationId,
+          );
+          if (hoistedAccess != null && hoistedAccess.loc != effect.value.loc) {
+            effects.push({
+              kind: 'MutateFrozen',
+              place: effect.value,
+              error: {
+                severity: ErrorSeverity.InvalidReact,
+                reason: `This variable is accessed before it is declared, which may prevent it from updating as the assigned value changes over time`,
+                description,
+                loc: hoistedAccess.loc,
+                suggestions: null,
+              },
+            });
+          }
+
+          effects.push({
+            kind: 'MutateFrozen',
+            place: effect.value,
+            error: {
+              severity: ErrorSeverity.InvalidReact,
+              reason: `This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time`,
+              description,
+              loc: effect.value.loc,
+              suggestions: null,
+            },
+          });
+        } else {
+          const reason = getWriteErrorReason({
+            kind: value.kind,
+            reason: value.reason,
+            context: new Set(),
+          });
+          const description =
+            effect.value.identifier.name !== null &&
+            effect.value.identifier.name.kind === 'named'
+              ? `Found mutation of \`${effect.value.identifier.name.value}\``
+              : null;
+          effects.push({
+            kind:
+              value.kind === ValueKind.Frozen ? 'MutateFrozen' : 'MutateGlobal',
+            place: effect.value,
+            error: {
+              severity: ErrorSeverity.InvalidReact,
+              reason,
+              description,
+              loc: effect.value.loc,
+              suggestions: null,
+            },
+          });
+        }
       }
       break;
     }
@@ -1959,28 +2020,17 @@ function computeEffectsForLegacySignature(
         break;
       }
       case Effect.ConditionallyMutateIterator: {
-        if (
-          isArrayType(place.identifier) ||
-          isSetType(place.identifier) ||
-          isMapType(place.identifier)
-        ) {
-          effects.push({
-            kind: 'Capture',
-            from: place,
-            into: lvalue,
-          });
-        } else {
-          effects.push({
-            kind: 'Capture',
-            from: place,
-            into: lvalue,
-          });
+        const mutateIterator = conditionallyMutateIterator(place);
+        if (mutateIterator != null) {
+          effects.push(mutateIterator);
+          // TODO: should we always push to captures?
           captures.push(place);
-          effects.push({
-            kind: 'MutateTransitiveConditionally',
-            value: place,
-          });
         }
+        effects.push({
+          kind: 'Capture',
+          from: place,
+          into: lvalue,
+        });
         break;
       }
       case Effect.Freeze: {
@@ -2170,6 +2220,7 @@ function computeEffectsForSignature(
     return null;
   }
   // Build substitutions
+  const mutableSpreads = new Set<IdentifierId>();
   const substitutions: Map<IdentifierId, Array<Place>> = new Map();
   substitutions.set(signature.receiver, [receiver]);
   substitutions.set(signature.returns, [lvalue]);
@@ -2187,6 +2238,13 @@ function computeEffectsForSignature(
       }
       const place = arg.kind === 'Identifier' ? arg : arg.place;
       getOrInsertWith(substitutions, signature.rest, () => []).push(place);
+
+      if (arg.kind === 'Spread') {
+        const mutateIterator = conditionallyMutateIterator(arg.place);
+        if (mutateIterator != null) {
+          mutableSpreads.add(arg.place.identifier.id);
+        }
+      }
     } else {
       const param = params[i];
       substitutions.set(param, [arg]);
@@ -2258,6 +2316,12 @@ function computeEffectsForSignature(
       case 'Freeze': {
         const values = substitutions.get(effect.value.identifier.id) ?? [];
         for (const value of values) {
+          if (mutableSpreads.has(value.identifier.id)) {
+            CompilerError.throwTodo({
+              reason: 'Support spread syntax for hook arguments',
+              loc: value.loc,
+            });
+          }
           effects.push({kind: 'Freeze', value, reason: effect.reason});
         }
         break;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-call-freezes-captured-identifier.expect.md

@@ -32,7 +32,7 @@ export const FIXTURE_ENTRYPOINT = {
   11 |   });
   12 |
 > 13 |   x.value += count;
-     |   ^ InvalidReact: This mutates a variable that React considers immutable (13:13)
+     |   ^ InvalidReact: Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook (13:13)
   14 |   return <Stringify x={x} cb={cb} />;
   15 | }
   16 |

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.hook-call-freezes-captured-memberexpr.expect.md

@@ -32,7 +32,7 @@ export const FIXTURE_ENTRYPOINT = {
   11 |   });
   12 |
 > 13 |   x.value += count;
-     |   ^ InvalidReact: This mutates a variable that React considers immutable (13:13)
+     |   ^ InvalidReact: Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook (13:13)
   14 |   return <Stringify x={x} cb={cb} />;
   15 | }
   16 |

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-hoisting-setstate.expect.md

@@ -38,13 +38,15 @@ export const FIXTURE_ENTRYPOINT = {
 ## Error
 
 ```
-  19 |   useEffect(() => setState(2), []);
+  17 |    * $2 = Function context=setState
+  18 |    */
+> 19 |   useEffect(() => setState(2), []);
+     |                   ^^^^^^^^ InvalidReact: This variable is accessed before it is declared, which may prevent it from updating as the assigned value changes over time. Variable `setState` is accessed before it is declared (19:19)
+
+InvalidReact: This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time. Variable `setState` is accessed before it is declared (21:21)
   20 |
-> 21 |   const [state, setState] = useState(0);
-     |                 ^^^^^^^^ InvalidReact: Updating a value used previously in an effect function or as an effect dependency is not allowed. Consider moving the mutation before calling useEffect(). Found mutation of `setState` (21:21)
+  21 |   const [state, setState] = useState(0);
   22 |   return <Stringify state={state} />;
-  23 | }
-  24 |
 ```
           
       

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-non-imported-reanimated-shared-value-writes.expect.md

@@ -27,7 +27,7 @@ function SomeComponent() {
    9 |   return (
   10 |     <Button
 > 11 |       onPress={() => (sharedVal.value = Math.random())}
-     |                       ^^^^^^^^^ InvalidReact: This mutates a variable that React considers immutable. Found mutation of `sharedVal` (11:11)
+     |                       ^^^^^^^^^ InvalidReact: Updating a value returned from a hook is not allowed. Consider moving the mutation into the hook where the value is constructed. Found mutation of `sharedVal` (11:11)
   12 |       title="Randomize"
   13 |     />
   14 |   );

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-valid-functiondecl-hoisting.expect.md

@@ -34,13 +34,13 @@ export const FIXTURE_ENTRYPOINT = {
 ## Error
 
 ```
-  13 |     return bar();
+  11 |
+  12 |   function foo() {
+> 13 |     return bar();
+     |            ^^^ Todo: [PruneHoistedContexts] Rewrite hoisted function references (13:13)
   14 |   }
-> 15 |   function bar() {
-     |            ^^^ Todo: [PruneHoistedContexts] Rewrite hoisted function references (15:15)
+  15 |   function bar() {
   16 |     return 42;
-  17 |   }
-  18 |
 ```
           
       

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-effect-dependencies/no-emit/retry-no-emit.expect.md

@@ -52,7 +52,7 @@ export const FIXTURE_ENTRYPOINT = {
 ## Logs
 
 ```
-{"kind":"CompileError","fnLoc":{"start":{"line":5,"column":0,"index":163},"end":{"line":13,"column":1,"index":357},"filename":"retry-no-emit.ts"},"detail":{"reason":"This mutates a variable that React considers immutable","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":11,"column":2,"index":320},"end":{"line":11,"column":6,"index":324},"filename":"retry-no-emit.ts","identifierName":"arr2"}}}
+{"kind":"CompileError","fnLoc":{"start":{"line":5,"column":0,"index":163},"end":{"line":13,"column":1,"index":357},"filename":"retry-no-emit.ts"},"detail":{"reason":"Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":11,"column":2,"index":320},"end":{"line":11,"column":6,"index":324},"filename":"retry-no-emit.ts","identifierName":"arr2"}}}
 {"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":7,"column":2,"index":216},"end":{"line":7,"column":36,"index":250},"filename":"retry-no-emit.ts"},"decorations":[{"start":{"line":7,"column":31,"index":245},"end":{"line":7,"column":34,"index":248},"filename":"retry-no-emit.ts","identifierName":"arr"}]}
 {"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":10,"column":2,"index":274},"end":{"line":10,"column":44,"index":316},"filename":"retry-no-emit.ts"},"decorations":[{"start":{"line":10,"column":25,"index":297},"end":{"line":10,"column":29,"index":301},"filename":"retry-no-emit.ts","identifierName":"arr2"},{"start":{"line":10,"column":25,"index":297},"end":{"line":10,"column":29,"index":301},"filename":"retry-no-emit.ts","identifierName":"arr2"},{"start":{"line":10,"column":35,"index":307},"end":{"line":10,"column":42,"index":314},"filename":"retry-no-emit.ts","identifierName":"propVal"}]}
 {"kind":"CompileSuccess","fnLoc":{"start":{"line":5,"column":0,"index":163},"end":{"line":13,"column":1,"index":357},"filename":"retry-no-emit.ts"},"fnName":"Foo","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/inner-function/nullable-objects/array-map-named-callback-cross-context.expect.md

@@ -57,67 +57,62 @@ import { Stringify } from "shared-runtime";
  * - cb1 is not assumed to be called since it's only used as a call operand
  */
 function useFoo(t0) {
-  const $ = _c(14);
-  let arr1;
-  let arr2;
+  const $ = _c(13);
+  const { arr1, arr2 } = t0;
   let t1;
-  if ($[0] !== t0) {
-    ({ arr1, arr2 } = t0);
-    let t2;
-    if ($[4] !== arr1[0]) {
-      t2 = (e) => arr1[0].value + e.value;
-      $[4] = arr1[0];
-      $[5] = t2;
-    } else {
-      t2 = $[5];
-    }
-    const cb1 = t2;
-    t1 = () => arr1.map(cb1);
-    $[0] = t0;
-    $[1] = arr1;
-    $[2] = arr2;
-    $[3] = t1;
+  if ($[0] !== arr1[0]) {
+    t1 = (e) => arr1[0].value + e.value;
+    $[0] = arr1[0];
+    $[1] = t1;
   } else {
-    arr1 = $[1];
-    arr2 = $[2];
-    t1 = $[3];
+    t1 = $[1];
   }
-  const getArrMap1 = t1;
+  const cb1 = t1;
   let t2;
-  if ($[6] !== arr2) {
-    t2 = (e_0) => arr2[0].value + e_0.value;
-    $[6] = arr2;
-    $[7] = t2;
+  if ($[2] !== arr1 || $[3] !== cb1) {
+    t2 = () => arr1.map(cb1);
+    $[2] = arr1;
+    $[3] = cb1;
+    $[4] = t2;
   } else {
-    t2 = $[7];
+    t2 = $[4];
   }
-  const cb2 = t2;
+  const getArrMap1 = t2;
   let t3;
-  if ($[8] !== arr1 || $[9] !== cb2) {
-    t3 = () => arr1.map(cb2);
-    $[8] = arr1;
-    $[9] = cb2;
-    $[10] = t3;
+  if ($[5] !== arr2) {
+    t3 = (e_0) => arr2[0].value + e_0.value;
+    $[5] = arr2;
+    $[6] = t3;
   } else {
-    t3 = $[10];
+    t3 = $[6];
   }
-  const getArrMap2 = t3;
+  const cb2 = t3;
   let t4;
-  if ($[11] !== getArrMap1 || $[12] !== getArrMap2) {
-    t4 = (
+  if ($[7] !== arr1 || $[8] !== cb2) {
+    t4 = () => arr1.map(cb2);
+    $[7] = arr1;
+    $[8] = cb2;
+    $[9] = t4;
+  } else {
+    t4 = $[9];
+  }
+  const getArrMap2 = t4;
+  let t5;
+  if ($[10] !== getArrMap1 || $[11] !== getArrMap2) {
+    t5 = (
       <Stringify
         getArrMap1={getArrMap1}
         getArrMap2={getArrMap2}
         shouldInvokeFns={true}
       />
     );
-    $[11] = getArrMap1;
-    $[12] = getArrMap2;
-    $[13] = t4;
+    $[10] = getArrMap1;
+    $[11] = getArrMap2;
+    $[12] = t5;
   } else {
-    t4 = $[13];
+    t5 = $[12];
   }
-  return t4;
+  return t5;
 }
 
 export const FIXTURE_ENTRYPOINT = {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/array-map-named-callback-cross-context.expect.md

@@ -58,67 +58,62 @@ import { Stringify } from "shared-runtime";
  * - cb1 is not assumed to be called since it's only used as a call operand
  */
 function useFoo(t0) {
-  const $ = _c(14);
-  let arr1;
-  let arr2;
+  const $ = _c(13);
+  const { arr1, arr2 } = t0;
   let t1;
-  if ($[0] !== t0) {
-    ({ arr1, arr2 } = t0);
-    let t2;
-    if ($[4] !== arr1[0]) {
-      t2 = (e) => arr1[0].value + e.value;
-      $[4] = arr1[0];
-      $[5] = t2;
-    } else {
-      t2 = $[5];
-    }
-    const cb1 = t2;
-    t1 = () => arr1.map(cb1);
-    $[0] = t0;
-    $[1] = arr1;
-    $[2] = arr2;
-    $[3] = t1;
+  if ($[0] !== arr1[0]) {
+    t1 = (e) => arr1[0].value + e.value;
+    $[0] = arr1[0];
+    $[1] = t1;
   } else {
-    arr1 = $[1];
-    arr2 = $[2];
-    t1 = $[3];
+    t1 = $[1];
   }
-  const getArrMap1 = t1;
+  const cb1 = t1;
   let t2;
-  if ($[6] !== arr2) {
-    t2 = (e_0) => arr2[0].value + e_0.value;
-    $[6] = arr2;
-    $[7] = t2;
+  if ($[2] !== arr1 || $[3] !== cb1) {
+    t2 = () => arr1.map(cb1);
+    $[2] = arr1;
+    $[3] = cb1;
+    $[4] = t2;
   } else {
-    t2 = $[7];
+    t2 = $[4];
   }
-  const cb2 = t2;
+  const getArrMap1 = t2;
   let t3;
-  if ($[8] !== arr1 || $[9] !== cb2) {
-    t3 = () => arr1.map(cb2);
-    $[8] = arr1;
-    $[9] = cb2;
-    $[10] = t3;
+  if ($[5] !== arr2) {
+    t3 = (e_0) => arr2[0].value + e_0.value;
+    $[5] = arr2;
+    $[6] = t3;
   } else {
-    t3 = $[10];
+    t3 = $[6];
   }
-  const getArrMap2 = t3;
+  const cb2 = t3;
   let t4;
-  if ($[11] !== getArrMap1 || $[12] !== getArrMap2) {
-    t4 = (
+  if ($[7] !== arr1 || $[8] !== cb2) {
+    t4 = () => arr1.map(cb2);
+    $[7] = arr1;
+    $[8] = cb2;
+    $[9] = t4;
+  } else {
+    t4 = $[9];
+  }
+  const getArrMap2 = t4;
+  let t5;
+  if ($[10] !== getArrMap1 || $[11] !== getArrMap2) {
+    t5 = (
       <Stringify
         getArrMap1={getArrMap1}
         getArrMap2={getArrMap2}
         shouldInvokeFns={true}
       />
     );
-    $[11] = getArrMap1;
-    $[12] = getArrMap2;
-    $[13] = t4;
+    $[10] = getArrMap1;
+    $[11] = getArrMap2;
+    $[12] = t5;
   } else {
-    t4 = $[13];
+    t5 = $[12];
   }
-  return t4;
+  return t5;
 }
 
 export const FIXTURE_ENTRYPOINT = {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.invalid-referencing-frozen-hoisted-storecontext-const.expect.md

@@ -0,0 +1,45 @@
+
+## Input
+
+```javascript
+//@flow @validatePreserveExistingMemoizationGuarantees @enableNewMutationAliasingModel
+
+import {useCallback} from 'react';
+import {useIdentity} from 'shared-runtime';
+
+function Component({content, refetch}) {
+  // This callback function accesses a hoisted const as a dependency,
+  // but it cannot reference it as a dependency since that would be a
+  // TDZ violation!
+  const onRefetch = useCallback(() => {
+    refetch(data);
+  }, [refetch]);
+
+  // The context variable gets frozen here since it's passed to a hook
+  const onSubmit = useIdentity(onRefetch);
+
+  // This has to error: onRefetch needs to memoize with `content` as a
+  // dependency, but the dependency comes later
+  const {data = null} = content;
+
+  return <Foo data={data} onSubmit={onSubmit} />;
+}
+
+```
+
+
+## Error
+
+```
+   9 |   // TDZ violation!
+  10 |   const onRefetch = useCallback(() => {
+> 11 |     refetch(data);
+     |             ^^^^ InvalidReact: This variable is accessed before it is declared, which may prevent it from updating as the assigned value changes over time. Variable `data` is accessed before it is declared (11:11)
+
+InvalidReact: This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time. Variable `data` is accessed before it is declared (19:19)
+  12 |   }, [refetch]);
+  13 |
+  14 |   // The context variable gets frozen here since it's passed to a hook
+```
+          
+      

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.invalid-referencing-frozen-hoisted-storecontext-const.js

@@ -0,0 +1,22 @@
+//@flow @validatePreserveExistingMemoizationGuarantees @enableNewMutationAliasingModel
+
+import {useCallback} from 'react';
+import {useIdentity} from 'shared-runtime';
+
+function Component({content, refetch}) {
+  // This callback function accesses a hoisted const as a dependency,
+  // but it cannot reference it as a dependency since that would be a
+  // TDZ violation!
+  const onRefetch = useCallback(() => {
+    refetch(data);
+  }, [refetch]);
+
+  // The context variable gets frozen here since it's passed to a hook
+  const onSubmit = useIdentity(onRefetch);
+
+  // This has to error: onRefetch needs to memoize with `content` as a
+  // dependency, but the dependency comes later
+  const {data = null} = content;
+
+  return <Foo data={data} onSubmit={onSubmit} />;
+}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/error.mutate-frozen-value.expect.md

@@ -19,7 +19,7 @@ function Component({a, b}) {
   3 |   const x = {a};
   4 |   useFreeze(x);
 > 5 |   x.y = true;
-    |   ^ InvalidReact: This mutates a variable that React considers immutable (5:5)
+    |   ^ InvalidReact: Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook (5:5)
   6 |   return <div>error</div>;
   7 | }
   8 |

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/repro-internal-compiler-shared-mutablerange-bug.expect.md

@@ -0,0 +1,80 @@
+
+## Input
+
+```javascript
+//@flow @validatePreserveExistingMemoizationGuarantees @enableNewMutationAliasingModel
+component Component(
+  onAsyncSubmit?: (() => void) => void,
+  onClose: (isConfirmed: boolean) => void
+) {
+  // When running inferReactiveScopeVariables,
+  // onAsyncSubmit and onClose update to share
+  // a mutableRange instance.
+  const onSubmit = useCallback(() => {
+    if (onAsyncSubmit) {
+      onAsyncSubmit(() => {
+        onClose(true);
+      });
+      return;
+    }
+  }, [onAsyncSubmit, onClose]);
+  // When running inferReactiveScopeVariables here,
+  // first the existing range gets updated (affecting
+  // onAsyncSubmit) and then onClose gets assigned a
+  // different mutable range instance, which is the
+  // one reset after AnalyzeFunctions.
+  // The fix is to fully reset mutable ranges *instances*
+  // after AnalyzeFunctions visit a function expression
+  return <Dialog onSubmit={onSubmit} onClose={() => onClose(false)} />;
+}
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+function Component(t0) {
+  const $ = _c(8);
+  const { onAsyncSubmit, onClose } = t0;
+  let t1;
+  if ($[0] !== onAsyncSubmit || $[1] !== onClose) {
+    t1 = () => {
+      if (onAsyncSubmit) {
+        onAsyncSubmit(() => {
+          onClose(true);
+        });
+        return;
+      }
+    };
+    $[0] = onAsyncSubmit;
+    $[1] = onClose;
+    $[2] = t1;
+  } else {
+    t1 = $[2];
+  }
+  const onSubmit = t1;
+  let t2;
+  if ($[3] !== onClose) {
+    t2 = () => onClose(false);
+    $[3] = onClose;
+    $[4] = t2;
+  } else {
+    t2 = $[4];
+  }
+  let t3;
+  if ($[5] !== onSubmit || $[6] !== t2) {
+    t3 = <Dialog onSubmit={onSubmit} onClose={t2} />;
+    $[5] = onSubmit;
+    $[6] = t2;
+    $[7] = t3;
+  } else {
+    t3 = $[7];
+  }
+  return t3;
+}
+
+```
+      
+### Eval output
+(kind: exception) Fixture not implemented

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/repro-internal-compiler-shared-mutablerange-bug.js

@@ -0,0 +1,25 @@
+//@flow @validatePreserveExistingMemoizationGuarantees @enableNewMutationAliasingModel
+component Component(
+  onAsyncSubmit?: (() => void) => void,
+  onClose: (isConfirmed: boolean) => void
+) {
+  // When running inferReactiveScopeVariables,
+  // onAsyncSubmit and onClose update to share
+  // a mutableRange instance.
+  const onSubmit = useCallback(() => {
+    if (onAsyncSubmit) {
+      onAsyncSubmit(() => {
+        onClose(true);
+      });
+      return;
+    }
+  }, [onAsyncSubmit, onClose]);
+  // When running inferReactiveScopeVariables here,
+  // first the existing range gets updated (affecting
+  // onAsyncSubmit) and then onClose gets assigned a
+  // different mutable range instance, which is the
+  // one reset after AnalyzeFunctions.
+  // The fix is to fully reset mutable ranges *instances*
+  // after AnalyzeFunctions visit a function expression
+  return <Dialog onSubmit={onSubmit} onClose={() => onClose(false)} />;
+}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/retry-no-emit.expect.md

@@ -52,7 +52,7 @@ export const FIXTURE_ENTRYPOINT = {
 ## Logs
 
 ```
-{"kind":"CompileError","fnLoc":{"start":{"line":5,"column":0,"index":195},"end":{"line":13,"column":1,"index":389},"filename":"retry-no-emit.ts"},"detail":{"reason":"This mutates a variable that React considers immutable","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":11,"column":2,"index":352},"end":{"line":11,"column":6,"index":356},"filename":"retry-no-emit.ts","identifierName":"arr2"}}}
+{"kind":"CompileError","fnLoc":{"start":{"line":5,"column":0,"index":195},"end":{"line":13,"column":1,"index":389},"filename":"retry-no-emit.ts"},"detail":{"reason":"Updating a value previously passed as an argument to a hook is not allowed. Consider moving the mutation before calling the hook","description":null,"severity":"InvalidReact","suggestions":null,"loc":{"start":{"line":11,"column":2,"index":352},"end":{"line":11,"column":6,"index":356},"filename":"retry-no-emit.ts","identifierName":"arr2"}}}
 {"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":7,"column":2,"index":248},"end":{"line":7,"column":36,"index":282},"filename":"retry-no-emit.ts"},"decorations":[{"start":{"line":7,"column":31,"index":277},"end":{"line":7,"column":34,"index":280},"filename":"retry-no-emit.ts","identifierName":"arr"}]}
 {"kind":"AutoDepsDecorations","fnLoc":{"start":{"line":10,"column":2,"index":306},"end":{"line":10,"column":44,"index":348},"filename":"retry-no-emit.ts"},"decorations":[{"start":{"line":10,"column":25,"index":329},"end":{"line":10,"column":29,"index":333},"filename":"retry-no-emit.ts","identifierName":"arr2"},{"start":{"line":10,"column":25,"index":329},"end":{"line":10,"column":29,"index":333},"filename":"retry-no-emit.ts","identifierName":"arr2"},{"start":{"line":10,"column":35,"index":339},"end":{"line":10,"column":42,"index":346},"filename":"retry-no-emit.ts","identifierName":"propVal"}]}
 {"kind":"CompileSuccess","fnLoc":{"start":{"line":5,"column":0,"index":195},"end":{"line":13,"column":1,"index":389},"filename":"retry-no-emit.ts"},"fnName":"Foo","memoSlots":0,"memoBlocks":0,"memoValues":0,"prunedMemoBlocks":0,"prunedMemoValues":0}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/shared-hook-calls.expect.md

@@ -30,60 +30,47 @@ import { c as _c, useFire } from "react/compiler-runtime"; // @enableFire @enabl
 import { fire } from "react";
 
 function Component(t0) {
-  const $ = _c(13);
-  let bar;
-  let baz;
-  let foo;
-  if ($[0] !== t0) {
-    ({ bar, baz } = t0);
-    let t1;
-    if ($[4] !== bar) {
-      t1 = () => {
-        console.log(bar);
-      };
-      $[4] = bar;
-      $[5] = t1;
-    } else {
-      t1 = $[5];
-    }
-    foo = t1;
-    $[0] = t0;
-    $[1] = bar;
-    $[2] = baz;
-    $[3] = foo;
+  const $ = _c(9);
+  const { bar, baz } = t0;
+  let t1;
+  if ($[0] !== bar) {
+    t1 = () => {
+      console.log(bar);
+    };
+    $[0] = bar;
+    $[1] = t1;
   } else {
-    bar = $[1];
-    baz = $[2];
-    foo = $[3];
+    t1 = $[1];
   }
-  const t1 = useFire(foo);
-  const t2 = useFire(baz);
-  let t3;
-  if ($[6] !== bar || $[7] !== t1 || $[8] !== t2) {
-    t3 = () => {
-      t1(bar);
+  const foo = t1;
+  const t2 = useFire(foo);
+  const t3 = useFire(baz);
+  let t4;
+  if ($[2] !== bar || $[3] !== t2 || $[4] !== t3) {
+    t4 = () => {
+      t2(bar);
+      t3(bar);
+    };
+    $[2] = bar;
+    $[3] = t2;
+    $[4] = t3;
+    $[5] = t4;
+  } else {
+    t4 = $[5];
+  }
+  useEffect(t4);
+  let t5;
+  if ($[6] !== bar || $[7] !== t2) {
+    t5 = () => {
       t2(bar);
     };
     $[6] = bar;
-    $[7] = t1;
-    $[8] = t2;
-    $[9] = t3;
+    $[7] = t2;
+    $[8] = t5;
   } else {
-    t3 = $[9];
+    t5 = $[8];
   }
-  useEffect(t3);
-  let t4;
-  if ($[10] !== bar || $[11] !== t1) {
-    t4 = () => {
-      t1(bar);
-    };
-    $[10] = bar;
-    $[11] = t1;
-    $[12] = t4;
-  } else {
-    t4 = $[12];
-  }
-  useEffect(t4);
+  useEffect(t5);
   return null;
 }
 

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/transform-fire/shared-hook-calls.expect.md

@@ -30,60 +30,47 @@ import { c as _c, useFire } from "react/compiler-runtime"; // @enableFire
 import { fire } from "react";
 
 function Component(t0) {
-  const $ = _c(13);
-  let bar;
-  let baz;
-  let foo;
-  if ($[0] !== t0) {
-    ({ bar, baz } = t0);
-    let t1;
-    if ($[4] !== bar) {
-      t1 = () => {
-        console.log(bar);
-      };
-      $[4] = bar;
-      $[5] = t1;
-    } else {
-      t1 = $[5];
-    }
-    foo = t1;
-    $[0] = t0;
-    $[1] = bar;
-    $[2] = baz;
-    $[3] = foo;
+  const $ = _c(9);
+  const { bar, baz } = t0;
+  let t1;
+  if ($[0] !== bar) {
+    t1 = () => {
+      console.log(bar);
+    };
+    $[0] = bar;
+    $[1] = t1;
   } else {
-    bar = $[1];
-    baz = $[2];
-    foo = $[3];
+    t1 = $[1];
   }
-  const t1 = useFire(foo);
-  const t2 = useFire(baz);
-  let t3;
-  if ($[6] !== bar || $[7] !== t1 || $[8] !== t2) {
-    t3 = () => {
-      t1(bar);
+  const foo = t1;
+  const t2 = useFire(foo);
+  const t3 = useFire(baz);
+  let t4;
+  if ($[2] !== bar || $[3] !== t2 || $[4] !== t3) {
+    t4 = () => {
+      t2(bar);
+      t3(bar);
+    };
+    $[2] = bar;
+    $[3] = t2;
+    $[4] = t3;
+    $[5] = t4;
+  } else {
+    t4 = $[5];
+  }
+  useEffect(t4);
+  let t5;
+  if ($[6] !== bar || $[7] !== t2) {
+    t5 = () => {
       t2(bar);
     };
     $[6] = bar;
-    $[7] = t1;
-    $[8] = t2;
-    $[9] = t3;
+    $[7] = t2;
+    $[8] = t5;
   } else {
-    t3 = $[9];
+    t5 = $[8];
   }
-  useEffect(t3);
-  let t4;
-  if ($[10] !== bar || $[11] !== t1) {
-    t4 = () => {
-      t1(bar);
-    };
-    $[10] = bar;
-    $[11] = t1;
-    $[12] = t4;
-  } else {
-    t4 = $[12];
-  }
-  useEffect(t4);
+  useEffect(t5);
   return null;
 }