[compiler] Remove redundant InferMutableContextVariables

This removes special casing for `PropertyStore` mutability inference within FunctionExpressions.

Author: mofeiZ

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

@@ -18,7 +18,6 @@ import {
 import {deadCodeElimination} from '../Optimization';
 import {inferReactiveScopeVariables} from '../ReactiveScopes';
 import {rewriteInstructionKindsBasedOnReassignment} from '../SSA';
-import {inferMutableContextVariables} from './InferMutableContextVariables';
 import {inferMutableRanges} from './InferMutableRanges';
 import inferReferenceEffects from './InferReferenceEffects';
 
@@ -78,7 +77,6 @@ function lower(func: HIRFunction): void {
 }
 
 function infer(loweredFunc: LoweredFunction): void {
-  const knownMutated = inferMutableContextVariables(loweredFunc.func);
   for (const operand of loweredFunc.func.context) {
     const identifier = operand.identifier;
     CompilerError.invariant(operand.effect === Effect.Unknown, {
@@ -95,10 +93,11 @@ function infer(loweredFunc: LoweredFunction): void {
        * render
        */
       operand.effect = Effect.Capture;
-    } else if (knownMutated.has(operand)) {
-      operand.effect = Effect.Mutate;
     } else if (isMutatedOrReassigned(identifier)) {
-      // Note that this also reflects if identifier is ConditionallyMutated
+      /**
+       * Reflects direct reassignments, PropertyStores, and ConditionallyMutate
+       * (directly or through maybe-aliases)
+       */
       operand.effect = Effect.Capture;
     } else {
       operand.effect = Effect.Read;

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

@@ -19,9 +19,14 @@ function Component() {
 
   // capture into a separate variable that is not a context variable.
   const y = x;
+  /**
+   * Note that this fixture currently produces a stale effect closure if `y = x
+   * = someGlobal` changes between renders. Under current compiler assumptions,
+   * that would be a rule of react violation.
+   */
   useEffect(() => {
     y.value = 'hello';
-  }, []);
+  });
 
   useEffect(() => {
     setState(someGlobal.value);
@@ -46,57 +51,50 @@ import { useEffect, useState } from "react";
 let someGlobal = { value: null };
 
 function Component() {
-  const $ = _c(7);
+  const $ = _c(5);
   const [state, setState] = useState(someGlobal);
+
+  let x = someGlobal;
+  while (x == null) {
+    x = someGlobal;
+  }
+
+  const y = x;
   let t0;
-  let t1;
-  let t2;
   if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
-    let x = someGlobal;
-    while (x == null) {
-      x = someGlobal;
-    }
-
-    const y = x;
-    t0 = useEffect;
-    t1 = () => {
+    t0 = () => {
       y.value = "hello";
     };
-    t2 = [];
     $[0] = t0;
+  } else {
+    t0 = $[0];
+  }
+  useEffect(t0);
+  let t1;
+  let t2;
+  if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
+    t1 = () => {
+      setState(someGlobal.value);
+    };
+    t2 = [someGlobal];
     $[1] = t1;
     $[2] = t2;
   } else {
-    t0 = $[0];
     t1 = $[1];
     t2 = $[2];
   }
-  t0(t1, t2);
-  let t3;
+  useEffect(t1, t2);
+
+  const t3 = String(state);
   let t4;
-  if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
-    t3 = () => {
-      setState(someGlobal.value);
-    };
-    t4 = [someGlobal];
+  if ($[3] !== t3) {
+    t4 = <div>{t3}</div>;
     $[3] = t3;
     $[4] = t4;
   } else {
-    t3 = $[3];
     t4 = $[4];
   }
-  useEffect(t3, t4);
-
-  const t5 = String(state);
-  let t6;
-  if ($[5] !== t5) {
-    t6 = <div>{t5}</div>;
-    $[5] = t5;
-    $[6] = t6;
-  } else {
-    t6 = $[6];
-  }
-  return t6;
+  return t4;
 }
 
 export const FIXTURE_ENTRYPOINT = {

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-mutate-global-in-effect-fixpoint.expect.md

@@ -15,9 +15,14 @@ function Component() {
 
   // capture into a separate variable that is not a context variable.
   const y = x;
+  /**
+   * Note that this fixture currently produces a stale effect closure if `y = x
+   * = someGlobal` changes between renders. Under current compiler assumptions,
+   * that would be a rule of react violation.
+   */
   useEffect(() => {
     y.value = 'hello';
-  }, []);
+  });
 
   useEffect(() => {
     setState(someGlobal.value);

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-mutate-global-in-effect-fixpoint.js

@@ -36,21 +36,22 @@ import { useSharedValue } from "react-native-reanimated";
  * of render
  */
 function SomeComponent() {
-  const $ = _c(3);
+  const $ = _c(2);
   const sharedVal = useSharedValue(0);
-
-  const T0 = Button;
-  const t0 = () => (sharedVal.value = Math.random());
-  let t1;
-  if ($[0] !== T0 || $[1] !== t0) {
-    t1 = <T0 onPress={t0} title="Randomize" />;
-    $[0] = T0;
+  let t0;
+  if ($[0] !== sharedVal) {
+    t0 = (
+      <Button
+        onPress={() => (sharedVal.value = Math.random())}
+        title="Randomize"
+      />
+    );
+    $[0] = sharedVal;
     $[1] = t0;
-    $[2] = t1;
   } else {
-    t1 = $[2];
+    t0 = $[1];
   }
-  return t1;
+  return t0;
 }
 
 ```