fix: Log all hybrid object names as joined string to help user (#508)

* fix: Log all hybrid object names as joined string to help user * fix: Avoid `std::string` call * fix: Include * Rename something * perf: even more
mrousavy · Jan 22, 2025 · 98575fd · 98575fd
1 parent 35a4efb
commit 98575fd
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 20 deletions.
diff --git a/example/ios/NitroExample.xcodeproj/project.pbxproj b/example/ios/NitroExample.xcodeproj/project.pbxproj
@@ -610,7 +610,10 @@
 					"-DFOLLY_CFG_NO_COROUTINES=1",
 					"-DFOLLY_HAVE_CLOCK_GETTIME=1",
 				);
-				OTHER_LDFLAGS = "$(inherited)  ";
+				OTHER_LDFLAGS = (
+					"$(inherited)",
+					" ",
+				);
 				REACT_NATIVE_PATH = "${PODS_ROOT}/../../../node_modules/react-native";
 				SDKROOT = iphoneos;
 				SWIFT_ACTIVE_COMPILATION_CONDITIONS = "$(inherited) DEBUG";
@@ -684,7 +687,10 @@
 					"-DFOLLY_CFG_NO_COROUTINES=1",
 					"-DFOLLY_HAVE_CLOCK_GETTIME=1",
 				);
-				OTHER_LDFLAGS = "$(inherited)  ";
+				OTHER_LDFLAGS = (
+					"$(inherited)",
+					" ",
+				);
 				REACT_NATIVE_PATH = "${PODS_ROOT}/../../../node_modules/react-native";
 				SDKROOT = iphoneos;
 				SWIFT_OBJC_INTEROP_MODE = objcxx;

diff --git a/example/ios/Podfile.lock b/example/ios/Podfile.lock
@@ -1895,7 +1895,7 @@ SPEC CHECKSUMS:
   glog: 08b301085f15bcbb6ff8632a8ebaf239aae04e6a
   hermes-engine: 1949ca944b195a8bde7cbf6316b9068e19cf53c6
   NitroImage: ccc116b3881f723f1d3f77743acf8028681160f1
-  NitroModules: 8448d25a61e25ef84aa056568d3036fd0c202e03
+  NitroModules: 1c0a6c59d4cfaedb7406f41cdf239269f2f5417b
   RCT-Folly: bf5c0376ffe4dd2cf438dcf86db385df9fdce648
   RCTDeprecation: 063fc281b30b7dc944c98fe53a7e266dab1a8706
   RCTRequired: 8eda2a5a745f6081157a4f34baac40b65fe02b31

