Refactor TurboModule filtering in NativeModuleRegistryBuilder

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
This commit is contained in:
Ramanpreet Nara
2019-12-05 18:57:16 -08:00
committed by Facebook Github Bot
parent e362470305
commit d9deee20e7
2 changed files with 38 additions and 15 deletions
@@ -10,7 +10,6 @@ package com.facebook.react;
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;
@@ -59,18 +58,6 @@ public class NativeModuleRegistryBuilder {
}
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);
}
}
@@ -12,6 +12,7 @@ import com.facebook.react.bridge.ModuleHolder;
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;
@@ -20,6 +21,7 @@ import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Set;
import javax.inject.Provider;
@@ -62,14 +64,48 @@ public abstract class TurboReactPackage implements ReactPackage {
// 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));