Skip to content

Commit

Permalink
Revert experiment that disables NativeModule codegen (#41978)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #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
  • Loading branch information
mdvacca authored and facebook-github-bot committed Jan 12, 2024
1 parent 3690e44 commit 0b14eed
Show file tree
Hide file tree
Showing 6 changed files with 6 additions and 115 deletions.
6 changes: 1 addition & 5 deletions packages/react-native/ReactAndroid/api/ReactAndroid.api
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 <init> (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 <init> (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;
}

Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ interface ModuleProvider {
private final boolean mEnableTurboModuleSyncVoidMethods =
ReactFeatureFlags.unstable_enableTurboModuleSyncVoidMethods;

private final boolean mIsLazy = ReactFeatureFlags.enableTurboModuleStableAPI;

// Lazy Props
private List<ReactPackage> mPackages;
private ReactApplicationContext mReactContext;
Expand All @@ -70,12 +68,6 @@ protected ReactPackageTurboModuleManagerDelegate(

private void initialize(
ReactApplicationContext reactApplicationContext, List<ReactPackage> packages) {
if (mIsLazy) {
mPackages = packages;
mReactContext = reactApplicationContext;
return;
}

final ReactApplicationContext applicationContext = reactApplicationContext;
for (ReactPackage reactPackage : packages) {
if (reactPackage instanceof BaseReactPackage) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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()) {
Expand All @@ -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()) {
Expand All @@ -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;
}
Expand Down Expand Up @@ -329,11 +259,6 @@ public NativeModule getLegacyModule(String moduleName) {

@Override
public List<String> 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<String> moduleNames = new ArrayList<>();
for (final ModuleProvider moduleProvider : mModuleProviders) {
for (final ReactModuleInfo moduleInfo : mPackageModuleInfos.get(moduleProvider).values()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ class DefaultTurboModuleManagerDelegate
private constructor(
context: ReactApplicationContext,
packages: List<ReactPackage>,
private val eagerlyInitializedModules: List<String>,
cxxReactPackages: List<CxxReactPackage>,
) : ReactPackageTurboModuleManagerDelegate(context, packages, initHybrid(cxxReactPackages)) {

Expand All @@ -36,26 +35,11 @@ private constructor(
"DefaultTurboModuleManagerDelegate.initHybrid() must never be called!")
}

override fun getEagerInitModuleNames(): List<String> {
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<String> = emptyList()
private var cxxReactPackageProviders:
MutableList<((context: ReactApplicationContext) -> CxxReactPackage)> =
mutableListOf()

fun setEagerInitModuleNames(eagerInitModuleNames: List<String>): Builder {
this.eagerInitModuleNames = eagerInitModuleNames
return this
}

fun addCxxReactPackage(provider: () -> CxxReactPackage): Builder {
this.cxxReactPackageProviders.add({ _ -> provider() })
return this
Expand All @@ -77,8 +61,7 @@ private constructor(
cxxReactPackages.add(cxxReactPackageProvider(context))
}

return DefaultTurboModuleManagerDelegate(
context, packages, eagerInitModuleNames, cxxReactPackages)
return DefaultTurboModuleManagerDelegate(context, packages, cxxReactPackages)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -338,9 +332,7 @@ private NativeModule getOrCreateModule(
+ "\". Was legacy: "
+ isLegacyModule(moduleName)
+ ". Was turbo: "
+ (isTurboModuleStableAPIEnabled()
? "[TurboModuleStableAPI enabled for " + moduleName + "]"
: isTurboModule(moduleName))
+ isTurboModule(moduleName)
+ ".");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 0b14eed

Please sign in to comment.