[compiler] FunctionExpression context locations point to first reference

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

@@ -72,21 +72,21 @@ 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,
     context: capturedRefs,
   });
   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/Environment.ts

@@ -246,7 +246,7 @@ export const EnvironmentConfigSchema = z.object({
   /**
    * Enable a new model for mutability and aliasing inference
    */
-  enableNewMutationAliasingModel: z.boolean().default(false),
+  enableNewMutationAliasingModel: z.boolean().default(true),
 
   /**
    * Enables inference of optional dependency chains. Without this flag

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/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/InferMutationAliasingEffects.ts

@@ -901,23 +901,44 @@ function applyEffect(
           console.log(prettyFormat(state.debugAbstractValue(value)));
         }
 
-        const reason = getWriteErrorReason({
-          kind: value.kind,
-          reason: value.reason,
-          context: new Set(),
-        });
+        let reason: string;
+        let description: string | null = null;
+
+        if (
+          mutationKind === 'mutate-frozen' &&
+          context.hoistedContextDeclarations.has(
+            effect.value.identifier.declarationId,
+          )
+        ) {
+          reason = `This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time`;
+          if (
+            effect.value.identifier.name !== null &&
+            effect.value.identifier.name.kind === 'named'
+          ) {
+            description = `Move the declaration of \`${effect.value.identifier.name.value}\` to before it is first referenced`;
+          }
+        } else {
+          reason = getWriteErrorReason({
+            kind: value.kind,
+            reason: value.reason,
+            context: new Set(),
+          });
+          if (
+            effect.value.identifier.name !== null &&
+            effect.value.identifier.name.kind === 'named'
+          ) {
+            description = `Found mutation of \`${effect.value.identifier.name.value}\``;
+          }
+        }
+
         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,
+            description,
             loc: effect.value.loc,
             suggestions: null,
           },

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-nested-scope-truncated-dep.expect.md

@@ -175,21 +175,14 @@ import {
  * and mutability.
  */
 function Component(t0) {
-  const $ = _c(4);
+  const $ = _c(2);
   const { prop } = t0;
   let t1;
   if ($[0] !== prop) {
     const obj = shallowCopy(prop);
     const aliasedObj = identity(obj);
-    let t2;
-    if ($[2] !== obj) {
-      t2 = [obj.id];
-      $[2] = obj;
-      $[3] = t2;
-    } else {
-      t2 = $[3];
-    }
-    const id = t2;
+
+    const id = [obj.id];
 
     mutate(aliasedObj);
     setPropertyByKey(aliasedObj, "id", prop.id + 1);

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-function-alias-computed-load-2-iife.expect.md

@@ -25,17 +25,25 @@ export const FIXTURE_ENTRYPOINT = {
 ```javascript
 import { c as _c } from "react/compiler-runtime";
 function bar(a) {
-  const $ = _c(2);
-  let y;
+  const $ = _c(4);
+  let t0;
   if ($[0] !== a) {
-    const x = [a];
+    t0 = [a];
+    $[0] = a;
+    $[1] = t0;
+  } else {
+    t0 = $[1];
+  }
+  const x = t0;
+  let y;
+  if ($[2] !== x[0][1]) {
     y = {};
 
     y = x[0][1];
-    $[0] = a;
-    $[1] = y;
+    $[2] = x[0][1];
+    $[3] = y;
   } else {
-    y = $[1];
+    y = $[3];
   }
   return y;
 }

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-function-alias-computed-load-3-iife.expect.md

@@ -29,20 +29,29 @@ export const FIXTURE_ENTRYPOINT = {
 ```javascript
 import { c as _c } from "react/compiler-runtime";
 function bar(a, b) {
-  const $ = _c(3);
-  let y;
+  const $ = _c(6);
+  let t0;
   if ($[0] !== a || $[1] !== b) {
-    const x = [a, b];
+    t0 = [a, b];
+    $[0] = a;
+    $[1] = b;
+    $[2] = t0;
+  } else {
+    t0 = $[2];
+  }
+  const x = t0;
+  let y;
+  if ($[3] !== x[0][1] || $[4] !== x[1][0]) {
     y = {};
     let t = {};
 
     y = x[0][1];
     t = x[1][0];
-    $[0] = a;
-    $[1] = b;
-    $[2] = y;
+    $[3] = x[0][1];
+    $[4] = x[1][0];
+    $[5] = y;
   } else {
-    y = $[2];
+    y = $[5];
   }
   return y;
 }

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-function-alias-computed-load-4-iife.expect.md

@@ -25,17 +25,25 @@ export const FIXTURE_ENTRYPOINT = {
 ```javascript
 import { c as _c } from "react/compiler-runtime";
 function bar(a) {
-  const $ = _c(2);
-  let y;
+  const $ = _c(4);
+  let t0;
   if ($[0] !== a) {
-    const x = [a];
+    t0 = [a];
+    $[0] = a;
+    $[1] = t0;
+  } else {
+    t0 = $[1];
+  }
+  const x = t0;
+  let y;
+  if ($[2] !== x[0].a[1]) {
     y = {};
 
     y = x[0].a[1];
-    $[0] = a;
-    $[1] = y;
+    $[2] = x[0].a[1];
+    $[3] = y;
   } else {
-    y = $[1];
+    y = $[3];
   }
   return y;
 }

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/capturing-function-alias-computed-load-iife.expect.md

@@ -24,17 +24,25 @@ export const FIXTURE_ENTRYPOINT = {
 ```javascript
 import { c as _c } from "react/compiler-runtime";
 function bar(a) {
-  const $ = _c(2);
-  let y;
+  const $ = _c(4);
+  let t0;
   if ($[0] !== a) {
-    const x = [a];
+    t0 = [a];
+    $[0] = a;
+    $[1] = t0;
+  } else {
+    t0 = $[1];
+  }
+  const x = t0;
+  let y;
+  if ($[2] !== x[0]) {
     y = {};
 
     y = x[0];
-    $[0] = a;
-    $[1] = y;
+    $[2] = x[0];
+    $[3] = y;
   } else {
-    y = $[1];
+    y = $[3];
   }
   return y;
 }

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

@@ -41,7 +41,7 @@ export const FIXTURE_ENTRYPOINT = {
   19 |   useEffect(() => setState(2), []);
   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)
+     |                 ^^^^^^^^ InvalidReact: This variable is accessed before it is declared, which prevents the earlier access from updating when this value changes over time. Move the declaration of `setState` to before it is first referenced (21:21)
   22 |   return <Stringify state={state} />;
   23 | }
   24 |

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-impure-functions-in-render.expect.md

@@ -20,7 +20,7 @@ function Component() {
   2 |
   3 | function Component() {
 > 4 |   const date = Date.now();
-    |                ^^^^^^^^ InvalidReact: Calling an impure function can produce unstable results. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent). `Date.now` is an impure function whose results may change on every call (4:4)
+    |                ^^^^^^^^^^ InvalidReact: Calling an impure function can produce unstable results. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent). `Date.now` is an impure function whose results may change on every call (4:4)
 
 InvalidReact: Calling an impure function can produce unstable results. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent). `performance.now` is an impure function whose results may change on every call (5:5)
 

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: Mutating a value returned from a function whose return value should not be mutated. Found mutation of `sharedVal` (11:11)
+     |                       ^^^^^^^^^ InvalidReact: This mutates a variable that React considers immutable. Found mutation of `sharedVal` (11:11)
   12 |       title="Randomize"
   13 |     />
   14 |   );