Skip to content

Commit

Permalink
cleanup warnOnLegacyNativeModuleSystemUse (#39417)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #39417

## Changelog
[Android][Breaking] - Deleting warnOnLegacyNativeModuleSystemUse

this has not fired in over a year, we should clean it up. there's an argument we want to keep this for consumers that are in the middle of upgrading, but personally i think that this needs to be built into the interop layer - currently we can only configure this in Fb specific infra.

Reviewed By: cortinico

Differential Revision: D49209540

fbshipit-source-id: bc21100b7d41af6a1b3ea81f15f92d3c74d6b672
  • Loading branch information
philIip authored and facebook-github-bot committed Sep 13, 2023
1 parent 9faf256 commit bc5cecc
Show file tree
Hide file tree
Showing 6 changed files with 0 additions and 82 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,6 @@ 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,
Expand Down Expand Up @@ -211,8 +207,6 @@ 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

import androidx.annotation.Nullable;
import com.facebook.proguard.annotations.DoNotStrip;
import com.facebook.react.config.ReactFeatureFlags;
import com.facebook.react.turbomodule.core.interfaces.TurboModule;
import com.facebook.systrace.Systrace;
import com.facebook.systrace.SystraceMessage;
Expand Down Expand Up @@ -116,17 +115,6 @@ public List<MethodDescriptor> getMethodDescriptors() {

@DoNotStrip
public @Nullable NativeMap getConstants() {
if (ReactFeatureFlags.warnOnLegacyNativeModuleSystemUse) {
ReactSoftExceptionLogger.logSoftException(
TAG,
new ReactNoCrashSoftException(
"Calling getConstants() on Java NativeModule (name = \""
+ mModuleHolder.getName()
+ "\", className = "
+ mModuleHolder.getClassName()
+ ")."));
}

if (!mModuleHolder.getHasConstants()) {
return null;
}
Expand Down Expand Up @@ -158,34 +146,10 @@ public List<MethodDescriptor> getMethodDescriptors() {

@DoNotStrip
public void invoke(int methodId, ReadableNativeArray parameters) {
if (ReactFeatureFlags.warnOnLegacyNativeModuleSystemUse) {
ReactSoftExceptionLogger.logSoftException(
TAG,
new ReactNoCrashSoftException(
"Calling method on Java NativeModule (name = \""
+ mModuleHolder.getName()
+ "\", className = "
+ mModuleHolder.getClassName()
+ ")."));
}

if (mMethods == null || methodId >= mMethods.size()) {
return;
}

if (ReactFeatureFlags.warnOnLegacyNativeModuleSystemUse) {
ReactSoftExceptionLogger.logSoftException(
TAG,
new ReactNoCrashSoftException(
"Calling "
+ mDescs.get(methodId).name
+ "() on Java NativeModule (name = \""
+ mModuleHolder.getName()
+ "\", className = "
+ mModuleHolder.getClassName()
+ ")."));
}

mMethods.get(methodId).invoke(mJSInstance, parameters);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
package com.facebook.react.bridge;

import com.facebook.infer.annotation.Assertions;
import com.facebook.react.config.ReactFeatureFlags;
import com.facebook.react.module.annotations.ReactModule;
import com.facebook.systrace.Systrace;
import java.util.ArrayList;
Expand Down Expand Up @@ -42,17 +41,6 @@ private ReactApplicationContext getReactApplicationContext() {
ArrayList<JavaModuleWrapper> javaModules = new ArrayList<>();
for (Map.Entry<String, ModuleHolder> entry : mModules.entrySet()) {
if (!entry.getValue().isCxxModule()) {
if (ReactFeatureFlags.warnOnLegacyNativeModuleSystemUse) {
ReactSoftExceptionLogger.logSoftException(
TAG,
new ReactNoCrashSoftException(
"Registering legacy NativeModule: Java NativeModule (name = \""
+ entry.getValue().getName()
+ "\", className = "
+ entry.getValue().getClassName()
+ ")."));
}

javaModules.add(new JavaModuleWrapper(jsInstance, entry.getValue()));
}
}
Expand All @@ -63,16 +51,6 @@ private ReactApplicationContext getReactApplicationContext() {
ArrayList<ModuleHolder> cxxModules = new ArrayList<>();
for (Map.Entry<String, ModuleHolder> entry : mModules.entrySet()) {
if (entry.getValue().isCxxModule()) {
if (ReactFeatureFlags.warnOnLegacyNativeModuleSystemUse) {
ReactSoftExceptionLogger.logSoftException(
TAG,
new ReactNoCrashSoftException(
"Registering legacy NativeModule: Cxx NativeModule (name = \""
+ entry.getValue().getName()
+ "\", className = "
+ entry.getValue().getClassName()
+ ")."));
}
cxxModules.add(entry.getValue());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,6 @@ public class ReactFeatureFlags {
/** Does the bridgeless architecture use the new create/reload/destroy routines */
public static volatile boolean enableBridgelessArchitectureNewCreateReloadDestroy = false;

/**
* After TurboModules and Fabric are enabled, we need to ensure that the legacy NativeModule isn't
* isn't used. So, turn this flag on to trigger warnings whenever the legacy NativeModule system
* is used.
*/
public static volatile boolean warnOnLegacyNativeModuleSystemUse = false;

/** This feature flag enables logs for Fabric */
public static boolean enableFabricLogs = false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,6 @@ CatalystInstanceImpl::initHybrid(jni::alias_ref<jclass>) {
CatalystInstanceImpl::CatalystInstanceImpl()
: instance_(std::make_unique<Instance>()) {}

void CatalystInstanceImpl::warnOnLegacyNativeModuleSystemUse() {
CxxNativeModule::setShouldWarnOnUse(true);
}

void CatalystInstanceImpl::registerNatives() {
registerHybrid({
makeNativeMethod("initHybrid", CatalystInstanceImpl::initHybrid),
Expand Down Expand Up @@ -140,9 +136,6 @@ void CatalystInstanceImpl::registerNatives() {
"getRuntimeExecutor", CatalystInstanceImpl::getRuntimeExecutor),
makeNativeMethod(
"getRuntimeScheduler", CatalystInstanceImpl::getRuntimeScheduler),
makeNativeMethod(
"warnOnLegacyNativeModuleSystemUse",
CatalystInstanceImpl::warnOnLegacyNativeModuleSystemUse),
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,6 @@ 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,
Expand Down

0 comments on commit bc5cecc

Please sign in to comment.