From 490c204dcf8b648994977e87cd3cc8c995ac8a32 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Tue, 14 Feb 2023 15:27:52 -0800
Subject: [PATCH] Move logic for making reactive scope decls all reactive

InferReactiveIdentifiers has some extra logic to find identifiers declared in
the same scope, and promote non-reactive identifiers to reactive if they appear
inside a reactive scope (reactive scope == scope with one or more (reactive)
dependencies). Even though the identifier alone might not be technically
reactive (have no reactive inputs), it can get re-recreated if the scope
re-evaluates.

We can now do this during PruneNonReactiveDependencies as we exit out of each
scope.
---
 .../src/ReactiveScopes/InferReactiveIdentifiers.ts | 14 --------------
 .../ReactiveScopes/PruneNonReactiveDependencies.ts |  7 ++++++-
 2 files changed, 6 insertions(+), 15 deletions(-)

diff --git a/compiler/forget/src/ReactiveScopes/InferReactiveIdentifiers.ts b/compiler/forget/src/ReactiveScopes/InferReactiveIdentifiers.ts
index f39cde96e9..5ee700aa6a 100644
--- a/compiler/forget/src/ReactiveScopes/InferReactiveIdentifiers.ts
+++ b/compiler/forget/src/ReactiveScopes/InferReactiveIdentifiers.ts
@@ -10,7 +10,6 @@ import {
   Identifier,
   ReactiveFunction,
   ReactiveInstruction,
-  ReactiveScope,
 } from "../HIR/HIR";
 import { parseHookCall } from "../Inference/InferReferenceEffects";
 import {
@@ -118,21 +117,8 @@ export function inferReactiveIdentifiers(
   for (const param of fn.params) {
     reactivityMap.set(param.identifier, true);
   }
-  const actuallyReactiveScopes = new Set<ReactiveScope>();
-
   visitReactiveFunction(fn, visitor, reactivityMap);
 
-  for (const [id, value] of reactivityMap) {
-    const { scope } = id;
-    if (value && scope != null) {
-      actuallyReactiveScopes.add(scope);
-    }
-  }
-  for (const [id, _] of reactivityMap) {
-    if (id.scope && actuallyReactiveScopes.has(id.scope)) {
-      reactivityMap.set(id, true);
-    }
-  }
   const result = new Set<Identifier>();
   reactivityMap.forEach((isReactive, id) => {
     if (isReactive) result.add(id);
diff --git a/compiler/forget/src/ReactiveScopes/PruneNonReactiveDependencies.ts b/compiler/forget/src/ReactiveScopes/PruneNonReactiveDependencies.ts
index c3c1449b10..8adaf70698 100644
--- a/compiler/forget/src/ReactiveScopes/PruneNonReactiveDependencies.ts
+++ b/compiler/forget/src/ReactiveScopes/PruneNonReactiveDependencies.ts
@@ -31,11 +31,16 @@ class Visitor extends ReactiveFunctionVisitor<State> {
         scope.scope.dependencies.delete(dep);
       }
     }
-    // If a scope now has no dependencies, then its declarations are all non-reactive
     if (scope.scope.dependencies.size === 0) {
+      // If a scope has no dependencies, then its declarations are all non-reactive
       for (const [, declaration] of scope.scope.declarations) {
         state.delete(declaration);
       }
+    } else {
+      // otherwise, all the scope's declarations are reactive
+      for (const [, declaration] of scope.scope.declarations) {
+        state.add(declaration);
+      }
     }
   }
 }