Share re-defined defineReadOnlyGlobal(...) (facebook#42801)

Summary: Fixes some tech debug Changelog: [Internal] [Fixed] Share re-defined defineReadOnlyGlobal(...) Reviewed By: javache Differential Revision: D53340274
christophpurrer · Feb 19, 2024 · 5f774d8 · 5f774d8
1 parent 475a156
commit 5f774d8
Show file tree

Hide file tree

Showing 10 changed files with 87 additions and 70 deletions.
diff --git a/packages/react-native/ReactCommon/ReactCommon.podspec b/packages/react-native/ReactCommon/ReactCommon.podspec
@@ -71,8 +71,9 @@ Pod::Spec.new do |s|
 
     ss.subspec "core" do |sss|
       sss.source_files = "react/nativemodule/core/ReactCommon/**/*.{cpp,h}"
-      sss.pod_target_xcconfig  = { "HEADER_SEARCH_PATHS" => "\"$(PODS_TARGET_SRCROOT)/ReactCommon\" \"$(PODS_CONFIGURATION_BUILD_DIR)/React-debug/React_debug.framework/Headers\"" }
+      sss.pod_target_xcconfig  = { "HEADER_SEARCH_PATHS" => "\"$(PODS_TARGET_SRCROOT)/ReactCommon\" \"$(PODS_CONFIGURATION_BUILD_DIR)/React-debug/React_debug.framework/Headers\" \"$(PODS_CONFIGURATION_BUILD_DIR)/React-utils/React_utils.framework/Headers\"" }
       sss.dependency "React-debug", version
+      sss.dependency "React-utils", version
     end
   end
 end
diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/CMakeLists.txt b/packages/react-native/ReactCommon/react/nativemodule/core/CMakeLists.txt
@@ -35,5 +35,6 @@ target_link_libraries(react_nativemodule_core
         jsi
         react_bridging
         react_debug
+        react_utils
         reactperflogger
         reactnativejni)
diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleBinding.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleBinding.cpp
@@ -7,11 +7,11 @@
 
 #include "TurboModuleBinding.h"
 
+#include <cxxreact/SystraceSection.h>
+#include <react/utils/jsi.h>
 #include <stdexcept>
 #include <string>
 
-#include <cxxreact/SystraceSection.h>
-
 using namespace facebook;
 
 namespace facebook::react {
@@ -62,33 +62,6 @@ class BridgelessNativeModuleProxy : public jsi::HostObject {
   }
 };
 
-// TODO(148359183): Merge this with the Bridgeless defineReadOnlyGlobal util
-static void defineReadOnlyGlobal(
-    jsi::Runtime& runtime,
-    std::string propName,
-    jsi::Value&& value) {
-  if (runtime.global().hasProperty(runtime, propName.c_str())) {
-    throw jsi::JSError(
-        runtime,
-        "Tried to redefine read-only global \"" + propName +
-            "\", but read-only globals can only be defined once.");
-  }
-  jsi::Object jsObject =
-      runtime.global().getProperty(runtime, "Object").asObject(runtime);
-  jsi::Function defineProperty = jsObject.getProperty(runtime, "defineProperty")
-                                     .asObject(runtime)
-                                     .asFunction(runtime);
-
-  jsi::Object descriptor = jsi::Object(runtime);
-  descriptor.setProperty(runtime, "value", std::move(value));
-  defineProperty.callWithThis(
-      runtime,
-      jsObject,
-      runtime.global(),
-      jsi::String::createFromUtf8(runtime, propName),
-      descriptor);
-}
-
 /**
  * Public API to install the TurboModule system.
  */

diff --git a/packages/react-native/ReactCommon/react/runtime/CMakeLists.txt b/packages/react-native/ReactCommon/react/runtime/CMakeLists.txt
@@ -38,4 +38,5 @@ target_link_libraries(
         react_utils
         jsinspector
         react_featureflags
+        react_utils
 )
diff --git a/packages/react-native/ReactCommon/react/runtime/React-RuntimeHermes.podspec b/packages/react-native/ReactCommon/react/runtime/React-RuntimeHermes.podspec
@@ -47,8 +47,8 @@ Pod::Spec.new do |s|
   s.dependency folly_dep_name, folly_version
   s.dependency "React-nativeconfig"
   s.dependency "React-jsitracing"
-  s.dependency "React-utils"
   s.dependency "React-jsi"
+  s.dependency "React-utils"
   s.dependency "React-RuntimeCore"
   s.dependency "React-featureflags"
   add_dependency(s, "React-jsinspector", :framework_name => 'jsinspector_modern')

diff --git a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp
@@ -10,15 +10,15 @@
 #include <cxxreact/ErrorUtils.h>
 #include <cxxreact/JSBigString.h>
 #include <cxxreact/JSExecutor.h>
+#include <cxxreact/ReactMarker.h>
 #include <cxxreact/SystraceSection.h>
 #include <glog/logging.h>
 #include <jsi/JSIDynamic.h>
 #include <jsi/instrumentation.h>
 #include <jsireact/JSIExecutor.h>
 #include <react/featureflags/ReactNativeFeatureFlags.h>
 #include <react/renderer/runtimescheduler/RuntimeSchedulerBinding.h>
-
-#include <cxxreact/ReactMarker.h>
+#include <react/utils/jsi.h>
 #include <iostream>
 #include <tuple>
 #include <utility>
@@ -140,43 +140,6 @@ ReactInstance::getRuntimeScheduler() noexcept {
   return runtimeScheduler_;
 }
 
-/**
- * Defines a property on the global object that is neither enumerable, nor
- * configurable, nor writable. This ensures that the private globals exposed by
- * ReactInstance cannot overwritten by third-party JavaScript code. It also
- * ensures that third-party JavaScript code unaware of these globals isn't able
- * to accidentally access them. In JavaScript, equivalent to:
- *
- * Object.defineProperty(global, propName, {
- *   value: value
- * })
- */
-static void defineReadOnlyGlobal(
-    jsi::Runtime& runtime,
-    std::string propName,
-    jsi::Value&& value) {
-  if (runtime.global().hasProperty(runtime, propName.c_str())) {
-    throw jsi::JSError(
-        runtime,
-        "Tried to redefine read-only global \"" + propName +
-            "\", but read-only globals can only be defined once.");
-  }
-  jsi::Object jsObject =
-      runtime.global().getProperty(runtime, "Object").asObject(runtime);
-  jsi::Function defineProperty = jsObject.getProperty(runtime, "defineProperty")
-                                     .asObject(runtime)
-                                     .asFunction(runtime);
-
-  jsi::Object descriptor = jsi::Object(runtime);
-  descriptor.setProperty(runtime, "value", std::move(value));
-  defineProperty.callWithThis(
-      runtime,
-      jsObject,
-      runtime.global(),
-      jsi::String::createFromUtf8(runtime, propName),
-      descriptor);
-}
-
 namespace {
 
 // Copied from JSIExecutor.cpp

diff --git a/packages/react-native/ReactCommon/react/utils/CMakeLists.txt b/packages/react-native/ReactCommon/react/utils/CMakeLists.txt
@@ -22,4 +22,5 @@ target_include_directories(react_utils PUBLIC ${REACT_COMMON_DIR})
 target_link_libraries(react_utils
         glog
         glog_init
+        jsireact
         react_debug)
diff --git a/packages/react-native/ReactCommon/react/utils/React-utils.podspec b/packages/react-native/ReactCommon/react/utils/React-utils.podspec
@@ -49,7 +49,14 @@ Pod::Spec.new do |s|
   end
 
   s.dependency "RCT-Folly", folly_version
+  s.dependency "React-jsi", version
   s.dependency "glog"
 
+  if ENV["USE_HERMES"] == nil || ENV["USE_HERMES"] == "1"
+    s.dependency "hermes-engine"
+  else
+    s.dependency "React-jsc"
+  end
+
   add_dependency(s, "React-debug")
 end
diff --git a/packages/react-native/ReactCommon/react/utils/jsi.cpp b/packages/react-native/ReactCommon/react/utils/jsi.cpp
@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#include "jsi.h"
+
+namespace facebook::react {
+
+void defineReadOnlyGlobal(
+    jsi::Runtime& runtime,
+    const std::string& propName,
+    jsi::Value&& value) {
+  auto global = runtime.global();
+  if (global.hasProperty(runtime, propName.c_str())) {
+    throw jsi::JSError(
+        runtime,
+        "Tried to redefine read-only global \"" + propName +
+            "\", but read-only globals can only be defined once.");
+  }
+  jsi::Object jsObject =
+      global.getProperty(runtime, "Object").asObject(runtime);
+  jsi::Function defineProperty = jsObject.getProperty(runtime, "defineProperty")
+                                     .asObject(runtime)
+                                     .asFunction(runtime);
+
+  jsi::Object descriptor = jsi::Object(runtime);
+  descriptor.setProperty(runtime, "value", std::move(value));
+  defineProperty.callWithThis(
+      runtime,
+      jsObject,
+      global,
+      jsi::String::createFromUtf8(runtime, propName),
+      descriptor);
+}
+
+} // namespace facebook::react
diff --git a/packages/react-native/ReactCommon/react/utils/jsi.h b/packages/react-native/ReactCommon/react/utils/jsi.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#pragma once
+
+#include <jsi/jsi.h>
+#include <string>
+
+namespace facebook::react {
+
+/**
+ * Defines a property on the global object that is neither enumerable, nor
+ * configurable, nor writable. This ensures that the private globals exposed by
+ * ReactInstance cannot overwritten by third-party JavaScript code. It also
+ * ensures that third-party JavaScript code unaware of these globals isn't able
+ * to accidentally access them. In JavaScript, equivalent to:
+ *
+ * Object.defineProperty(global, propName, {
+ *   value: value
+ * })
+ */
+void defineReadOnlyGlobal(
+    jsi::Runtime& runtime,
+    const std::string& propName,
+    jsi::Value&& value);
+
+} // namespace facebook::react