Skip to content

Commit f536f82

Browse files
RSNarafacebook-github-bot
authored andcommitted
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
1 parent b7fd68e commit f536f82

File tree

5 files changed

+54
-0
lines changed

5 files changed

+54
-0
lines changed

ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,11 @@ private CatalystInstanceImpl(
139139

140140
FLog.d(ReactConstants.TAG, "Initializing React Xplat Bridge before initializeBridge");
141141
Systrace.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "initializeCxxBridge");
142+
143+
if (ReactFeatureFlags.warnOnLegacyNativeModuleSystemUse) {
144+
warnOnLegacyNativeModuleSystemUse();
145+
}
146+
142147
initializeBridge(
143148
new BridgeCallback(this),
144149
jsExecutor,
@@ -206,6 +211,8 @@ public void extendNativeModules(NativeModuleRegistry modules) {
206211
private native void jniExtendNativeModules(
207212
Collection<JavaModuleWrapper> javaModules, Collection<ModuleHolder> cxxModules);
208213

214+
private native void warnOnLegacyNativeModuleSystemUse();
215+
209216
private native void initializeBridge(
210217
ReactCallback callback,
211218
JavaScriptExecutor jsExecutor,

ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929

3030
#include "CxxModuleWrapper.h"
3131
#include "JNativeRunnable.h"
32+
#include "JReactSoftExceptionLogger.h"
3233
#include "JavaScriptExecutorHolder.h"
3334
#include "JniJSModulesUnbundle.h"
3435
#include "NativeArray.h"
@@ -92,6 +93,15 @@ CatalystInstanceImpl::initHybrid(jni::alias_ref<jclass>) {
9293
CatalystInstanceImpl::CatalystInstanceImpl()
9394
: instance_(std::make_unique<Instance>()) {}
9495

96+
void logSoftException(std::string message) {
97+
JReactSoftExceptionLogger::logNoThrowSoftExceptionWithMessage(
98+
"ReactNativeLogger#warning", message);
99+
}
100+
101+
void CatalystInstanceImpl::warnOnLegacyNativeModuleSystemUse() {
102+
CxxNativeModule::setWarnOnUsageLogger(&logSoftException);
103+
}
104+
95105
void CatalystInstanceImpl::registerNatives() {
96106
registerHybrid({
97107
makeNativeMethod("initHybrid", CatalystInstanceImpl::initHybrid),
@@ -127,6 +137,9 @@ void CatalystInstanceImpl::registerNatives() {
127137
CatalystInstanceImpl::handleMemoryPressure),
128138
makeNativeMethod(
129139
"getRuntimeExecutor", CatalystInstanceImpl::getRuntimeExecutor),
140+
makeNativeMethod(
141+
"warnOnLegacyNativeModuleSystemUse",
142+
CatalystInstanceImpl::warnOnLegacyNativeModuleSystemUse),
130143
});
131144

132145
JNativeRunnable::registerNatives();

ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ class CatalystInstanceImpl : public jni::HybridClass<CatalystInstanceImpl> {
6161
jni::alias_ref<jni::JCollection<ModuleHolder::javaobject>::javaobject>
6262
cxxModules);
6363

64+
// When called from CatalystInstanceImpl.java, warnings will be logged when
65+
// CxxNativeModules are used. Java NativeModule usages log error in Java.
66+
void warnOnLegacyNativeModuleSystemUse();
67+
6468
void extendNativeModules(
6569
jni::alias_ref<jni::JCollection<
6670
JavaModuleWrapper::javaobject>::javaobject> javaModules,

ReactCommon/cxxreact/CxxNativeModule.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,12 @@ CxxModule::Callback convertCallback(
5454

5555
} // namespace
5656

57+
WarnOnUsageLogger CxxNativeModule::warnOnUsageLogger_ = nullptr;
58+
59+
void CxxNativeModule::setWarnOnUsageLogger(WarnOnUsageLogger logger) {
60+
warnOnUsageLogger_ = logger;
61+
}
62+
5763
std::string CxxNativeModule::getName() {
5864
return name_;
5965
}
@@ -87,6 +93,12 @@ folly::dynamic CxxNativeModule::getConstants() {
8793
return nullptr;
8894
}
8995

96+
if (warnOnUsageLogger_) {
97+
warnOnUsageLogger_(
98+
"Calling getConstants() on Cxx NativeModule (name = \"" + getName() +
99+
"\").");
100+
}
101+
90102
folly::dynamic constants = folly::dynamic::object();
91103
for (auto &pair : module_->getConstants()) {
92104
constants.insert(std::move(pair.first), std::move(pair.second));
@@ -121,6 +133,12 @@ void CxxNativeModule::invoke(
121133
"Method ", method.name, " is synchronous but invoked asynchronously"));
122134
}
123135

136+
if (warnOnUsageLogger_) {
137+
warnOnUsageLogger_(
138+
"Calling " + method.name + "() on Cxx NativeModule (name = \"" +
139+
getName() + "\").");
140+
}
141+
124142
if (params.size() < method.callbacks) {
125143
throw std::invalid_argument(folly::to<std::string>(
126144
"Expected ",
@@ -204,6 +222,12 @@ MethodCallResult CxxNativeModule::callSerializableNativeHook(
204222
"Method ", method.name, " is asynchronous but invoked synchronously"));
205223
}
206224

225+
if (warnOnUsageLogger_) {
226+
warnOnUsageLogger_(
227+
"Calling " + method.name + "() on Cxx NativeModule (name = \"" +
228+
getName() + "\").");
229+
}
230+
207231
return method.syncFunc(std::move(args));
208232
}
209233

ReactCommon/cxxreact/CxxNativeModule.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ namespace react {
2020
class Instance;
2121
class MessageQueueThread;
2222

23+
typedef void (*WarnOnUsageLogger)(std::string message);
24+
2325
std::function<void(folly::dynamic)> makeCallback(
2426
std::weak_ptr<Instance> instance,
2527
const folly::dynamic &callbackId);
@@ -46,6 +48,8 @@ class RN_EXPORT CxxNativeModule : public NativeModule {
4648
unsigned int hookId,
4749
folly::dynamic &&args) override;
4850

51+
static void setWarnOnUsageLogger(WarnOnUsageLogger logger);
52+
4953
private:
5054
void lazyInit();
5155

@@ -55,6 +59,8 @@ class RN_EXPORT CxxNativeModule : public NativeModule {
5559
std::shared_ptr<MessageQueueThread> messageQueueThread_;
5660
std::unique_ptr<xplat::module::CxxModule> module_;
5761
std::vector<xplat::module::CxxModule::Method> methods_;
62+
63+
static WarnOnUsageLogger warnOnUsageLogger_;
5864
};
5965

6066
} // namespace react

0 commit comments

Comments
 (0)