From f18a01bf59be6107daa817432f20b9e912e84d35 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Thu, 21 Mar 2024 17:12:09 -0700
Subject: [PATCH] Add todo for destructuring to context variables

We don't have `DestructureContext` yet, this adds a todo for the cases where we
should use one.
---
 .../ValidateContextVariableLValues.ts         | 44 +++++++++++++------
 .../error.todo-reassign-const.expect.md       |  8 ++--
 2 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/compiler/packages/babel-plugin-react-forget/src/Validation/ValidateContextVariableLValues.ts b/compiler/packages/babel-plugin-react-forget/src/Validation/ValidateContextVariableLValues.ts
index 4af3b2612a..6b8ca1f774 100644
--- a/compiler/packages/babel-plugin-react-forget/src/Validation/ValidateContextVariableLValues.ts
+++ b/compiler/packages/babel-plugin-react-forget/src/Validation/ValidateContextVariableLValues.ts
@@ -56,7 +56,7 @@ function validateContextVariableLValuesImpl(
         }
         case "Destructure": {
           for (const lvalue of eachPatternOperand(value.lvalue.pattern)) {
-            visit(identifierKinds, lvalue, "local");
+            visit(identifierKinds, lvalue, "destructure");
           }
           break;
         }
@@ -84,23 +84,39 @@ function validateContextVariableLValuesImpl(
   }
 }
 
-type IdentifierKinds = Map<IdentifierId, "local" | "context">;
+type IdentifierKinds = Map<
+  IdentifierId,
+  { place: Place; kind: "local" | "context" | "destructure" }
+>;
 
 function visit(
   identifiers: IdentifierKinds,
   place: Place,
-  kind: "local" | "context"
+  kind: "local" | "context" | "destructure"
 ): void {
-  const prevKind = identifiers.get(place.identifier.id);
-  if (prevKind !== undefined && prevKind !== kind) {
-    CompilerError.invariant(false, {
-      reason: `Expected all references to a variable to be consistently local or context references`,
-      loc: place.loc,
-      description: `Identifier ${printPlace(
-        place
-      )} is referenced as a ${kind} variable, but was previously referenced as a ${prevKind} variable`,
-      suggestions: null,
-    });
+  const prev = identifiers.get(place.identifier.id);
+  if (prev !== undefined) {
+    const wasContext = prev.kind === "context";
+    const isContext = kind === "context";
+    if (wasContext !== isContext) {
+      if (prev.kind === "destructure" || kind === "destructure") {
+        CompilerError.throwTodo({
+          reason: `Support destructuring of context variables`,
+          loc: kind === "destructure" ? place.loc : prev.place.loc,
+          description: null,
+          suggestions: null,
+        });
+      }
+
+      CompilerError.invariant(false, {
+        reason: `Expected all references to a variable to be consistently local or context references`,
+        loc: place.loc,
+        description: `Identifier ${printPlace(
+          place
+        )} is referenced as a ${kind} variable, but was previously referenced as a ${prev} variable`,
+        suggestions: null,
+      });
+    }
   }
-  identifiers.set(place.identifier.id, kind);
+  identifiers.set(place.identifier.id, { place, kind });
 }
diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-reassign-const.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-reassign-const.expect.md
index 14a29be062..49a0655ac8 100644
--- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-reassign-const.expect.md
+++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-reassign-const.expect.md
@@ -21,13 +21,13 @@ function Component({ foo }) {
 ## Error
 
 ```
+  1 | import { Stringify } from "shared-runtime";
   2 |
-  3 | function Component({ foo }) {
-> 4 |   let bar = foo.bar;
-    |             ^^^ [ReactForget] Invariant: Expected all references to a variable to be consistently local or context references. Identifier <unknown> foo$1 is referenced as a context variable, but was previously referenced as a local variable (4:4)
+> 3 | function Component({ foo }) {
+    |                      ^^^ [ReactForget] Todo: Support destructuring of context variables (3:3)
+  4 |   let bar = foo.bar;
   5 |   return (
   6 |     <Stringify
-  7 |       handler={() => {
 ```
           
       
\ No newline at end of file