From 9859fbc2ec9f50eba3672322a85ba7fd9fc11145 Mon Sep 17 00:00:00 2001 From: Phillip Pan Date: Wed, 13 Sep 2023 17:16:53 -0700 Subject: [PATCH] cleanup warnOnLegacyNativeModuleSystemUse (#39417) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/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: 0e2bb293999ad356e564a0acf069211a7a205f21 --- .../react/bridge/CatalystInstanceImpl.java | 6 ---- .../react/bridge/JavaModuleWrapper.java | 36 ------------------- .../react/bridge/NativeModuleRegistry.java | 22 ------------ .../react/config/ReactFeatureFlags.java | 7 ---- .../jni/react/jni/CatalystInstanceImpl.cpp | 7 ---- .../main/jni/react/jni/CatalystInstanceImpl.h | 4 --- 6 files changed, 82 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java index 68817d66c7787f..2fb5e10a4a7568 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java @@ -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, @@ -211,8 +207,6 @@ public void extendNativeModules(NativeModuleRegistry modules) { private native void jniExtendNativeModules( Collection javaModules, Collection cxxModules); - private native void warnOnLegacyNativeModuleSystemUse(); - private native void initializeBridge( ReactCallback callback, JavaScriptExecutor jsExecutor, diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaModuleWrapper.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaModuleWrapper.java index 521f00f07a9c1f..4f0533165ff0d2 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaModuleWrapper.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaModuleWrapper.java @@ -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; @@ -116,17 +115,6 @@ public List 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; } @@ -158,34 +146,10 @@ public List 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); } } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModuleRegistry.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModuleRegistry.java index cb934797512ca3..d4ad9fafa614d0 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModuleRegistry.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModuleRegistry.java @@ -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; @@ -42,17 +41,6 @@ private ReactApplicationContext getReactApplicationContext() { ArrayList javaModules = new ArrayList<>(); for (Map.Entry 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())); } } @@ -63,16 +51,6 @@ private ReactApplicationContext getReactApplicationContext() { ArrayList cxxModules = new ArrayList<>(); for (Map.Entry 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()); } } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java index 40c0ec38e52943..420fba36c0df83 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -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; diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp index e8761b39ad5143..dd837d50dff5e8 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp @@ -99,10 +99,6 @@ CatalystInstanceImpl::initHybrid(jni::alias_ref) { CatalystInstanceImpl::CatalystInstanceImpl() : instance_(std::make_unique()) {} -void CatalystInstanceImpl::warnOnLegacyNativeModuleSystemUse() { - CxxNativeModule::setShouldWarnOnUse(true); -} - void CatalystInstanceImpl::registerNatives() { registerHybrid({ makeNativeMethod("initHybrid", CatalystInstanceImpl::initHybrid), @@ -140,9 +136,6 @@ void CatalystInstanceImpl::registerNatives() { "getRuntimeExecutor", CatalystInstanceImpl::getRuntimeExecutor), makeNativeMethod( "getRuntimeScheduler", CatalystInstanceImpl::getRuntimeScheduler), - makeNativeMethod( - "warnOnLegacyNativeModuleSystemUse", - CatalystInstanceImpl::warnOnLegacyNativeModuleSystemUse), }); } diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h b/packages/react-native/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h index a3cac21b2bc2c5..7c88f0b6949aa5 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h +++ b/packages/react-native/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h @@ -62,10 +62,6 @@ class CatalystInstanceImpl : public jni::HybridClass { jni::alias_ref::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::javaobject> javaModules,