Warn whenever CxxNativeModules are used

Summary: After this diff, when ReactFeatureFlags.warnOnLegacyNativeModuleSystemUse is enabled, the legacy NativeModule infra will log soft exceptions whenever legacy NativeModules are accessed/used. Changelog: [Internal] Reviewed By: p-sun Differential Revision: D30272695 fbshipit-source-id: 7111402c1d8b883a600dcb4559e9ff1d56447070
facebook · Aug 13, 2021 · f536f82 · f536f82
1 parent b7fd68e
commit f536f82
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 0 deletions.
diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java
@@ -139,6 +139,11 @@ private CatalystInstanceImpl(
 
     FLog.d(ReactConstants.TAG, "Initializing React Xplat Bridge before initializeBridge");
     Systrace.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "initializeCxxBridge");
+
+    if (ReactFeatureFlags.warnOnLegacyNativeModuleSystemUse) {
+      warnOnLegacyNativeModuleSystemUse();
+    }
+
     initializeBridge(
         new BridgeCallback(this),
         jsExecutor,
@@ -206,6 +211,8 @@ public void extendNativeModules(NativeModuleRegistry modules) {
   private native void jniExtendNativeModules(
       Collection<JavaModuleWrapper> javaModules, Collection<ModuleHolder> cxxModules);
 
+  private native void warnOnLegacyNativeModuleSystemUse();
+
   private native void initializeBridge(
       ReactCallback callback,
       JavaScriptExecutor jsExecutor,

diff --git a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp
@@ -29,6 +29,7 @@
 
 #include "CxxModuleWrapper.h"
 #include "JNativeRunnable.h"
+#include "JReactSoftExceptionLogger.h"
 #include "JavaScriptExecutorHolder.h"
 #include "JniJSModulesUnbundle.h"
 #include "NativeArray.h"
@@ -92,6 +93,15 @@ CatalystInstanceImpl::initHybrid(jni::alias_ref<jclass>) {
 CatalystInstanceImpl::CatalystInstanceImpl()
     : instance_(std::make_unique<Instance>()) {}
 
+void logSoftException(std::string message) {
+  JReactSoftExceptionLogger::logNoThrowSoftExceptionWithMessage(
+      "ReactNativeLogger#warning", message);
+}
+
+void CatalystInstanceImpl::warnOnLegacyNativeModuleSystemUse() {
+  CxxNativeModule::setWarnOnUsageLogger(&logSoftException);
+}
+
 void CatalystInstanceImpl::registerNatives() {
   registerHybrid({
       makeNativeMethod("initHybrid", CatalystInstanceImpl::initHybrid),
@@ -127,6 +137,9 @@ void CatalystInstanceImpl::registerNatives() {
           CatalystInstanceImpl::handleMemoryPressure),
       makeNativeMethod(
           "getRuntimeExecutor", CatalystInstanceImpl::getRuntimeExecutor),
+      makeNativeMethod(
+          "warnOnLegacyNativeModuleSystemUse",
+          CatalystInstanceImpl::warnOnLegacyNativeModuleSystemUse),
   });
 
   JNativeRunnable::registerNatives();

diff --git a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h
@@ -61,6 +61,10 @@ class CatalystInstanceImpl : public jni::HybridClass<CatalystInstanceImpl> {
       jni::alias_ref<jni::JCollection<ModuleHolder::javaobject>::javaobject>
           cxxModules);
 
+  // When called from CatalystInstanceImpl.java, warnings will be logged when
+  // CxxNativeModules are used. Java NativeModule usages log error in Java.
+  void warnOnLegacyNativeModuleSystemUse();
+
   void extendNativeModules(
       jni::alias_ref<jni::JCollection<
           JavaModuleWrapper::javaobject>::javaobject> javaModules,

diff --git a/ReactCommon/cxxreact/CxxNativeModule.cpp b/ReactCommon/cxxreact/CxxNativeModule.cpp
@@ -54,6 +54,12 @@ CxxModule::Callback convertCallback(
 
 } // namespace
 
+WarnOnUsageLogger CxxNativeModule::warnOnUsageLogger_ = nullptr;
+
+void CxxNativeModule::setWarnOnUsageLogger(WarnOnUsageLogger logger) {
+  warnOnUsageLogger_ = logger;
+}
+
 std::string CxxNativeModule::getName() {
   return name_;
 }
@@ -87,6 +93,12 @@ folly::dynamic CxxNativeModule::getConstants() {
     return nullptr;
   }
 
+  if (warnOnUsageLogger_) {
+    warnOnUsageLogger_(
+        "Calling getConstants() on Cxx NativeModule (name = \"" + getName() +
+        "\").");
+  }
+
   folly::dynamic constants = folly::dynamic::object();
   for (auto &pair : module_->getConstants()) {
     constants.insert(std::move(pair.first), std::move(pair.second));
@@ -121,6 +133,12 @@ void CxxNativeModule::invoke(
         "Method ", method.name, " is synchronous but invoked asynchronously"));
   }
 
+  if (warnOnUsageLogger_) {
+    warnOnUsageLogger_(
+        "Calling " + method.name + "() on Cxx NativeModule (name = \"" +
+        getName() + "\").");
+  }
+
   if (params.size() < method.callbacks) {
     throw std::invalid_argument(folly::to<std::string>(
         "Expected ",
@@ -204,6 +222,12 @@ MethodCallResult CxxNativeModule::callSerializableNativeHook(
         "Method ", method.name, " is asynchronous but invoked synchronously"));
   }
 
+  if (warnOnUsageLogger_) {
+    warnOnUsageLogger_(
+        "Calling " + method.name + "() on Cxx NativeModule (name = \"" +
+        getName() + "\").");
+  }
+
   return method.syncFunc(std::move(args));
 }
 

diff --git a/ReactCommon/cxxreact/CxxNativeModule.h b/ReactCommon/cxxreact/CxxNativeModule.h
@@ -20,6 +20,8 @@ namespace react {
 class Instance;
 class MessageQueueThread;
 
+typedef void (*WarnOnUsageLogger)(std::string message);
+
 std::function<void(folly::dynamic)> makeCallback(
     std::weak_ptr<Instance> instance,
     const folly::dynamic &callbackId);
@@ -46,6 +48,8 @@ class RN_EXPORT CxxNativeModule : public NativeModule {
       unsigned int hookId,
       folly::dynamic &&args) override;
 
+  static void setWarnOnUsageLogger(WarnOnUsageLogger logger);
+
  private:
   void lazyInit();
 
@@ -55,6 +59,8 @@ class RN_EXPORT CxxNativeModule : public NativeModule {
   std::shared_ptr<MessageQueueThread> messageQueueThread_;
   std::unique_ptr<xplat::module::CxxModule> module_;
   std::vector<xplat::module::CxxModule::Method> methods_;
+
+  static WarnOnUsageLogger warnOnUsageLogger_;
 };
 
 } // namespace react