Deprecate TurboModuleManagerDelegate.getLegacyCxxModule (#36667)

Summary: Pull Request resolved: #36667 ## Context TurboModuleManagerDelegate exposes two methods that create TurboModules: - TurboModule getModule() - CxxModuleWrapper getLegacyCxxModule() ## Problem TurboModuleManagerDelegate.getLegacyCxxModule() is redundant: getModule() could just return all the modules that getLegacyCxxModule() returns: getLegacyCxxModule returns modules that implement TurboModule. ## Changes So, let's deprecate getLegacyCxxModule(). This will simplify the implementation of TurboModuleManager. Changelog: [Android][Deprecated] - Deprecate TurboModuleManager.getLegacyCxxModule Reviewed By: cortinico Differential Revision: D44407802 fbshipit-source-id: 88a6cf6597db76d8a74fd777d68ccf4f43aa6811
facebook · Mar 28, 2023 · 7a08fbb · 7a08fbb
1 parent 3af66bf
commit 7a08fbb
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 61 deletions.
diff --git a/...ReactAndroid/src/main/java/com/facebook/react/ReactPackageTurboModuleManagerDelegate.java b/...ReactAndroid/src/main/java/com/facebook/react/ReactPackageTurboModuleManagerDelegate.java
@@ -9,7 +9,6 @@
 
 import androidx.annotation.Nullable;
 import com.facebook.infer.annotation.Assertions;
-import com.facebook.proguard.annotations.DoNotStrip;
 import com.facebook.react.bridge.CxxModuleWrapper;
 import com.facebook.react.bridge.ModuleSpec;
 import com.facebook.react.bridge.NativeModule;
@@ -142,36 +141,6 @@ public NativeModule getModule(String moduleName) {
   @Nullable
   @Override
   public TurboModule getModule(String moduleName) {
-    TurboModule module = resolveModule(moduleName);
-    if (module == null) {
-      return null;
-    }
-
-    if (module instanceof CxxModuleWrapper) {
-      return null;
-    }
-
-    return module;
-  }
-
-  @Nullable
-  @Override
-  @DoNotStrip
-  public CxxModuleWrapper getLegacyCxxModule(String moduleName) {
-    TurboModule module = resolveModule(moduleName);
-    if (module == null) {
-      return null;
-    }
-
-    if (!(module instanceof CxxModuleWrapper)) {
-      return null;
-    }
-
-    return (CxxModuleWrapper) module;
-  }
-
-  @Nullable
-  private TurboModule resolveModule(String moduleName) {
     NativeModule resolvedModule = null;
 
     for (final ModuleProvider moduleProvider : mModuleProviders) {
@@ -203,6 +172,13 @@ private TurboModule resolveModule(String moduleName) {
     return null;
   }
 
+  @Deprecated
+  @Nullable
+  @Override
+  public CxxModuleWrapper getLegacyCxxModule(String moduleName) {
+    return null;
+  }
+
   @Override
   public List<String> getEagerInitModuleNames() {
     List<String> moduleNames = new ArrayList<>();

diff --git a/...ve/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManager.java b/...ve/ReactAndroid/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManager.java
@@ -31,8 +31,7 @@
 public class TurboModuleManager implements JSIModule, TurboModuleRegistry {
   private static volatile boolean sIsSoLibraryLoaded;
   private final List<String> mEagerInitModuleNames;
-  private final ModuleProvider<TurboModule> mJavaModuleProvider;
-  private final ModuleProvider<TurboModule> mCxxModuleProvider;
+  private final ModuleProvider<TurboModule> mModuleProvider;
 
   // Prevents the creation of new TurboModules once cleanup as been initiated.
   private final Object mModuleCleanupLock = new Object();
@@ -65,35 +64,27 @@ public TurboModuleManager(
     mEagerInitModuleNames =
         delegate == null ? new ArrayList<String>() : delegate.getEagerInitModuleNames();
 
-    mJavaModuleProvider =
+    mModuleProvider =
         new ModuleProvider<TurboModule>() {
           @Nullable
           public TurboModule getModule(String moduleName) {
             if (delegate == null) {
               return null;
             }
 
-            return delegate.getModule(moduleName);
-          }
-        };
-
-    mCxxModuleProvider =
-        new ModuleProvider<TurboModule>() {
-          @Nullable
-          public TurboModule getModule(String moduleName) {
-            if (delegate == null) {
-              return null;
-            }
-
-            CxxModuleWrapper nativeModule = delegate.getLegacyCxxModule(moduleName);
-            if (nativeModule != null) {
-              // TurboModuleManagerDelegate must always return TurboModules
-              Assertions.assertCondition(
-                  nativeModule instanceof TurboModule,
-                  "CxxModuleWrapper \"" + moduleName + "\" is not a TurboModule");
-              return (TurboModule) nativeModule;
+            TurboModule module = 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 null;
+            return module;
           }
         };
   }
@@ -194,11 +185,7 @@ private NativeModule getOrCreateNativeModule(
 
     if (shouldCreateModule) {
       TurboModulePerfLogger.moduleCreateConstructStart(moduleName, moduleHolder.getModuleId());
-      NativeModule nativeModule = (NativeModule) mJavaModuleProvider.getModule(moduleName);
-
-      if (nativeModule == null) {
-        nativeModule = (NativeModule) mCxxModuleProvider.getModule(moduleName);
-      }
+      NativeModule nativeModule = (NativeModule) mModuleProvider.getModule(moduleName);
 
       TurboModulePerfLogger.moduleCreateConstructEnd(moduleName, moduleHolder.getModuleId());
       TurboModulePerfLogger.moduleCreateSetUpStart(moduleName, moduleHolder.getModuleId());

diff --git a/...Android/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManagerDelegate.java b/...Android/src/main/java/com/facebook/react/turbomodule/core/TurboModuleManagerDelegate.java
@@ -39,9 +39,12 @@ protected TurboModuleManagerDelegate() {
   public abstract TurboModule getModule(String moduleName);
 
   /**
-   * Create an return a CxxModuleWrapper NativeModule with name `moduleName`. If `moduleName` isn't
-   * a CxxModule, return null.
+   * Create and return a CxxModuleWrapper NativeModule with name `moduleName`. If `moduleName` isn't
+   * a CxxModule, return null. CxxModuleWrapper must implement TurboModule.
+   *
+   * <p>Deprecated. Please just return your CxxModuleWrappers from getModule.
    */
+  @Deprecated
   @Nullable
   public abstract CxxModuleWrapper getLegacyCxxModule(String moduleName);