From 0b14eed185dfbf3a99732db18c1968395505595d Mon Sep 17 00:00:00 2001 From: David Vacca Date: Fri, 12 Jan 2024 09:20:31 -0800 Subject: [PATCH] Revert experiment that disables NativeModule codegen (#41978) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/41978 I'm revering the removal of ReactModule codegen. We are postpoinging the removal of the codegen for the future, the reasons are: - resources: the experiment that removes the codegen shows neutral metrics, but the codegen is shared between bridge and bridgeless, so we will need to implement and test the removal for bridge and we don't have the time to do this right now. - reduce fragmentation: we don't want to fragment NativeModules configuration between bridge and bridgeless, doing so will bring a lot of confusion to developers - we don't want to introduce a public APIs in 0.73 that we know they are not used in production for now, we better remove these "unstable" apis before 0.74 cut Note: I'm updating ReactAndroid.api because this is an intended change of APIs which were not part of 0.73 and we don't want them to be part of 0.74. changelog: [internal] internal Reviewed By: RSNara Differential Revision: D52223650 fbshipit-source-id: 681bf5e4aab776505f64b1972a6ace6340db4587 --- .../ReactAndroid/api/ReactAndroid.api | 6 +- ...eactPackageTurboModuleManagerDelegate.java | 75 ------------------- .../react/config/ReactFeatureFlags.java | 3 - .../DefaultTurboModuleManagerDelegate.kt | 19 +---- .../turbomodule/core/TurboModuleManager.java | 16 +--- .../core/TurboModuleManagerDelegate.java | 2 - 6 files changed, 6 insertions(+), 115 deletions(-) diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api index 0199815b06dd63..3eda49cb92bd54 100644 --- a/packages/react-native/ReactAndroid/api/ReactAndroid.api +++ b/packages/react-native/ReactAndroid/api/ReactAndroid.api @@ -326,7 +326,6 @@ public abstract class com/facebook/react/ReactPackageTurboModuleManagerDelegate public fun getLegacyModule (Ljava/lang/String;)Lcom/facebook/react/bridge/NativeModule; public fun getModule (Ljava/lang/String;)Lcom/facebook/react/internal/turbomodule/core/interfaces/TurboModule; public fun unstable_enableSyncVoidMethods ()Z - public fun unstable_isLazyTurboModuleDelegate ()Z public fun unstable_isLegacyModuleRegistered (Ljava/lang/String;)Z public fun unstable_isModuleRegistered (Ljava/lang/String;)Z public fun unstable_shouldEnableLegacyModuleInterop ()Z @@ -1916,7 +1915,6 @@ public class com/facebook/react/config/ReactFeatureFlags { public static field enableOnDemandReactChoreographer Z public static field enableRemoveDeleteTreeInstruction Z public static field enableTextSpannableCache Z - public static field enableTurboModuleStableAPI Z public static field enableViewRecycling Z public static field excludeYogaFromRawProps Z public static field fixStoppedSurfaceTagSetLeak Z @@ -1989,8 +1987,7 @@ public abstract class com/facebook/react/defaults/DefaultReactNativeHost : com/f public final class com/facebook/react/defaults/DefaultTurboModuleManagerDelegate : com/facebook/react/ReactPackageTurboModuleManagerDelegate { public static final field Companion Lcom/facebook/react/defaults/DefaultTurboModuleManagerDelegate$Companion; - public synthetic fun (Lcom/facebook/react/bridge/ReactApplicationContext;Ljava/util/List;Ljava/util/List;Ljava/util/List;Lkotlin/jvm/internal/DefaultConstructorMarker;)V - public fun getEagerInitModuleNames ()Ljava/util/List; + public synthetic fun (Lcom/facebook/react/bridge/ReactApplicationContext;Ljava/util/List;Ljava/util/List;Lkotlin/jvm/internal/DefaultConstructorMarker;)V public static final fun initHybrid (Ljava/util/List;)Lcom/facebook/jni/HybridData; } @@ -1999,7 +1996,6 @@ public final class com/facebook/react/defaults/DefaultTurboModuleManagerDelegate public final fun addCxxReactPackage (Lkotlin/jvm/functions/Function0;)Lcom/facebook/react/defaults/DefaultTurboModuleManagerDelegate$Builder; public final fun addCxxReactPackage (Lkotlin/jvm/functions/Function1;)Lcom/facebook/react/defaults/DefaultTurboModuleManagerDelegate$Builder; public synthetic fun build (Lcom/facebook/react/bridge/ReactApplicationContext;Ljava/util/List;)Lcom/facebook/react/ReactPackageTurboModuleManagerDelegate; - public final fun setEagerInitModuleNames (Ljava/util/List;)Lcom/facebook/react/defaults/DefaultTurboModuleManagerDelegate$Builder; } public final class com/facebook/react/defaults/DefaultTurboModuleManagerDelegate$Companion { diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactPackageTurboModuleManagerDelegate.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactPackageTurboModuleManagerDelegate.java index 949276ec20b195..aba8502f55c405 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactPackageTurboModuleManagerDelegate.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactPackageTurboModuleManagerDelegate.java @@ -48,8 +48,6 @@ interface ModuleProvider { private final boolean mEnableTurboModuleSyncVoidMethods = ReactFeatureFlags.unstable_enableTurboModuleSyncVoidMethods; - private final boolean mIsLazy = ReactFeatureFlags.enableTurboModuleStableAPI; - // Lazy Props private List mPackages; private ReactApplicationContext mReactContext; @@ -70,12 +68,6 @@ protected ReactPackageTurboModuleManagerDelegate( private void initialize( ReactApplicationContext reactApplicationContext, List packages) { - if (mIsLazy) { - mPackages = packages; - mReactContext = reactApplicationContext; - return; - } - final ReactApplicationContext applicationContext = reactApplicationContext; for (ReactPackage reactPackage : packages) { if (reactPackage instanceof BaseReactPackage) { @@ -154,66 +146,21 @@ private void initialize( @Override public boolean unstable_shouldEnableLegacyModuleInterop() { - if (mIsLazy) { - return false; - } - return mShouldEnableLegacyModuleInterop; } @Override public boolean unstable_shouldRouteTurboModulesThroughLegacyModuleInterop() { - if (mIsLazy) { - return false; - } - return mShouldRouteTurboModulesThroughLegacyModuleInterop; } public boolean unstable_enableSyncVoidMethods() { - if (mIsLazy) { - return false; - } - return mEnableTurboModuleSyncVoidMethods; } @Nullable @Override public TurboModule getModule(String moduleName) { - if (mIsLazy) { - /* - * Returns first TurboModule found with the name received as a parameter. There's no - * warning or error if there are more than one TurboModule registered with the same name in - * different packages. This method relies on the order of insertion of ReactPackage into - * mPackages. Usually the size of mPackages is very small (2 or 3 packages in the majority of - * the cases) - */ - for (ReactPackage reactPackage : mPackages) { - if (reactPackage instanceof BaseReactPackage) { - BaseReactPackage baseReactPackage = (BaseReactPackage) reactPackage; - try { - TurboModule nativeModule = - (TurboModule) baseReactPackage.getModule(moduleName, mReactContext); - if (nativeModule != null) { - return nativeModule; - } - } catch (IllegalArgumentException ex) { - // TODO T170570617: remove this catch statement and let exception bubble up - FLog.e( - ReactConstants.TAG, - ex, - "Caught exception while constructing module '%s'. This was previously ignored but will not be caught in the future.", - moduleName); - } - } else { - throw new IllegalArgumentException( - "ReactPackage must be an instance of TurboReactPackage"); - } - } - return null; - } - NativeModule resolvedModule = null; for (final ModuleProvider moduleProvider : mModuleProviders) { @@ -248,17 +195,8 @@ public TurboModule getModule(String moduleName) { return (TurboModule) resolvedModule; } - @Override - public boolean unstable_isLazyTurboModuleDelegate() { - return mIsLazy; - } - @Override public boolean unstable_isModuleRegistered(String moduleName) { - if (mIsLazy) { - throw new UnsupportedOperationException("unstable_isModuleRegistered is not supported"); - } - for (final ModuleProvider moduleProvider : mModuleProviders) { final ReactModuleInfo moduleInfo = mPackageModuleInfos.get(moduleProvider).get(moduleName); if (moduleInfo != null && moduleInfo.isTurboModule()) { @@ -270,10 +208,6 @@ public boolean unstable_isModuleRegistered(String moduleName) { @Override public boolean unstable_isLegacyModuleRegistered(String moduleName) { - if (mIsLazy) { - return false; - } - for (final ModuleProvider moduleProvider : mModuleProviders) { final ReactModuleInfo moduleInfo = mPackageModuleInfos.get(moduleProvider).get(moduleName); if (moduleInfo != null && !moduleInfo.isTurboModule()) { @@ -286,10 +220,6 @@ public boolean unstable_isLegacyModuleRegistered(String moduleName) { @Nullable @Override public NativeModule getLegacyModule(String moduleName) { - if (mIsLazy) { - throw new UnsupportedOperationException("Legacy Modules are not supported"); - } - if (!unstable_shouldEnableLegacyModuleInterop()) { return null; } @@ -329,11 +259,6 @@ public NativeModule getLegacyModule(String moduleName) { @Override public List getEagerInitModuleNames() { - if (mIsLazy) { - throw new UnsupportedOperationException( - "Running delegate in lazy mode. Please override getEagerInitModuleNames() and return a list of module names that need to be initialized eagerly."); - } - List moduleNames = new ArrayList<>(); for (final ModuleProvider moduleProvider : mModuleProviders) { for (final ReactModuleInfo moduleInfo : mPackageModuleInfos.get(moduleProvider).values()) { 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 ee1b3c44354cb2..8a3d1b586bf273 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 @@ -148,9 +148,6 @@ public class ReactFeatureFlags { /** When enabled, rawProps in Props will not include Yoga specific props. */ public static boolean excludeYogaFromRawProps = false; - /** Enables Stable API for TurboModule (removal of ReactModule, ReactModuleInfoProvider). */ - public static boolean enableTurboModuleStableAPI = false; - /** * When enabled, it uses the modern fork of RuntimeScheduler that allows scheduling tasks with * priorities from any thread. diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultTurboModuleManagerDelegate.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultTurboModuleManagerDelegate.kt index 71af0142cd636e..45ca857cab2401 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultTurboModuleManagerDelegate.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultTurboModuleManagerDelegate.kt @@ -27,7 +27,6 @@ class DefaultTurboModuleManagerDelegate private constructor( context: ReactApplicationContext, packages: List, - private val eagerlyInitializedModules: List, cxxReactPackages: List, ) : ReactPackageTurboModuleManagerDelegate(context, packages, initHybrid(cxxReactPackages)) { @@ -36,26 +35,11 @@ private constructor( "DefaultTurboModuleManagerDelegate.initHybrid() must never be called!") } - override fun getEagerInitModuleNames(): List { - if (unstable_isLazyTurboModuleDelegate()) { - return eagerlyInitializedModules - } - - // Use ReactModuleInfo to get the eager init module names - return super.getEagerInitModuleNames() - } - class Builder : ReactPackageTurboModuleManagerDelegate.Builder() { - private var eagerInitModuleNames: List = emptyList() private var cxxReactPackageProviders: MutableList<((context: ReactApplicationContext) -> CxxReactPackage)> = mutableListOf() - fun setEagerInitModuleNames(eagerInitModuleNames: List): Builder { - this.eagerInitModuleNames = eagerInitModuleNames - return this - } - fun addCxxReactPackage(provider: () -> CxxReactPackage): Builder { this.cxxReactPackageProviders.add({ _ -> provider() }) return this @@ -77,8 +61,7 @@ private constructor( cxxReactPackages.add(cxxReactPackageProvider(context)) } - return DefaultTurboModuleManagerDelegate( - context, packages, eagerInitModuleNames, cxxReactPackages) + return DefaultTurboModuleManagerDelegate(context, packages, cxxReactPackages) } } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/turbomodule/core/TurboModuleManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/turbomodule/core/TurboModuleManager.java index 81887fd80ca9e6..1d7a4397bcedf2 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/turbomodule/core/TurboModuleManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/turbomodule/core/TurboModuleManager.java @@ -194,7 +194,7 @@ private CxxModuleWrapper getTurboLegacyCxxModule(String moduleName) { * This API is invoked from global.__turboModuleProxy. * Only call getModule if the native module is a turbo module. */ - if (!isTurboModuleStableAPIEnabled() && !isTurboModule(moduleName)) { + if (!isTurboModule(moduleName)) { return null; } @@ -204,10 +204,6 @@ private CxxModuleWrapper getTurboLegacyCxxModule(String moduleName) { : null; } - public boolean isTurboModuleStableAPIEnabled() { - return mDelegate != null && mDelegate.unstable_isLazyTurboModuleDelegate(); - } - // used from TurboModuleManager.cpp @SuppressWarnings("unused") @DoNotStrip @@ -221,7 +217,7 @@ private TurboModule getTurboJavaModule(String moduleName) { * This API is invoked from global.__turboModuleProxy. * Only call getModule if the native module is a turbo module. */ - if (!isTurboModuleStableAPIEnabled() && !isTurboModule(moduleName)) { + if (!isTurboModule(moduleName)) { return null; } @@ -252,9 +248,7 @@ public NativeModule getModule(String moduleName) { + "\", but TurboModuleManager was tearing down. Returning null. Was legacy: " + isLegacyModule(moduleName) + ". Was turbo: " - + (isTurboModuleStableAPIEnabled() - ? "[TurboModuleStableAPI enabled for " + moduleName + "]" - : isTurboModule(moduleName)) + + isTurboModule(moduleName) + "."); return null; } @@ -338,9 +332,7 @@ private NativeModule getOrCreateModule( + "\". Was legacy: " + isLegacyModule(moduleName) + ". Was turbo: " - + (isTurboModuleStableAPIEnabled() - ? "[TurboModuleStableAPI enabled for " + moduleName + "]" - : isTurboModule(moduleName)) + + isTurboModule(moduleName) + "."); } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/turbomodule/core/TurboModuleManagerDelegate.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/turbomodule/core/TurboModuleManagerDelegate.java index 7cb82679a3aefa..c80bd0e6e10a6f 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/turbomodule/core/TurboModuleManagerDelegate.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/turbomodule/core/TurboModuleManagerDelegate.java @@ -45,8 +45,6 @@ protected TurboModuleManagerDelegate(HybridData hybridData) { public abstract boolean unstable_isModuleRegistered(String moduleName); - public abstract boolean unstable_isLazyTurboModuleDelegate(); - /** * Create an return a legacy NativeModule with name `moduleName`. If `moduleName` is a * TurboModule, return null.