diff --git a/packages/react-native-nitro-modules/android/src/main/cpp/platform/ThreadUtils.cpp b/packages/react-native-nitro-modules/android/src/main/cpp/platform/ThreadUtils.cpp
@@ -17,12 +17,12 @@ namespace margelo::nitro {
 std::string ThreadUtils::getThreadName() {
 #ifdef HAVE_ANDROID_PTHREAD_SETNAME_NP
   // Try using pthread APIs
-  pthread_t this_thread = pthread_self();
-  char thread_name[16]; // Thread name length limit in Android is 16 characters
+  pthread_t thisThread = pthread_self();
+  char threadName[16]; // Thread name length limit in Android is 16 characters
 
-  int result = pthread_getname_np(this_thread, thread_name, sizeof(thread_name));
+  int result = pthread_getname_np(thisThread, threadName, sizeof(threadName));
   if (result == 0) {
-    return std::string(thread_name);
+    return std::string(threadName);
   }
 #endif
 

diff --git a/packages/react-native-nitro-modules/cpp/jsi/JSIHelpers.hpp b/packages/react-native-nitro-modules/cpp/jsi/JSIHelpers.hpp
@@ -46,7 +46,7 @@ static inline bool isPlainObject(jsi::Runtime& runtime, const jsi::Object& objec
  */
 static inline std::string getRuntimeId(jsi::Runtime& runtime) {
   std::string threadName = ThreadUtils::getThreadName();
-  return runtime.description() + std::string(" (") + threadName + std::string(")");
+  return runtime.description() + " (" + threadName + ")";
 }
 
 } // namespace margelo::nitro
diff --git a/packages/react-native-nitro-modules/cpp/registry/HybridObjectRegistry.cpp b/packages/react-native-nitro-modules/cpp/registry/HybridObjectRegistry.cpp
@@ -8,6 +8,7 @@
 #include "HybridObjectRegistry.hpp"
 #include "NitroDefines.hpp"
 #include "NitroLogger.hpp"
+#include <numeric>
 
 namespace margelo::nitro {
 
@@ -29,17 +30,25 @@ std::vector<std::string> HybridObjectRegistry::getAllHybridObjectNames() {
   return keys;
 }
 
+std::string HybridObjectRegistry::getAllRegisteredHybridObjectNamesToString() {
+  std::vector<std::string> names = getAllHybridObjectNames();
+  return std::accumulate(std::next(names.begin()), names.end(), names[0], [](std::string a, std::string b) { return a + ", " + b; });
+}
+
 void HybridObjectRegistry::registerHybridObjectConstructor(const std::string& hybridObjectName, HybridObjectConstructorFn&& constructorFn) {
   Logger::log(LogLevel::Info, TAG, "Registering HybridObject \"%s\"...", hybridObjectName.c_str());
   auto& map = HybridObjectRegistry::getRegistry();
 #ifdef NITRO_DEBUG
   if (map.contains(hybridObjectName)) [[unlikely]] {
+    auto allObjectNames = getAllRegisteredHybridObjectNamesToString();
     auto message =
-        "HybridObject \"" + std::string(hybridObjectName) +
+        "HybridObject \"" + hybridObjectName +
         "\" has already been "
         "registered in the Nitro Modules HybridObjectRegistry! Suggestions:\n"
         "- If you just installed another library, maybe both libraries are using the same name?\n"
-        "- If you just registered your own HybridObject, maybe you accidentally called `registerHybridObjectConstructor(...)` twice?";
+        "- If you just registered your own HybridObject, maybe you accidentally called `registerHybridObjectConstructor(...)` twice?\n"
+        "- All registered HybridObjects: [" +
+        allObjectNames + "]";
     throw std::runtime_error(message);
   }
 #endif
@@ -57,16 +66,20 @@ std::shared_ptr<HybridObject> HybridObjectRegistry::createHybridObject(const std
   auto& map = HybridObjectRegistry::getRegistry();
   auto fn = map.find(hybridObjectName);
   if (fn == map.end()) [[unlikely]] {
-    auto message = "Cannot create an instance of HybridObject \"" + std::string(hybridObjectName) +
-                   "\" - It has not yet been registered in the Nitro Modules HybridObjectRegistry! Suggestions:\n"
-                   "- If you use Nitrogen, make sure your `nitro.json` contains `" +
-                   std::string(hybridObjectName) +
-                   "` on this platform.\n"
-                   "- If you use Nitrogen, make sure your library (*Package.java)/app (MainApplication.java) calls "
-                   "`$$androidCxxLibName$$OnLoad.initializeNative()` somewhere on app-startup.\n"
-                   "- If you use Nitrogen, make sure your cpp-adapter.cpp calls `margelo::nitro::$$cxxNamespace$$::initialize(vm)`.\n"
-                   "- If you use Nitrogen, inspect the generated `$$androidCxxLibName$$OnLoad.cpp` file.\n"
-                   "- If you don't use Nitrogen, make sure you called `HybridObjectRegistry.registerHybridObject(...)`.";
+    auto allObjectNames = getAllRegisteredHybridObjectNamesToString();
+    auto message =
+        "Cannot create an instance of HybridObject \"" + hybridObjectName +
+        "\" - It has not yet been registered in the Nitro Modules HybridObjectRegistry! Suggestions:\n"
+        "- If you use Nitrogen, make sure your `nitro.json` contains `" +
+        hybridObjectName +
+        "` on this platform.\n"
+        "- If you use Nitrogen, make sure your library (*Package.java)/app (MainApplication.java) calls "
+        "`$$androidCxxLibName$$OnLoad.initializeNative()` somewhere on app-startup.\n"
+        "- If you use Nitrogen, make sure your `cpp-adapter.cpp`/`OnLoad.cpp` calls `margelo::nitro::$$cxxNamespace$$::initialize(vm)`.\n"
+        "- If you use Nitrogen, inspect the generated `$$androidCxxLibName$$OnLoad.cpp` file.\n"
+        "- If you don't use Nitrogen, make sure you called `HybridObjectRegistry.registerHybridObject(...)`."
+        "- All registered HybridObjects: [" +
+        allObjectNames + "]";
     throw std::runtime_error(message);
   }
   std::shared_ptr<HybridObject> instance = fn->second();

diff --git a/packages/react-native-nitro-modules/cpp/registry/HybridObjectRegistry.hpp b/packages/react-native-nitro-modules/cpp/registry/HybridObjectRegistry.hpp
@@ -44,6 +44,7 @@ class HybridObjectRegistry {
 
 private:
   static std::unordered_map<std::string, HybridObjectConstructorFn>& getRegistry();
+  static std::string getAllRegisteredHybridObjectNamesToString();
 
 private:
   static constexpr auto TAG = "HybridObjectRegistry";