Skip to content

Commit

Permalink
Java: Make TurboModuleManager's APIs use NativeModule interface (#36629)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #36629

The scope of TurboModuleManager is increasing:
- Eventually, it'll be capable of creating interop NativeModules (i.e: NativeModules that don't implement TurboModule).

So, instead of creating duplicate methods for NativeModules on the TurboModuleManager, this diff changes the APIs of TurboModuleManager to work with the NativeModule interface.

Thoughts?

## Questions
**Question:** Is this a breaking change for open source?
- Technically, yes. This diff changes the public interface of TurboModuleManager.

**Question:** How large of a thrash will this cause for open source apps?
- The thrash should be minimal. People in open source shouldn't be creating their own TurboModuleManager. They also shouldn't be directly accessing the TurboModuleManager object either.

**Question:** Is this change safe?
- Yeah. All the code that calls into TurboModuleRegistry converts TurboModules it returns into NativeModules.

**Question:** Is this change move us in the right direction?
- Long term, the TurboModule system will support legacy modules as well as TurboModules.
- I think it makes a lot of sense to have one Java-facing registry: after all, Java will just treat these NativeModules/TurboModules as regular Java objects, and call public methods on them. It doesn't care if the module is TurboModule-compatible or not.
- As for the TurboModuleRegistry abstraction, I think we should eventually rename this to NativeModuleRegistry after we delete the current NativeModuleRegistry.
- Still thinking about this though. I will leave this diff in review to welcome comments.

Changelog: [Android][Deprecated] - Deprecate TurboModuleRegistry.getModule(), getModules(), hasModule(),

Reviewed By: mdvacca

Differential Revision: D43801531

fbshipit-source-id: 4af7cbc2e2dc7c1d664acbd38c83aa93aae23c9f
  • Loading branch information
