Facebook/react-native
1
0
Fork 0
mirror of https://github.com/facebook/react-native.git synced 2025-11-01 09:14:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

Simplify the gating for the Fb4a TurboModule interop test (#36985)

Browse Source 
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/36985

This diff simplifies TurboModuleManagerDelegate. Now, it doesn't do any filtering.

What TurboModuleManagerDelegate does:
- If the TurboModule interop layer is on, getLegacyModule(moduleName) starts returning legacy modules.
- If the TurboModule interop layer is off, getLegacyModule(moduleName) returns null.

This should help with T144183369. In that crash, some native modules are returned as null. And complicated filtering could be a contributor to that problem. So, simplifying filtering might mitigate that issue.

Even if it doesn't, this makes the TurboModuleManager easier to understand, which makes root causing/mitigating that problem easier.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D45131297

fbshipit-source-id: 9e37804e09c9f04ab0610f5b444574ae65c9d638
This commit is contained in:
Ramanpreet Nara
2023-04-20 11:42:21 -07:00
committed by Facebook GitHub Bot
parent 53d3629a65
commit 0c1b7d374c
2 changed files with 55 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactPackageTurboModuleManagerDelegate.java
+4 -13
View File
@@ -44,12 +44,6 @@ public abstract class ReactPackageTurboModuleManagerDelegate extends TurboModule
&& ReactFeatureFlags.unstable_useTurboModuleInterop;
}
private static boolean shouldRouteTurboModulesThroughInteropLayer() {
return ReactFeatureFlags.enableBridgelessArchitecture
&& ReactFeatureFlags.unstable_useTurboModuleInterop
&& ReactFeatureFlags.unstable_useTurboModuleInteropForAllTurboModules;
}
protected ReactPackageTurboModuleManagerDelegate(
ReactApplicationContext reactApplicationContext, List<ReactPackage> packages) {
super();
@@ -152,10 +146,6 @@ public abstract class ReactPackageTurboModuleManagerDelegate extends TurboModule
@Nullable
@Override
public TurboModule getModule(String moduleName) {
if (shouldRouteTurboModulesThroughInteropLayer()) {
return null;
}
NativeModule resolvedModule = null;
for (final ModuleProvider moduleProvider : mModuleProviders) {
@@ -181,7 +171,8 @@ public abstract class ReactPackageTurboModuleManagerDelegate extends TurboModule
}
// Skip TurboModule-incompatible modules
if (!(resolvedModule instanceof TurboModule)) {
boolean isLegacyModule = !(resolvedModule instanceof TurboModule);
if (isLegacyModule) {
return null;
}
@@ -201,7 +192,7 @@ public abstract class ReactPackageTurboModuleManagerDelegate extends TurboModule
try {
final ReactModuleInfo moduleInfo = mPackageModuleInfos.get(moduleProvider).get(moduleName);
if (moduleInfo != null
&& (!moduleInfo.isTurboModule() || shouldRouteTurboModulesThroughInteropLayer())
&& !moduleInfo.isTurboModule()
&& (resolvedModule == null || moduleInfo.canOverrideExistingModule())) {
final NativeModule module = moduleProvider.getModule(moduleName);
@@ -221,7 +212,7 @@ public abstract class ReactPackageTurboModuleManagerDelegate extends TurboModule
// Skip TurboModule-compatible modules
boolean isLegacyModule = !(resolvedModule instanceof TurboModule);
if (!(isLegacyModule || shouldRouteTurboModulesThroughInteropLayer())) {
if (!isLegacyModule) {
return null;
}
packages/react-native/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManager.java
+51 -45
View File
@@ -36,8 +36,8 @@ import java.util.Map;
public class TurboModuleManager implements JSIModule, TurboModuleRegistry {
private static volatile boolean sIsSoLibraryLoaded;
private final List<String> mEagerInitModuleNames;
private final ModuleProvider<TurboModule> mModuleProvider;
private final ModuleProvider<NativeModule> mLegacyModuleProvider;
private final ModuleProvider mTurboModuleProvider;
private final ModuleProvider mLegacyModuleProvider;
// Prevents the creation of new TurboModules once cleanup as been initiated.
private final Object mModuleCleanupLock = new Object();
@@ -68,47 +68,43 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry {
installJSIBindings(shouldCreateLegacyModules());
mEagerInitModuleNames =
delegate == null ? new ArrayList<String>() : delegate.getEagerInitModuleNames();
delegate == null ? new ArrayList<>() : delegate.getEagerInitModuleNames();
mModuleProvider =
moduleName -> {
if (delegate == null || shouldRouteTurboModulesThroughInteropLayer()) {
return null;
}
ModuleProvider nullProvider = moduleName -> null;
TurboModule module = delegate.getModule(moduleName);
if (module == null) {
CxxModuleWrapper legacyCxxModule = delegate.getLegacyCxxModule(moduleName);
mTurboModuleProvider =
delegate == null
? nullProvider
: moduleName -> {
NativeModule module = (NativeModule) delegate.getModule(moduleName);
if (module == null) {
CxxModuleWrapper legacyCxxModule = delegate.getLegacyCxxModule(moduleName);
if (legacyCxxModule != null) {
// TurboModuleManagerDelegate.getLegacyCxxModule() must always return TurboModules
Assertions.assertCondition(
legacyCxxModule instanceof TurboModule,
"CxxModuleWrapper \"" + moduleName + "\" is not a TurboModule");
module = (TurboModule) legacyCxxModule;
}
}
return module;
};
if (legacyCxxModule != null) {
// TurboModuleManagerDelegate.getLegacyCxxModule() must always return TurboModules
Assertions.assertCondition(
legacyCxxModule instanceof TurboModule,
"CxxModuleWrapper \"" + moduleName + "\" is not a TurboModule");
return legacyCxxModule;
}
}
return module;
};
mLegacyModuleProvider =
moduleName -> {
if (delegate == null || !shouldCreateLegacyModules()) {
return null;
}
NativeModule nativeModule = delegate.getLegacyModule(moduleName);
if (nativeModule != null) {
if (!shouldRouteTurboModulesThroughInteropLayer()) {
// TurboModuleManagerDelegate.getLegacyModule must never return a TurboModule
Assertions.assertCondition(
!(nativeModule instanceof TurboModule),
"NativeModule \"" + moduleName + "\" is a TurboModule");
}
return nativeModule;
}
return nativeModule;
};
delegate == null || !shouldCreateLegacyModules()
? nullProvider
: moduleName -> {
NativeModule nativeModule = delegate.getLegacyModule(moduleName);
if (nativeModule != null) {
// TurboModuleManagerDelegate.getLegacyModule must never return a TurboModule
Assertions.assertCondition(
!(nativeModule instanceof TurboModule),
"NativeModule \"" + moduleName + "\" is a TurboModule");
return nativeModule;
}
return null;
};
}
private static boolean shouldCreateLegacyModules() {
@@ -136,8 +132,12 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry {
@Nullable
private NativeModule getLegacyJavaModule(String moduleName) {
final NativeModule module = getNativeModule(moduleName);
return !(module instanceof CxxModuleWrapper)
&& (shouldRouteTurboModulesThroughInteropLayer() || !(module instanceof TurboModule))
if (shouldRouteTurboModulesThroughInteropLayer()) {
return !(module instanceof CxxModuleWrapper) ? module : null;
}
return !(module instanceof CxxModuleWrapper) && !(module instanceof TurboModule)
? module
: null;
}
@@ -147,8 +147,12 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry {
@Nullable
private CxxModuleWrapper getLegacyCxxModule(String moduleName) {
final NativeModule module = getNativeModule(moduleName);
return module instanceof CxxModuleWrapper
&& (shouldRouteTurboModulesThroughInteropLayer() || !(module instanceof TurboModule))
if (shouldRouteTurboModulesThroughInteropLayer()) {
return module instanceof CxxModuleWrapper ? (CxxModuleWrapper) module : null;
}
return module instanceof CxxModuleWrapper && !(module instanceof TurboModule)
? (CxxModuleWrapper) module
: null;
}
@@ -160,6 +164,7 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry {
if (shouldRouteTurboModulesThroughInteropLayer()) {
return null;
}
final NativeModule module = getNativeModule(moduleName);
return module instanceof CxxModuleWrapper && module instanceof TurboModule
? (CxxModuleWrapper) module
@@ -172,6 +177,7 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry {
if (shouldRouteTurboModulesThroughInteropLayer()) {
return null;
}
final NativeModule module = getNativeModule(moduleName);
return !(module instanceof CxxModuleWrapper) && module instanceof TurboModule
? (TurboModule) module
@@ -254,7 +260,7 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry {
if (shouldCreateModule) {
TurboModulePerfLogger.moduleCreateConstructStart(moduleName, moduleHolder.getModuleId());
NativeModule nativeModule = (NativeModule) mModuleProvider.getModule(moduleName);
NativeModule nativeModule = mTurboModuleProvider.getModule(moduleName);
if (nativeModule == null) {
nativeModule = mLegacyModuleProvider.getModule(moduleName);
@@ -476,8 +482,8 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry {
}
}
private interface ModuleProvider<T> {
private interface ModuleProvider {
@Nullable
T getModule(String name);
NativeModule getModule(String name);
}
}