From 43ef2ffe7858a75ded2ce63c7026bb28326362f0 Mon Sep 17 00:00:00 2001
From: Ramanpreet Nara <ramanpreet@meta.com>
Date: Sat, 11 Mar 2023 16:59:39 -0800
Subject: [PATCH] Refactor: Move HostFunction management logic to
 TurboModule::get()

Summary:
## Rationale
Better separation of concerns. Less confusion. All the logic around TurboModule HostFunction management is in TurboModule::get(). And TurboModule::createHostFunction() is only responsible for creating the HostFunction. Also, there aren't two TurboModule::get functions, each with different/unclear responsibilities.

## Motivation
The interop layer (i.e: D43918998) will have to override TurboModule::createHostFunction():
- TurboModule::createHostFunction() relies on static functions for method dispatch.
- The interop layer cannot use static functions for method dispatch: interop modules' methods are only be discoverable at runtime.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D43918940

fbshipit-source-id: f2ffd210329a10967e9b2f0c925f4990cb470083
---
 .../nativemodule/core/ReactCommon/TurboModule.cpp    | 11 ++---------
 .../nativemodule/core/ReactCommon/TurboModule.h      | 12 ++++++++++--
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/ReactCommon/react/nativemodule/core/ReactCommon/TurboModule.cpp b/ReactCommon/react/nativemodule/core/ReactCommon/TurboModule.cpp
index 0eb26e7b625..75b7c5d4696 100644
--- a/ReactCommon/react/nativemodule/core/ReactCommon/TurboModule.cpp
+++ b/ReactCommon/react/nativemodule/core/ReactCommon/TurboModule.cpp
@@ -15,11 +15,11 @@ TurboModule::TurboModule(
     std::shared_ptr<CallInvoker> jsInvoker)
     : name_(std::move(name)), jsInvoker_(std::move(jsInvoker)) {}
 
-jsi::Value TurboModule::get(
+jsi::Value TurboModule::createHostFunction(
     jsi::Runtime &runtime,
     const jsi::PropNameID &propName,
     const MethodMetadata &meta) {
-  auto result = jsi::Function::createFromHostFunction(
+  return jsi::Function::createFromHostFunction(
       runtime,
       propName,
       static_cast<unsigned int>(meta.argCount),
@@ -28,13 +28,6 @@ jsi::Value TurboModule::get(
           const jsi::Value &thisVal,
           const jsi::Value *args,
           size_t count) { return meta.invoker(rt, *this, args, count); });
-  // If we have a JS wrapper, cache the result of this lookup
-  // We don't cache misses, to allow for methodMap_ to dynamically be extended
-  if (jsRepresentation_) {
-    jsRepresentation_->lock(runtime).asObject(runtime).setProperty(
-        runtime, propName, result);
-  }
-  return result;
 }
 
 void TurboModule::emitDeviceEvent(
diff --git a/ReactCommon/react/nativemodule/core/ReactCommon/TurboModule.h b/ReactCommon/react/nativemodule/core/ReactCommon/TurboModule.h
index 5491e80eee3..8ca27a22b2d 100644
--- a/ReactCommon/react/nativemodule/core/ReactCommon/TurboModule.h
+++ b/ReactCommon/react/nativemodule/core/ReactCommon/TurboModule.h
@@ -54,7 +54,15 @@ class JSI_EXPORT TurboModule : public facebook::jsi::HostObject {
         // Method was not found, let JS decide what to do.
         return facebook::jsi::Value::undefined();
       } else {
-        return get(runtime, propName, p->second);
+        auto moduleMethod = createHostFunction(runtime, propName, p->second);
+        // If we have a JS wrapper, cache the result of this lookup
+        // We don't cache misses, to allow for methodMap_ to dynamically be
+        // extended
+        if (jsRepresentation_) {
+          jsRepresentation_->lock(runtime).asObject(runtime).setProperty(
+              runtime, propName, moduleMethod);
+        }
+        return moduleMethod;
       }
     }
   }
@@ -108,7 +116,7 @@ class JSI_EXPORT TurboModule : public facebook::jsi::HostObject {
   friend class TurboModuleBinding;
   std::unique_ptr<jsi::WeakObject> jsRepresentation_;
 
-  facebook::jsi::Value get(
+  facebook::jsi::Value createHostFunction(
       facebook::jsi::Runtime &runtime,
       const facebook::jsi::PropNameID &propName,
       const MethodMetadata &meta);