RSNara authored and facebook-github-bot committed Mar 28, 2023
1 parent cb07358 commit 3af66bf
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import com.facebook.react.config.ReactFeatureFlags;
import com.facebook.react.module.annotations.ReactModule;
import com.facebook.react.turbomodule.core.CallInvokerHolderImpl;
import com.facebook.react.turbomodule.core.interfaces.TurboModule;
import com.facebook.react.turbomodule.core.interfaces.TurboModuleRegistry;
import com.facebook.systrace.Systrace;
import com.facebook.systrace.TraceListener;
Expand Down Expand Up @@ -458,7 +457,7 @@ public <T extends JavaScriptModule> T getJSModule(Class<T> jsInterface) {
@Override
public <T extends NativeModule> boolean hasNativeModule(Class<T> nativeModuleInterface) {
String moduleName = getNameFromAnnotation(nativeModuleInterface);
return getTurboModuleRegistry() != null && getTurboModuleRegistry().hasModule(moduleName)
return getTurboModuleRegistry() != null && getTurboModuleRegistry().hasNativeModule(moduleName)
? true
: mNativeModuleRegistry.hasModule(moduleName);
}
Expand All @@ -483,9 +482,9 @@ private TurboModuleRegistry getTurboModuleRegistry() {
@Nullable
public NativeModule getNativeModule(String moduleName) {
if (getTurboModuleRegistry() != null) {
TurboModule turboModule = getTurboModuleRegistry().getModule(moduleName);
if (turboModule != null) {
return (NativeModule) turboModule;
NativeModule module = getTurboModuleRegistry().getNativeModule(moduleName);
if (module != null) {
return module;
}
}

Expand All @@ -510,8 +509,8 @@ public Collection<NativeModule> getNativeModules() {
nativeModules.addAll(mNativeModuleRegistry.getAllModules());

if (getTurboModuleRegistry() != null) {
for (TurboModule turboModule : getTurboModuleRegistry().getModules()) {
nativeModules.add((NativeModule) turboModule);
for (NativeModule module : getTurboModuleRegistry().getNativeModules()) {
nativeModules.add(module);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,39 +105,39 @@ public List<String> getEagerInitModuleNames() {
@DoNotStrip
@Nullable
private CxxModuleWrapper getLegacyCxxModule(String moduleName) {
final TurboModule turboModule = getModule(moduleName);
if (!(turboModule instanceof CxxModuleWrapper)) {
return null;
}

return (CxxModuleWrapper) turboModule;
final NativeModule module = getNativeModule(moduleName);
return module instanceof CxxModuleWrapper ? (CxxModuleWrapper) module : null;
}

@DoNotStrip
@Nullable
private TurboModule getJavaModule(String moduleName) {
final TurboModule turboModule = getModule(moduleName);
if (turboModule instanceof CxxModuleWrapper) {
return null;
}
final NativeModule module = getNativeModule(moduleName);
return !(module instanceof CxxModuleWrapper) && module instanceof TurboModule
? (TurboModule) module
: null;
}

return turboModule;
@Deprecated
public TurboModule getModule(String moduleName) {
NativeModule module = getNativeModule(moduleName);
return module instanceof TurboModule ? (TurboModule) module : null;
}

/**
* Return the TurboModule instance that corresponds to the provided moduleName.
* Return the NativeModule instance that corresponds to the provided moduleName.
*
* <p>This method: - Creates and initializes the module if it doesn't already exist. - Returns
* null after TurboModuleManager has been torn down.
*/
@Nullable
public TurboModule getModule(String moduleName) {
public NativeModule getNativeModule(String moduleName) {
ModuleHolder moduleHolder;

synchronized (mModuleCleanupLock) {
if (mModuleCleanupStarted) {
/*
* Always return null after cleanup has started, so that getModule(moduleName) returns null.
* Always return null after cleanup has started, so that getNativeModule(moduleName) returns null.
*/
return null;
}
Expand All @@ -154,7 +154,7 @@ public TurboModule getModule(String moduleName) {
}

TurboModulePerfLogger.moduleCreateStart(moduleName, moduleHolder.getModuleId());
TurboModule module = (TurboModule) getOrCreateModule(moduleName, moduleHolder, true);
NativeModule module = getOrCreateNativeModule(moduleName, moduleHolder, true);

if (module != null) {
TurboModulePerfLogger.moduleCreateEnd(moduleName, moduleHolder.getModuleId());
Expand All @@ -172,7 +172,7 @@ public TurboModule getModule(String moduleName) {
* first thread creates x. All n - 1 other threads wait until the x is created and initialized.
*/
@Nullable
private NativeModule getOrCreateModule(
private NativeModule getOrCreateNativeModule(
String moduleName, @NonNull ModuleHolder moduleHolder, boolean shouldPerfLog) {
boolean shouldCreateModule = false;

Expand Down Expand Up @@ -249,32 +249,63 @@ private NativeModule getOrCreateModule(
}
}

/** Which TurboModules have been created? */
@Deprecated
public Collection<TurboModule> getModules() {
final Collection<TurboModule> modules = new ArrayList<>();

for (final NativeModule module : getNativeModules()) {
if (module instanceof TurboModule) {
modules.add((TurboModule) module);
}
}

return modules;
}

/** Which NativeModules have been created? */
public Collection<NativeModule> getNativeModules() {
final List<ModuleHolder> moduleHolders = new ArrayList<>();
synchronized (mModuleCleanupLock) {
moduleHolders.addAll(mModuleHolders.values());
}

final List<TurboModule> turboModules = new ArrayList<>();
final List<NativeModule> modules = new ArrayList<>();
for (final ModuleHolder moduleHolder : moduleHolders) {
synchronized (moduleHolder) {
// No need to wait for the TurboModule to finish being created and initialized
if (moduleHolder.getModule() != null) {
turboModules.add((TurboModule) moduleHolder.getModule());
modules.add(moduleHolder.getModule());
}
}
}

return turboModules;
return modules;
}

@Deprecated
public boolean hasModule(String moduleName) {
ModuleHolder moduleHolder;
synchronized (mModuleCleanupLock) {
moduleHolder = mModuleHolders.get(moduleName);
}

if (moduleHolder != null) {
synchronized (moduleHolder) {
if (moduleHolder.getModule() instanceof TurboModule) {
return true;
}
}
}

return false;
}

public boolean hasNativeModule(String moduleName) {
ModuleHolder moduleHolder;
synchronized (mModuleCleanupLock) {
moduleHolder = mModuleHolders.get(moduleName);
}

if (moduleHolder != null) {
synchronized (moduleHolder) {
if (moduleHolder.getModule() != null) {
Expand Down Expand Up @@ -321,7 +352,7 @@ public void onCatalystInstanceDestroy() {
* initialized. In this case, we should wait for initialization to complete, before destroying
* the TurboModule.
*/
final NativeModule nativeModule = getOrCreateModule(moduleName, moduleHolder, false);
final NativeModule nativeModule = getOrCreateNativeModule(moduleName, moduleHolder, false);

if (nativeModule != null) {
nativeModule.invalidate();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
load("//tools/build_defs/oss:rn_defs.bzl", "react_native_dep", "rn_android_library")
load("//tools/build_defs/oss:rn_defs.bzl", "react_native_dep", "react_native_target", "rn_android_library")

rn_android_library(
name = "interfaces",
Expand All @@ -15,6 +15,7 @@ rn_android_library(
"PUBLIC",
],
deps = [
react_native_target("java/com/facebook/react/bridge:interfaces"),
react_native_dep("third-party/android/androidx:annotation"),
react_native_dep("third-party/java/jsr-305:jsr-305"),
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,48 @@
package com.facebook.react.turbomodule.core.interfaces;

import androidx.annotation.Nullable;
import com.facebook.react.bridge.NativeModule;
import java.util.Collection;
import java.util.List;

/** Interface to allow for creating and retrieving TurboModules. */
/**
* Interface to allow for creating and retrieving NativeModules. Why is this this class prefixed
* with "Turbo", even though it supports both legacy NativeModules, and TurboModules? Because there
* already is a NativeModuleRegistry (a part of the legacy architecture). Once that class is
* deleted, we should rename this interface accordingly.
*/
public interface TurboModuleRegistry {

/**
* Return the TurboModule instance that has that name `moduleName`. If the `moduleName`
* TurboModule hasn't been instantiated, instantiate it. If no TurboModule is registered under
* `moduleName`, return null.
*/
@Deprecated
@Nullable
TurboModule getModule(String moduleName);

/** Get all instantiated TurboModules. */
@Deprecated
Collection<TurboModule> getModules();

/** Has the TurboModule with name `moduleName` been instantiated? */
@Deprecated
boolean hasModule(String moduleName);

/**
* Return the NativeModule instance that has that name `moduleName`. If the `moduleName`
* NativeModule hasn't been instantiated, instantiate it. If no NativeModule is registered under
* `moduleName`, return null.
*/
@Nullable
NativeModule getNativeModule(String moduleName);

/** Get all instantiated NativeModule. */
Collection<NativeModule> getNativeModules();

/** Has the NativeModule with name `moduleName` been instantiated? */
boolean hasNativeModule(String moduleName);

/**
* Return the names of all the NativeModules that are supposed to be eagerly initialized. By
* calling getModule on each name, this allows the application to eagerly initialize its
Expand Down

0 comments on commit 3af66bf

Please sign in to comment.