From 130b809ac2006ebfb381244bfa4bf3828304ea67 Mon Sep 17 00:00:00 2001
From: Sathya Gunasekaran <gsathya.ceg@gmail.com>
Date: Wed, 28 Feb 2024 16:19:43 -0800
Subject: [PATCH] Allow property loads from hook

Looking up certain properties on a hook is a common pattern for logging.

It's non-ideal but it's not a bug to do this.

This updates Forget to not error on this pattern.
---
 .../src/Validation/ValidateHooksUsage.ts      |  3 +-
 ...or.hook-property-load-local-hook.expect.md | 37 +++++++++++++++++
 .../error.hook-property-load-local-hook.js    | 14 +++++++
 .../hook-property-load-local.expect.md        | 40 +++++++++++++++++++
 .../compiler/hook-property-load-local.js      | 12 ++++++
 5 files changed, 104 insertions(+), 2 deletions(-)
 create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.hook-property-load-local-hook.expect.md
 create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.hook-property-load-local-hook.js
 create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/hook-property-load-local.expect.md
 create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/hook-property-load-local.js

diff --git a/compiler/packages/babel-plugin-react-forget/src/Validation/ValidateHooksUsage.ts b/compiler/packages/babel-plugin-react-forget/src/Validation/ValidateHooksUsage.ts
index dd42e09457..85ebbda978 100644
--- a/compiler/packages/babel-plugin-react-forget/src/Validation/ValidateHooksUsage.ts
+++ b/compiler/packages/babel-plugin-react-forget/src/Validation/ValidateHooksUsage.ts
@@ -233,7 +233,6 @@ export function validateHooksUsage(fn: HIRFunction): void {
           break;
         }
         case "PropertyLoad": {
-          visitPlace(instr.value.object);
           const objectKind = getKindForPlace(instr.value.object);
           const isHookProperty = isHookName(instr.value.property);
           let kind: Kind;
@@ -249,7 +248,7 @@ export function validateHooksUsage(fn: HIRFunction): void {
                *   let x = useFoo.useBar; // useFoo is KnownHook, any property from it inherits KnownHook
                * }
                */
-              kind = Kind.KnownHook;
+              kind = isHookProperty ? Kind.KnownHook : Kind.Local;
               break;
             }
             case Kind.PotentialHook: {
diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.hook-property-load-local-hook.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.hook-property-load-local-hook.expect.md
new file mode 100644
index 0000000000..ae88f597b1
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.hook-property-load-local-hook.expect.md
@@ -0,0 +1,37 @@
+
+## Input
+
+```javascript
+function useFoo() {}
+useFoo.useBar = function () {
+  return "foo";
+};
+
+function Foo() {
+  let bar = useFoo.useBar;
+  return bar();
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Foo,
+  params: [],
+};
+
+```
+
+
+## Error
+
+```
+   5 |
+   6 | function Foo() {
+>  7 |   let bar = useFoo.useBar;
+     |             ^^^^^^^^^^^^^ [ReactForget] InvalidReact: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (7:7)
+
+[ReactForget] InvalidReact: Hooks may not be referenced as normal values, they must be called. See the Rules of Hooks (https://react.dev/warnings/invalid-hook-call-warning) (8:8)
+   8 |   return bar();
+   9 | }
+  10 |
+```
+          
+      
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.hook-property-load-local-hook.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.hook-property-load-local-hook.js
new file mode 100644
index 0000000000..088bef3d4a
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.hook-property-load-local-hook.js
@@ -0,0 +1,14 @@
+function useFoo() {}
+useFoo.useBar = function () {
+  return "foo";
+};
+
+function Foo() {
+  let bar = useFoo.useBar;
+  return bar();
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Foo,
+  params: [],
+};
diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/hook-property-load-local.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/hook-property-load-local.expect.md
new file mode 100644
index 0000000000..de49c0c22a
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/hook-property-load-local.expect.md
@@ -0,0 +1,40 @@
+
+## Input
+
+```javascript
+function useFoo() {}
+
+function Foo() {
+  let name = useFoo.name;
+  console.log(name);
+  return name;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Foo,
+  params: [],
+};
+
+```
+
+## Code
+
+```javascript
+function useFoo() {}
+
+function Foo() {
+  const name = useFoo.name;
+  console.log(name);
+  return name;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Foo,
+  params: [],
+};
+
+```
+      
+### Eval output
+(kind: ok) "useFoo"
+logs: ['useFoo']
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/hook-property-load-local.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/hook-property-load-local.js
new file mode 100644
index 0000000000..ec3de1516d
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/hook-property-load-local.js
@@ -0,0 +1,12 @@
+function useFoo() {}
+
+function Foo() {
+  let name = useFoo.name;
+  console.log(name);
+  return name;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Foo,
+  params: [],
+};