improve NativeModuleRegistryBuilder.java (#16402)

Summary:
I met the error `Native module xyz tried to override xyz for module name xyzModuleName. If this was your intention...` after something went wrong during `react-native link` - one module somehow ended up being included twice in the `getPackages` method, as in:

```java
Override
        protected List<ReactPackage> getPackages() {
            return Arrays.<ReactPackage>asList(
                    new MainReactPackage(),
            new WowPackage(),
            new WowPackage(),
```

Since I have > 20 native modules it took me a little while to find out what the problem was. The improved error message should make the problem clearer to anybody who may encounter it. I did try to refactor the code a little more, by extracting the whole part of:

```java
        String name = moduleHolder.getName();
        if (namesToType.containsKey(name)) {
          Class<? extends NativeModule> existingNativeModule = namesToType.get(name);
          if (!moduleHolder.getCanOverrideExistingModule()) {
            throw new IllegalStateException(getModuleOverridingExceptionMessage(
              type.getSimpleName(),
              existingNativeModule.getSimpleName(),
              name
            ));
          }

          mModules.remove(existingNativeModule);
        }

        namesToType.put(name, type);
        mModules.put(type, moduleHolder);
```

out into a separate method since there were two places where nearly identical code was written.

I have built RN from source and used it in a very simple app with RN vector icons (the package creates a native module as can be seen [here](https://github.com/oblador/react-native-vector-icons/blob/master/android/src/main/java/com/oblador/vectoricons/VectorIconsPackage.java#L19)).

After including the module twice, ie.

```java
        Override
        protected List<ReactPackage> getPackages() {
            return Arrays.<ReactPackage>asList(
                    new MainReactPackage(),
                    new VectorIconsPackage(),
                    new VectorIconsPackage()
            );
        }
```

I get the improved error description, as seen in the screenshot.

<img src="https://user-images.githubusercontent.com/1566403/36340960-3289d9d0-13e7-11e8-8d17-e1651da17841.png" height="500">

[ANDROID] [MINOR] [NativeModuleRegistryBuilder] - Improve error message and refactor putting native modules to module maps
Closes https://github.com/facebook/react-native/pull/16402

Differential Revision: D8421392

Pulled By: hramos

fbshipit-source-id: 719bd37b4681933d35858621b402ae73dd460a5b
This commit is contained in:
Vojtech Novak 2018-06-14 08:19:37 -07:00 коммит произвёл Facebook Github Bot
Родитель ef4c42d284
Коммит e3d3533bc5
1 изменённых файлов: 15 добавлений и 21 удалений

Просмотреть файл

@ -80,19 +80,7 @@ public class NativeModuleRegistryBuilder {
} }
String name = moduleHolder.getName(); String name = moduleHolder.getName();
if (namesToType.containsKey(name)) { putModuleTypeAndHolderToModuleMaps(type, name, moduleHolder);
Class<? extends NativeModule> existingNativeModule = namesToType.get(name);
if (!moduleHolder.getCanOverrideExistingModule()) {
throw new IllegalStateException("Native module " + type.getSimpleName() +
" tried to override " + existingNativeModule.getSimpleName() + " for module name " +
name + ". If this was your intention, set canOverrideExistingModule=true");
}
mModules.remove(existingNativeModule);
}
namesToType.put(name, type);
mModules.put(type, moduleHolder);
} }
} else { } else {
FLog.d( FLog.d(
@ -117,19 +105,25 @@ public class NativeModuleRegistryBuilder {
public void addNativeModule(NativeModule nativeModule) { public void addNativeModule(NativeModule nativeModule) {
String name = nativeModule.getName(); String name = nativeModule.getName();
Class<? extends NativeModule> type = nativeModule.getClass(); Class<? extends NativeModule> type = nativeModule.getClass();
if (namesToType.containsKey(name)) { putModuleTypeAndHolderToModuleMaps(type, name, new ModuleHolder(nativeModule));
Class<? extends NativeModule> existingModule = namesToType.get(name); }
if (!nativeModule.canOverrideExistingModule()) {
private void putModuleTypeAndHolderToModuleMaps(Class<? extends NativeModule> type, String underName,
ModuleHolder moduleHolder) throws IllegalStateException {
if (namesToType.containsKey(underName)) {
Class<? extends NativeModule> existingNativeModule = namesToType.get(underName);
if (!moduleHolder.getCanOverrideExistingModule()) {
throw new IllegalStateException("Native module " + type.getSimpleName() + throw new IllegalStateException("Native module " + type.getSimpleName() +
" tried to override " + existingModule.getSimpleName() + " for module name " + " tried to override " + existingNativeModule.getSimpleName() + " for module name " +
name + ". If this was your intention, set canOverrideExistingModule=true"); underName + ". Check the getPackages() method in MainApplication.java, it might be " +
"that module is being created twice. " +
"If this was your intention, set canOverrideExistingModule=true");
} }
mModules.remove(existingModule); mModules.remove(existingNativeModule);
} }
namesToType.put(name, type); namesToType.put(underName, type);
ModuleHolder moduleHolder = new ModuleHolder(nativeModule);
mModules.put(type, moduleHolder); mModules.put(type, moduleHolder);
} }