From 0cd3994f1a4ec1172af840cf5de3dddb1ae9e9c4 Mon Sep 17 00:00:00 2001 From: Ram N Date: Wed, 19 Sep 2018 18:44:05 -0700 Subject: [PATCH] Channge interface to getNativeModule to use strings instead of classes Summary: Now that NativeModules are stored based on String keys instead of classnames, the old innterface to getNativeModules(Class moduleInterface) is deprecated. This interface is also incorrect since a native module with the same name may be overridden, causing issues. Getting native modules by name is also similar to what JavaScript does Reviewed By: achen1 Differential Revision: D9697827 fbshipit-source-id: ff832bd2ea5e1c7cfe7d8c0c3a66f0d755b2c354 --- .../react/bridge/CatalystInstance.java | 1 + .../react/bridge/CatalystInstanceImpl.java | 21 +++++- .../react/bridge/NativeModuleRegistry.java | 69 ++++++++++--------- 3 files changed, 57 insertions(+), 34 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java index 700cab701..50aff5fcb 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java @@ -63,6 +63,7 @@ public interface CatalystInstance T getJSModule(Class jsInterface); boolean hasNativeModule(Class nativeModuleInterface); T getNativeModule(Class nativeModuleInterface); + NativeModule getNativeModule(String moduleName); T getJSIModule(Class jsiModuleInterface); Collection getNativeModules(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java index ba257c840..b0cf871ad 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java @@ -23,8 +23,11 @@ import com.facebook.react.bridge.queue.ReactQueueConfigurationImpl; import com.facebook.react.bridge.queue.ReactQueueConfigurationSpec; import com.facebook.react.common.ReactConstants; import com.facebook.react.common.annotations.VisibleForTesting; +import com.facebook.react.module.annotations.ReactModule; import com.facebook.systrace.Systrace; import com.facebook.systrace.TraceListener; +import java.lang.annotation.Annotation; +import java.lang.annotation.Native; import java.lang.ref.WeakReference; import java.util.ArrayList; import java.util.Collection; @@ -422,13 +425,25 @@ public class CatalystInstanceImpl implements CatalystInstance { @Override public boolean hasNativeModule(Class nativeModuleInterface) { - return mNativeModuleRegistry.hasModule(nativeModuleInterface); + return mNativeModuleRegistry.hasModule(getNameFromAnnotation(nativeModuleInterface)); } - // This is only ever called with UIManagerModule or CurrentViewerModule. @Override public T getNativeModule(Class nativeModuleInterface) { - return mNativeModuleRegistry.getModule(nativeModuleInterface); + return (T) mNativeModuleRegistry.getModule(getNameFromAnnotation(nativeModuleInterface)); + } + + @Override + public NativeModule getNativeModule(String moduleName) { + return mNativeModuleRegistry.getModule(moduleName); + } + + private String getNameFromAnnotation(Class nativeModuleInterface){ + ReactModule annotation = nativeModuleInterface.getAnnotation(ReactModule.class); + if (annotation == null) { + throw new IllegalArgumentException("Could not find @ReactModule annotation in " + nativeModuleInterface.getCanonicalName()); + } + return annotation.name(); } // This is only used by com.facebook.react.modules.common.ModuleDataCleaner diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModuleRegistry.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModuleRegistry.java index 4a2fa06e1..126e7fb75 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModuleRegistry.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModuleRegistry.java @@ -1,39 +1,32 @@ /** * Copyright (c) Facebook, Inc. and its affiliates. * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. + *

This source code is licensed under the MIT license found in the LICENSE file in the root + * directory of this source tree. */ - package com.facebook.react.bridge; import com.facebook.infer.annotation.Assertions; import com.facebook.react.module.annotations.ReactModule; import com.facebook.systrace.Systrace; -import java.lang.annotation.Annotation; import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Map; -/** - * A set of Java APIs to expose to a particular JavaScript instance. - */ +/** A set of Java APIs to expose to a particular JavaScript instance. */ public class NativeModuleRegistry { private final ReactApplicationContext mReactApplicationContext; private final Map mModules; public NativeModuleRegistry( - ReactApplicationContext reactApplicationContext, - Map modules) { + ReactApplicationContext reactApplicationContext, Map modules) { mReactApplicationContext = reactApplicationContext; mModules = modules; } - /** - * Private getters for combining NativeModuleRegistrys - */ + /** Private getters for combining NativeModuleRegistrys */ private Map getModuleMap() { return mModules; } @@ -42,12 +35,10 @@ public class NativeModuleRegistry { return mReactApplicationContext; } - /* package */ Collection getJavaModules( - JSInstance jsInstance) { + /* package */ Collection getJavaModules(JSInstance jsInstance) { ArrayList javaModules = new ArrayList<>(); for (Map.Entry entry : mModules.entrySet()) { if (!entry.getValue().isCxxModule()) { - //if (!CxxModuleWrapperBase.class.isAssignableFrom(entry.getValue().getModule().getClass())) { javaModules.add(new JavaModuleWrapper(jsInstance, entry.getValue())); } } @@ -65,12 +56,13 @@ public class NativeModuleRegistry { } /* - * Adds any new modules to the current module registry - */ + * Adds any new modules to the current module registry + */ /* package */ void registerModules(NativeModuleRegistry newRegister) { - Assertions.assertCondition(mReactApplicationContext.equals(newRegister.getReactApplicationContext()), - "Extending native modules with non-matching application contexts."); + Assertions.assertCondition( + mReactApplicationContext.equals(newRegister.getReactApplicationContext()), + "Extending native modules with non-matching application contexts."); Map newModules = newRegister.getModuleMap(); @@ -86,8 +78,7 @@ public class NativeModuleRegistry { /* package */ void notifyJSInstanceDestroy() { mReactApplicationContext.assertOnNativeModulesQueueThread(); Systrace.beginSection( - Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, - "NativeModuleRegistry_notifyJSInstanceDestroy"); + Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "NativeModuleRegistry_notifyJSInstanceDestroy"); try { for (ModuleHolder module : mModules.values()) { module.destroy(); @@ -98,14 +89,14 @@ public class NativeModuleRegistry { } /* package */ void notifyJSInstanceInitialized() { - mReactApplicationContext.assertOnNativeModulesQueueThread("From version React Native v0.44, " + - "native modules are explicitly not initialized on the UI thread. See " + - "https://github.com/facebook/react-native/wiki/Breaking-Changes#d4611211-reactnativeandroidbreaking-move-nativemodule-initialization-off-ui-thread---aaachiuuu " + - " for more details."); + mReactApplicationContext.assertOnNativeModulesQueueThread( + "From version React Native v0.44, " + + "native modules are explicitly not initialized on the UI thread. See " + + "https://github.com/facebook/react-native/wiki/Breaking-Changes#d4611211-reactnativeandroidbreaking-move-nativemodule-initialization-off-ui-thread---aaachiuuu " + + " for more details."); ReactMarker.logMarker(ReactMarkerConstants.NATIVE_MODULE_INITIALIZE_START); Systrace.beginSection( - Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, - "NativeModuleRegistry_notifyJSInstanceInitialized"); + Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "NativeModuleRegistry_notifyJSInstanceInitialized"); try { for (ModuleHolder module : mModules.values()) { module.markInitializable(); @@ -118,7 +109,8 @@ public class NativeModuleRegistry { public void onBatchComplete() { // The only native module that uses the onBatchComplete is the UI Manager. Hence, instead of - // iterating over all the modules for find this one instance, and then calling it, we short-circuit + // iterating over all the modules for find this one instance, and then calling it, we + // short-circuit // the search, and simply call OnBatchComplete on the UI Manager. // With Fabric, UIManager would no longer be a NativeModule, so this call would simply go away ModuleHolder moduleHolder = mModules.get("UIManager"); @@ -135,10 +127,25 @@ public class NativeModuleRegistry { public T getModule(Class moduleInterface) { ReactModule annotation = moduleInterface.getAnnotation(ReactModule.class); if (annotation == null) { - throw new IllegalArgumentException("Could not find @ReactModule annotation in class " + moduleInterface.getName()); + throw new IllegalArgumentException( + "Could not find @ReactModule annotation in class " + moduleInterface.getName()); } - return (T) Assertions.assertNotNull( - mModules.get(annotation.name()), annotation.name() + " could not be found. Is it defined in " + moduleInterface.getName()).getModule(); + return (T) + Assertions.assertNotNull( + mModules.get(annotation.name()), + annotation.name() + + " could not be found. Is it defined in " + + moduleInterface.getName()) + .getModule(); + } + + public boolean hasModule(String name) { + return mModules.containsKey(name); + } + + public NativeModule getModule(String name) { + return Assertions.assertNotNull( + mModules.get(name), "Could not find module with name " + name).getModule(); } public List getAllModules() {