compiler: treat pruned scope outputs as reactive

Mostly addresses the issue with non-reactive pruned scopes. Before, values from pruned scopes would not be memoized, but could still be depended upon by downstream scopes. However, those downstream scopes would assume the value could never change. This could allow the developer to observe two different versions of a value - the freshly created one (if observed outside a scope) or a cached one (if observed inside, or through) a scope which used the value but didn't depend on it.

The fix here is to consider the outputs of pruned reactive scopes as reactive. Note that this is a partial fix because of things like control variables — the full solution would be to mark these values as reactive, and then re-run InferReactivePlaces. We can do this once we've fully converted our pipeline to use HIR everywhere. For now, this should fix most issues in practice because PruneNonReactiveDependencies already does basic alias tracking (see new fixture).

ghstack-source-id: 11f48618d0413075c43ee88529e00e8e523e61f7
Pull Request resolved: #29790

Author: josephsavona

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CollectReactiveIdentifiers.ts

@@ -9,7 +9,9 @@ import {
   IdentifierId,
   InstructionId,
   Place,
+  PrunedReactiveScopeBlock,
   ReactiveFunction,
+  isPrimitiveType,
 } from "../HIR/HIR";
 import { ReactiveFunctionVisitor, visitReactiveFunction } from "./visitors";
 
@@ -40,6 +42,19 @@ class Visitor extends ReactiveFunctionVisitor<Set<IdentifierId>> {
       state.add(place.identifier.id);
     }
   }
+
+  override visitPrunedScope(
+    scopeBlock: PrunedReactiveScopeBlock,
+    state: Set<IdentifierId>
+  ): void {
+    this.traversePrunedScope(scopeBlock, state);
+
+    for (const [id, decl] of scopeBlock.scope.declarations) {
+      if (!isPrimitiveType(decl.identifier)) {
+        state.add(id);
+      }
+    }
+  }
 }
 
 /*

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-pruned-scope-leaks-value.expect.md

@@ -39,7 +39,7 @@ export const FIXTURE_ENTRYPOINT = {
 ```javascript
 import { c as _c } from "react/compiler-runtime";
 function Component(props) {
-  const $ = _c(1);
+  const $ = _c(2);
 
   const a = [];
   const b = [];
@@ -53,11 +53,12 @@ function Component(props) {
     x = 1;
   }
   let t0;
-  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+  if ($[0] !== x) {
     t0 = [x];
-    $[0] = t0;
+    $[0] = x;
+    $[1] = t0;
   } else {
-    t0 = $[0];
+    t0 = $[1];
   }
   return t0;
 }

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-pruned-scope-leaks-value.ts

@@ -39,7 +39,7 @@ import { useEffect, useState } from "react";
 import { mutate } from "shared-runtime";
 
 function Component(props) {
-  const $ = _c(6);
+  const $ = _c(8);
   const x = [{ ...props.value }];
   let t0;
   let t1;
@@ -64,25 +64,27 @@ function Component(props) {
     return <span key={item.id}>{item.text}</span>;
   });
   let t3;
-  if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
+  if ($[2] !== y) {
     t3 = mutate(y);
-    $[2] = t3;
+    $[2] = y;
+    $[3] = t3;
   } else {
-    t3 = $[2];
+    t3 = $[3];
   }
   let t4;
-  if ($[3] !== onClick || $[4] !== t2) {
+  if ($[4] !== onClick || $[5] !== t2 || $[6] !== t3) {
     t4 = (
       <div onClick={onClick}>
         {t2}
         {t3}
       </div>
     );
-    $[3] = onClick;
-    $[4] = t2;
-    $[5] = t4;
+    $[4] = onClick;
+    $[5] = t2;
+    $[6] = t3;
+    $[7] = t4;
   } else {
-    t4 = $[5];
+    t4 = $[7];
   }
   return t4;
 }

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/reactive-control-dependency-from-interleaved-reactivity-for-of.expect.md

@@ -0,0 +1,112 @@
+
+## Input
+
+```javascript
+import invariant from "invariant";
+import {
+  makeObject_Primitives,
+  mutate,
+  sum,
+  useIdentity,
+} from "shared-runtime";
+
+/**
+ * Here, `z`'s original memo block is removed due to the inner hook call.
+ * However, we also infer that `z` is non-reactive, so by default we would create
+ * the memo block for `thing = [y, z]` as only depending on `y`.
+ *
+ * This could then mean that `thing[1]` and `z` may not refer to the same value,
+ * since z recreates every time but `thing` doesn't correspondingly invalidate.
+ *
+ * The fix is to consider pruned memo block outputs as reactive, since they will
+ * recreate on every render. This means `thing` depends on both y and z.
+ */
+function MyApp({ count }) {
+  const z = makeObject_Primitives();
+  const x = useIdentity(2);
+  const y = sum(x, count);
+  mutate(z);
+  const z2 = z;
+  const thing = [y, z2];
+  if (thing[1] !== z) {
+    invariant(false, "oh no!");
+  }
+  return thing;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: MyApp,
+  params: [{ count: 2 }],
+  sequentialRenders: [{ count: 2 }, { count: 2 }, { count: 3 }],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+import invariant from "invariant";
+import {
+  makeObject_Primitives,
+  mutate,
+  sum,
+  useIdentity,
+} from "shared-runtime";
+
+/**
+ * Here, `z`'s original memo block is removed due to the inner hook call.
+ * However, we also infer that `z` is non-reactive, so by default we would create
+ * the memo block for `thing = [y, z]` as only depending on `y`.
+ *
+ * This could then mean that `thing[1]` and `z` may not refer to the same value,
+ * since z recreates every time but `thing` doesn't correspondingly invalidate.
+ *
+ * The fix is to consider pruned memo block outputs as reactive, since they will
+ * recreate on every render. This means `thing` depends on both y and z.
+ */
+function MyApp(t0) {
+  const $ = _c(6);
+  const { count } = t0;
+  const z = makeObject_Primitives();
+  const x = useIdentity(2);
+  let t1;
+  if ($[0] !== x || $[1] !== count) {
+    t1 = sum(x, count);
+    $[0] = x;
+    $[1] = count;
+    $[2] = t1;
+  } else {
+    t1 = $[2];
+  }
+  const y = t1;
+  mutate(z);
+  const z2 = z;
+  let t2;
+  if ($[3] !== y || $[4] !== z2) {
+    t2 = [y, z2];
+    $[3] = y;
+    $[4] = z2;
+    $[5] = t2;
+  } else {
+    t2 = $[5];
+  }
+  const thing = t2;
+  if (thing[1] !== z) {
+    invariant(false, "oh no!");
+  }
+  return thing;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: MyApp,
+  params: [{ count: 2 }],
+  sequentialRenders: [{ count: 2 }, { count: 2 }, { count: 3 }],
+};
+
+```
+      
+### Eval output
+(kind: ok) [4,{"a":0,"b":"value1","c":true,"wat0":"joe"}]
+[4,{"a":0,"b":"value1","c":true,"wat0":"joe"}]
+[5,{"a":0,"b":"value1","c":true,"wat0":"joe"}]