Skip to content

Commit

Permalink
Refactor TurboModule filtering in NativeModuleRegistryBuilder
Browse files Browse the repository at this point in the history
Summary:
## Context
`NativeModuleRegistryBuilder` calls `TurboReactPackage.getNativeModuleIterator()` to access ModuleHolders for all the NativeModules in the `TurboReactPackage`. We then filter out the ModuleHolders that contain `TurboModules`, before using that list to make create the `NativeModuleRegistry`.

## Problem
Creating `ModuleHolders` has the side-effect of actually creating the NativeModule if it requires eager initialization. See [ModuleHolder.java](https://fburl.com/diffusion/4avdtio0):

```
class ModuleHolder {
  // ...
  public ModuleHolder(ReactModuleInfo moduleInfo, Provider<? extends NativeModule> provider) {
    mName = moduleInfo.name();
    mProvider = provider;
    mReactModuleInfo = moduleInfo;
    if (moduleInfo.needsEagerInit()) {
      mModule = create(); // HERE!
    }
  }

```

So, we need to filter out TurboModules before we even create ModuleHolders.

Changelog:
[Android][Fixed] - Refactor TurboModule filtering in NativeModuleRegistryBuilder

Reviewed By: PeteTheHeat, mdvacca

Differential Revision: D18814010

fbshipit-source-id: a120d2b619b9280ba70e21d131dccc5a9fc6346d
  • Loading branch information
RSNara authored and facebook-github-bot committed Dec 6, 2019
1 parent e362470 commit d9deee2
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import com.facebook.react.bridge.ModuleHolder;
import com.facebook.react.bridge.NativeModuleRegistry;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.config.ReactFeatureFlags;
import java.util.HashMap;
import java.util.Map;

Expand Down Expand Up @@ -59,18 +58,6 @@ public void processPackage(ReactPackage reactPackage) {
}
mModules.remove(existingNativeModule);
}
if (ReactFeatureFlags.useTurboModules && moduleHolder.isTurboModule()) {
// If this module is a TurboModule, and if TurboModules are enabled, don't add this module

// This condition is after checking for overrides, since if there is already a module,
// and we want to override it with a turbo module, we would need to remove the modules thats
// already in the list, and then NOT add the new module, since that will be directly exposed

// Note that is someone uses {@link NativeModuleRegistry#registerModules}, we will NOT check
// for TurboModules - assuming that people wanted to explicitly register native modules
// there
continue;
}
mModules.put(name, moduleHolder);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.facebook.react.bridge.ModuleSpec;
import com.facebook.react.bridge.NativeModule;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.config.ReactFeatureFlags;
import com.facebook.react.module.model.ReactModuleInfo;
import com.facebook.react.module.model.ReactModuleInfoProvider;
import com.facebook.react.uimanager.ViewManager;
Expand All @@ -20,6 +21,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Set;
import javax.inject.Provider;

Expand Down Expand Up @@ -62,14 +64,48 @@ public Iterable<ModuleHolder> getNativeModuleIterator(
// This should ideally be an IteratorConvertor, but we don't have any internal library for it
public Iterator<ModuleHolder> iterator() {
return new Iterator<ModuleHolder>() {
Map.Entry<String, ReactModuleInfo> nextEntry = null;

private void findNext() {
while (entrySetIterator.hasNext()) {
Map.Entry<String, ReactModuleInfo> entry = entrySetIterator.next();
ReactModuleInfo reactModuleInfo = entry.getValue();

// This Iterator is used to create the NativeModule registry. The NativeModule
// registry must not have TurboModules. Therefore, if TurboModules are enabled, and
// the current NativeModule is a TurboModule, we need to skip iterating over it.
if (ReactFeatureFlags.useTurboModules && reactModuleInfo.isTurboModule()) {
continue;
}

nextEntry = entry;
return;
}
nextEntry = null;
}

@Override
public boolean hasNext() {
return entrySetIterator.hasNext();
if (nextEntry == null) {
findNext();
}
return nextEntry != null;
}

@Override
public ModuleHolder next() {
Map.Entry<String, ReactModuleInfo> entry = entrySetIterator.next();
if (nextEntry == null) {
findNext();
}

if (nextEntry == null) {
throw new NoSuchElementException("ModuleHolder not found");
}

Map.Entry<String, ReactModuleInfo> entry = nextEntry;

// Advance iterator
findNext();
String name = entry.getKey();
ReactModuleInfo reactModuleInfo = entry.getValue();
return new ModuleHolder(reactModuleInfo, new ModuleHolderProvider(name, reactContext));
Expand Down

0 comments on commit d9deee2

Please sign in to comment.