Translating NativeArray into NativeMap on getConstants

Reviewed By: javache Differential Revision: D5321741 fbshipit-source-id: 2465c69a5bd1d4f3cf20ba73e163372b12616312
facebook · Jun 29, 2017 · 25d19e3 · 25d19e3
1 parent 33057aa
commit 25d19e3
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 21 deletions.
diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/JSCJavaScriptExecutor.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/JSCJavaScriptExecutor.java
@@ -15,13 +15,10 @@
 @DoNotStrip
 public class JSCJavaScriptExecutor extends JavaScriptExecutor {
   public static class Factory implements JavaScriptExecutor.Factory {
-    private ReadableNativeArray mJSCConfig;
+    private ReadableNativeMap mJSCConfig;
 
     public Factory(WritableNativeMap jscConfig) {
-      // TODO (t10707444): use NativeMap, which requires moving NativeMap out of OnLoad.
-      WritableNativeArray array = new WritableNativeArray();
-      array.pushMap(jscConfig);
-      mJSCConfig = array;
+      mJSCConfig = jscConfig;
     }
 
     @Override
@@ -34,9 +31,9 @@ public JavaScriptExecutor create() throws Exception {
     ReactBridge.staticInit();
   }
 
-  public JSCJavaScriptExecutor(ReadableNativeArray jscConfig) {
+  public JSCJavaScriptExecutor(ReadableNativeMap jscConfig) {
     super(initHybrid(jscConfig));
   }
 
-  private native static HybridData initHybrid(ReadableNativeArray jscConfig);
+  private native static HybridData initHybrid(ReadableNativeMap jscConfig);
 }
diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaModuleWrapper.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaModuleWrapper.java
@@ -122,10 +122,8 @@ public List<MethodDescriptor> getMethodDescriptors() {
     return mDescs;
   }
 
-  // TODO mhorowitz: make this return NativeMap, which requires moving
-  // NativeMap out of OnLoad.
   @DoNotStrip
-  public @Nullable NativeArray getConstants() {
+  public @Nullable NativeMap getConstants() {
     if (!mModuleHolder.getHasConstants()) {
       return null;
     }
@@ -145,9 +143,7 @@ public List<MethodDescriptor> getMethodDescriptors() {
     Systrace.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "create WritableNativeMap");
     ReactMarker.logMarker(CONVERT_CONSTANTS_START, moduleName);
     try {
-      WritableNativeArray array = new WritableNativeArray();
-      array.pushMap(Arguments.makeNativeMap(map));
-      return array;
+      return Arguments.makeNativeMap(map);
     } finally {
       ReactMarker.logMarker(CONVERT_CONSTANTS_END);
       Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);

diff --git a/ReactAndroid/src/main/jni/react/jni/JavaModuleWrapper.cpp b/ReactAndroid/src/main/jni/react/jni/JavaModuleWrapper.cpp
@@ -74,13 +74,12 @@ std::vector<MethodDescriptor> JavaNativeModule::getMethods() {
 
 folly::dynamic JavaNativeModule::getConstants() {
   static auto constantsMethod =
-    wrapper_->getClass()->getMethod<NativeArray::javaobject()>("getConstants");
+    wrapper_->getClass()->getMethod<NativeMap::javaobject()>("getConstants");
   auto constants = constantsMethod(wrapper_);
   if (!constants) {
     return nullptr;
   } else {
-    // See JavaModuleWrapper#getConstants for the other side of this hack.
-    return cthis(constants)->consume()[0];
+    return cthis(constants)->consume();
   }
 }
 
@@ -147,13 +146,12 @@ std::vector<MethodDescriptor> NewJavaNativeModule::getMethods() {
 
 folly::dynamic NewJavaNativeModule::getConstants() {
   static auto constantsMethod =
-    wrapper_->getClass()->getMethod<NativeArray::javaobject()>("getConstants");
+    wrapper_->getClass()->getMethod<NativeMap::javaobject()>("getConstants");
   auto constants = constantsMethod(wrapper_);
   if (!constants) {
     return nullptr;
   } else {
-    // See JavaModuleWrapper#getConstants for the other side of this hack.
-    return cthis(constants)->consume()[0];
+    return cthis(constants)->consume();
   }
 }
 

diff --git a/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp b/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp
@@ -39,9 +39,9 @@ class JSCJavaScriptExecutorHolder : public HybridClass<JSCJavaScriptExecutorHold
  public:
   static constexpr auto kJavaDescriptor = "Lcom/facebook/react/bridge/JSCJavaScriptExecutor;";
 
-  static local_ref<jhybriddata> initHybrid(alias_ref<jclass>, ReadableNativeArray* jscConfigArray) {
+  static local_ref<jhybriddata> initHybrid(alias_ref<jclass>, ReadableNativeMap* jscConfig) {
     // See JSCJavaScriptExecutor.Factory() for the other side of this hack.
-    folly::dynamic jscConfigMap = jscConfigArray->consume()[0];
+    folly::dynamic jscConfigMap = jscConfig->consume();
     return makeCxxInstance(std::make_shared<JSCExecutorFactory>(std::move(jscConfigMap)));
   }