From f312d6eccda7ceb8a01da8eac332a126c70dac1e Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Mon, 21 Aug 2023 15:44:07 -0700 Subject: [PATCH] Ensure TM system has consistent view of interop flags (#39086) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/39086 Our largest blocker for the TurboModule interop layer is a "module not found" issue. **Hypothesis:** This is a gating-related bug. ## Changes This diff tries to simplify the gating of the TurboModule interop layer: Instead of reading the flags again and again from two different classes (the module manager and its delegate), just read the flags once, when the module system is initialized: https://www.internalfb.com/code/fbsource/[ae79b760626ec81ceadbf2829e1593199d4df031]/xplat/js/react-native-github/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/ReactInstance.java?lines=106-113%2C210-215%2C217-223%2C251 This will ensure that the TurboModule system has one consistent view of the interop layer flags, throughout its lifetime. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D48489274 fbshipit-source-id: 05eb64c5f7bd89dd65aac7390c3eb09234d87f96 --- ...eactPackageTurboModuleManagerDelegate.java | 26 ++++++++++++----- .../turbomodule/core/TurboModuleManager.java | 29 +++++++++---------- .../core/TurboModuleManagerDelegate.java | 13 +++++++++ 3 files changed, 45 insertions(+), 23 deletions(-) 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 fccc59ced74..fff2a356717 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 @@ -34,14 +34,26 @@ public abstract class ReactPackageTurboModuleManagerDelegate extends TurboModule private final Map> mPackageModuleInfos = new HashMap<>(); - private static boolean shouldSupportLegacyPackages() { - return ReactFeatureFlags.enableBridgelessArchitecture - && ReactFeatureFlags.unstable_useTurboModuleInterop; + private final boolean mShouldEnableLegacyModuleInterop = + ReactFeatureFlags.enableBridgelessArchitecture + && ReactFeatureFlags.unstable_useTurboModuleInterop; + + private final boolean mShouldRouteTurboModulesThroughLegacyModuleInterop = + mShouldEnableLegacyModuleInterop + && ReactFeatureFlags.unstable_useTurboModuleInteropForAllTurboModules; + + @Override + public boolean unstable_shouldEnableLegacyModuleInterop() { + return mShouldEnableLegacyModuleInterop; } - private static boolean shouldCreateLegacyModules() { - return ReactFeatureFlags.enableBridgelessArchitecture - && ReactFeatureFlags.unstable_useTurboModuleInterop; + @Override + public boolean unstable_shouldRouteTurboModulesThroughLegacyModuleInterop() { + return mShouldRouteTurboModulesThroughLegacyModuleInterop; + } + + private boolean shouldSupportLegacyPackages() { + return unstable_shouldEnableLegacyModuleInterop(); } protected ReactPackageTurboModuleManagerDelegate( @@ -191,7 +203,7 @@ public abstract class ReactPackageTurboModuleManagerDelegate extends TurboModule @Nullable @Override public NativeModule getLegacyModule(String moduleName) { - if (!shouldCreateLegacyModules()) { + if (!unstable_shouldEnableLegacyModuleInterop()) { return null; } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManager.java index 161476dcc5b..20a3d8e280f 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManager.java @@ -20,7 +20,6 @@ import com.facebook.react.bridge.NativeModule; import com.facebook.react.bridge.ReactNoCrashSoftException; import com.facebook.react.bridge.ReactSoftExceptionLogger; import com.facebook.react.bridge.RuntimeExecutor; -import com.facebook.react.config.ReactFeatureFlags; import com.facebook.react.turbomodule.core.interfaces.CallInvokerHolder; import com.facebook.react.turbomodule.core.interfaces.NativeMethodCallInvokerHolder; import com.facebook.react.turbomodule.core.interfaces.TurboModule; @@ -72,7 +71,7 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry { (CallInvokerHolderImpl) jsCallInvokerHolder, (NativeMethodCallInvokerHolderImpl) nativeMethodCallInvokerHolder, delegate); - installJSIBindings(shouldCreateLegacyModules()); + installJSIBindings(shouldEnableLegacyModuleInterop()); mEagerInitModuleNames = delegate == null ? new ArrayList<>() : delegate.getEagerInitModuleNames(); @@ -85,7 +84,7 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry { : moduleName -> (NativeModule) delegate.getModule(moduleName); mLegacyModuleProvider = - delegate == null || !shouldCreateLegacyModules() + delegate == null || !shouldEnableLegacyModuleInterop() ? nullProvider : moduleName -> { NativeModule nativeModule = delegate.getLegacyModule(moduleName); @@ -108,15 +107,13 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry { return mDelegate != null && mDelegate.unstable_isLegacyModuleRegistered(moduleName); } - private static boolean shouldCreateLegacyModules() { - return ReactFeatureFlags.enableBridgelessArchitecture - && ReactFeatureFlags.unstable_useTurboModuleInterop; + private boolean shouldEnableLegacyModuleInterop() { + return mDelegate != null && mDelegate.unstable_shouldEnableLegacyModuleInterop(); } - private static boolean shouldRouteTurboModulesThroughInteropLayer() { - return ReactFeatureFlags.enableBridgelessArchitecture - && ReactFeatureFlags.unstable_useTurboModuleInterop - && ReactFeatureFlags.unstable_useTurboModuleInteropForAllTurboModules; + private boolean shouldRouteTurboModulesThroughLegacyModuleInterop() { + return mDelegate != null + && mDelegate.unstable_shouldRouteTurboModulesThroughLegacyModuleInterop(); } @Override @@ -134,7 +131,7 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry { @DoNotStrip @Nullable private NativeModule getLegacyJavaModule(String moduleName) { - if (shouldRouteTurboModulesThroughInteropLayer()) { + if (shouldRouteTurboModulesThroughLegacyModuleInterop()) { final NativeModule module = getModule(moduleName); return !(module instanceof CxxModuleWrapper) ? module : null; } @@ -157,7 +154,7 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry { @DoNotStrip @Nullable private CxxModuleWrapper getLegacyCxxModule(String moduleName) { - if (shouldRouteTurboModulesThroughInteropLayer()) { + if (shouldRouteTurboModulesThroughLegacyModuleInterop()) { final NativeModule module = getModule(moduleName); return module instanceof CxxModuleWrapper ? (CxxModuleWrapper) module : null; } @@ -180,7 +177,7 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry { @DoNotStrip @Nullable private CxxModuleWrapper getTurboLegacyCxxModule(String moduleName) { - if (shouldRouteTurboModulesThroughInteropLayer()) { + if (shouldRouteTurboModulesThroughLegacyModuleInterop()) { return null; } @@ -201,7 +198,7 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry { @DoNotStrip @Nullable private TurboModule getTurboJavaModule(String moduleName) { - if (shouldRouteTurboModulesThroughInteropLayer()) { + if (shouldRouteTurboModulesThroughLegacyModuleInterop()) { return null; } @@ -398,9 +395,9 @@ public class TurboModuleManager implements JSIModule, TurboModuleRegistry { return false; } - private static void logError(String message) { + private void logError(String message) { FLog.e("TurboModuleManager", message); - if (shouldRouteTurboModulesThroughInteropLayer()) { + if (shouldRouteTurboModulesThroughLegacyModuleInterop()) { ReactSoftExceptionLogger.logSoftException( "TurboModuleManager", new ReactNoCrashSoftException(message)); } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManagerDelegate.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManagerDelegate.java index 5e83bb5d8e6..38986df7d8a 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManagerDelegate.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManagerDelegate.java @@ -57,5 +57,18 @@ public abstract class TurboModuleManagerDelegate { return new ArrayList<>(); } + /** Can the TurboModule system create legacy modules? */ + public boolean unstable_shouldEnableLegacyModuleInterop() { + return false; + } + + /** + * Should the TurboModule system treat all turbo native modules as though they were legacy + * modules? This method is for testing purposes only. + */ + public boolean unstable_shouldRouteTurboModulesThroughLegacyModuleInterop() { + return false; + } + protected synchronized void maybeLoadOtherSoLibraries() {} }