From d43059d0b801360c921bd1a3103274a179361925 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Fri, 20 Dec 2019 08:46:45 -0800 Subject: [PATCH] Correct ReactModuleSpecProcessor TurboModule detection logic Summary: ## Problem For efficiency reasons, we'd only check whether the current class or its superclass implemented the `TurboModule` interface. However, it's possible for NativeModules to exist that use inheritance, and have their base class extend a code-generated spec. In this case, the superclass of the superclass of the NativeModule will implement `TurboModule`. ## Solution To fix this problem, I'm relying on the `Types.isAssignable` API and checking whether the NativeModule can be assigned to the `TurboModule` interface. This is a more reliable way of knowing whether a NativeModule is a TurboModule or not. **Note:** Had to adjust the buck dependencies of FrescoModule to make the `mTypes.isAssignable` check work. Changelog: [Android][Fixed] - Correct TurboModule detection logic in ReactModuleSpecProcessor Reviewed By: fkgozali Differential Revision: D19183671 fbshipit-source-id: ad21881453fe7027d9432048108f6ba344fd7e63 --- .../main/java/com/facebook/react/bridge/BUCK | 1 + .../processing/ReactModuleSpecProcessor.java | 38 +++++++++---------- .../com/facebook/react/modules/fresco/BUCK | 5 ++- 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/BUCK b/ReactAndroid/src/main/java/com/facebook/react/bridge/BUCK index c147933c5e8..68c1e25fcd9 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/BUCK +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/BUCK @@ -11,6 +11,7 @@ INTERFACES = [ "ReadableArray.java", "ReadableType.java", "NativeArrayInterface.java", + "LifecycleEventListener.java", ] rn_android_library( diff --git a/ReactAndroid/src/main/java/com/facebook/react/module/processing/ReactModuleSpecProcessor.java b/ReactAndroid/src/main/java/com/facebook/react/module/processing/ReactModuleSpecProcessor.java index 99d9c760d12..373c78cc354 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/module/processing/ReactModuleSpecProcessor.java +++ b/ReactAndroid/src/main/java/com/facebook/react/module/processing/ReactModuleSpecProcessor.java @@ -15,7 +15,6 @@ import com.facebook.react.module.annotations.ReactModule; import com.facebook.react.module.annotations.ReactModuleList; import com.facebook.react.module.model.ReactModuleInfo; import com.facebook.react.module.model.ReactModuleInfoProvider; -import com.facebook.react.turbomodule.core.interfaces.TurboModule; import com.squareup.javapoet.ClassName; import com.squareup.javapoet.CodeBlock; import com.squareup.javapoet.JavaFile; @@ -152,6 +151,16 @@ public class ReactModuleSpecProcessor extends AbstractProcessor { } else { builder.addStatement("$T map = new $T()", MAP_TYPE, INSTANTIATED_MAP_TYPE); + String turboModuleInterfaceCanonicalName = + "com.facebook.react.turbomodule.core.interfaces.TurboModule"; + TypeMirror turboModuleInterface = + mElements.getTypeElement(turboModuleInterfaceCanonicalName).asType(); + + if (turboModuleInterface == null) { + throw new RuntimeException( + "com.facebook.react.turbomodule.core.interfaces.TurboModule interface not found."); + } + for (String nativeModule : nativeModules) { String keyString = nativeModule; @@ -171,12 +180,15 @@ public class ReactModuleSpecProcessor extends AbstractProcessor { + "Did you forget to add the @ReactModule annotation to the native module?"); } - boolean isTurboModule = isTurboModuleTypeElement(typeElement); - if (!isTurboModule) { - TypeMirror nativeModuleSpecTypeMirror = typeElement.getSuperclass(); - isTurboModule = - isTurboModuleTypeElement( - mElements.getTypeElement(nativeModuleSpecTypeMirror.toString())); + boolean isTurboModule; + try { + isTurboModule = mTypes.isAssignable(typeElement.asType(), turboModuleInterface); + } catch (Exception ex) { + throw new RuntimeException( + "Failed to check if " + + nativeModule + + " is type-assignable to " + + turboModuleInterfaceCanonicalName); } List elements = typeElement.getEnclosedElements(); @@ -222,18 +234,6 @@ public class ReactModuleSpecProcessor extends AbstractProcessor { return builder.build(); } - /** - * A Module is a Turbo Module if it either implements TurboModule or its super class, typically - * NativeModuleSpec implements TurboMobile - */ - private boolean isTurboModuleTypeElement(TypeElement typeElement) { - if (typeElement == null) { - return false; - } - return typeElement.getInterfaces().stream() - .anyMatch(el -> el.toString().equals(TurboModule.class.getName())); - } - private static class ReactModuleSpecException extends Exception { public final String mMessage; diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/fresco/BUCK b/ReactAndroid/src/main/java/com/facebook/react/modules/fresco/BUCK index 0cad1b2e00a..23de9ad34e3 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/fresco/BUCK +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/fresco/BUCK @@ -30,7 +30,10 @@ rn_android_library( react_native_target("java/com/facebook/react/bridge:bridge"), react_native_target("java/com/facebook/react/common:common"), react_native_target("java/com/facebook/react/module/annotations:annotations"), - react_native_target("java/com/facebook/react/modules/common:common"), react_native_target("java/com/facebook/react/modules/network:network"), ], + exported_deps = [ + react_native_target("java/com/facebook/react/modules/common:common"), + react_native_target("java/com/facebook/react/bridge:interfaces"), + ], )