Skip to content

Commit

Permalink
Correct ReactModuleSpecProcessor TurboModule detection logic
Browse files Browse the repository at this point in the history
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
  • Loading branch information
RSNara authored and facebook-github-bot committed Dec 20, 2019
1 parent c20963e commit d43059d
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 20 deletions.
1 change: 1 addition & 0 deletions ReactAndroid/src/main/java/com/facebook/react/bridge/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ INTERFACES = [
"ReadableArray.java",
"ReadableType.java",
"NativeArrayInterface.java",
"LifecycleEventListener.java",
]

rn_android_library(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
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;
Expand Down Expand Up @@ -152,6 +151,16 @@ private CodeBlock getCodeBlockForReactModuleInfos(List<String> nativeModules)
} 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;

Expand All @@ -171,12 +180,15 @@ private CodeBlock getCodeBlockForReactModuleInfos(List<String> nativeModules)
+ "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<? extends Element> elements = typeElement.getEnclosedElements();
Expand Down Expand Up @@ -222,18 +234,6 @@ private CodeBlock getCodeBlockForReactModuleInfos(List<String> nativeModules)
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
],
)

0 comments on commit d43059d

Please sign in to comment.