From 2df90738d5b3d4659f50005507a3dd6bb2a70564 Mon Sep 17 00:00:00 2001 From: Kevin Gozali Date: Tue, 18 Jun 2019 16:15:14 -0700 Subject: [PATCH] Android: Use enum type for looking up JSIModule's Summary: To avoid unnecessary class loads, and better modularity, let's use string keys (enum) to access JSIModule's. For now all JSIModule's are all known inside the core infra (only FabricUIManager and TurboModuleManager right now), so let's keep it simple and explicitly list them out. The only problem here is we lose some form of type safety... Reviewed By: JoshuaGross Differential Revision: D15872777 fbshipit-source-id: 9c2de7ef1e88ef3a6dff5888d644f9d8963af2a3 --- .../react/testing/ReactAppTestActivity.java | 5 +++-- .../facebook/react/bridge/CatalystInstance.java | 2 +- .../react/bridge/CatalystInstanceImpl.java | 4 ++-- .../react/bridge/JSIModuleRegistry.java | 12 ++++++------ .../facebook/react/bridge/JSIModuleSpec.java | 2 +- .../facebook/react/bridge/JSIModuleType.java | 17 +++++++++++++++++ .../react/uimanager/UIManagerHelper.java | 3 ++- 7 files changed, 32 insertions(+), 13 deletions(-) create mode 100644 ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModuleType.java diff --git a/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactAppTestActivity.java b/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactAppTestActivity.java index 6fdc5431de6..1982602dc54 100644 --- a/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactAppTestActivity.java +++ b/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactAppTestActivity.java @@ -23,6 +23,7 @@ import com.facebook.react.bridge.JSIModule; import com.facebook.react.bridge.JSIModulePackage; import com.facebook.react.bridge.JSIModuleProvider; import com.facebook.react.bridge.JSIModuleSpec; +import com.facebook.react.bridge.JSIModuleType; import com.facebook.react.bridge.JavaScriptContextHolder; import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.bridge.ReactContext; @@ -239,8 +240,8 @@ public class ReactAppTestActivity extends FragmentActivity return Arrays.asList( new JSIModuleSpec() { @Override - public Class getJSIModuleClass() { - return UIManager.class; + public JSIModuleType getJSIModuleType() { + return JSIModuleType.UIManager; } @Override 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 69751f22508..4ee84ea0675 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java @@ -66,7 +66,7 @@ public interface CatalystInstance boolean hasNativeModule(Class nativeModuleInterface); T getNativeModule(Class nativeModuleInterface); NativeModule getNativeModule(String moduleName); - T getJSIModule(Class jsiModuleInterface); + JSIModule getJSIModule(JSIModuleType moduleType); 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 1933f10ae70..05720aa72c8 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java @@ -511,8 +511,8 @@ public class CatalystInstanceImpl implements CatalystInstance { } @Override - public T getJSIModule(Class jsiModuleInterface) { - return mJSIModuleRegistry.getModule(jsiModuleInterface); + public JSIModule getJSIModule(JSIModuleType moduleType) { + return mJSIModuleRegistry.getModule(moduleType); } private native long getJavaScriptContext(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModuleRegistry.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModuleRegistry.java index c90d4007762..46c7868f727 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModuleRegistry.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModuleRegistry.java @@ -14,21 +14,21 @@ import java.util.Map; public class JSIModuleRegistry { - private final Map mModules = new HashMap<>(); + private final Map mModules = new HashMap<>(); public JSIModuleRegistry() { } - public T getModule(Class moduleClass) { - JSIModuleHolder jsiModuleHolder = mModules.get(moduleClass); + public JSIModule getModule(JSIModuleType moduleType) { + JSIModuleHolder jsiModuleHolder = mModules.get(moduleType); if (jsiModuleHolder == null) { - throw new IllegalArgumentException("Unable to find JSIModule for class " + moduleClass); + throw new IllegalArgumentException("Unable to find JSIModule for class " + moduleType); } - return (T) Assertions.assertNotNull(jsiModuleHolder.getJSIModule()); + return Assertions.assertNotNull(jsiModuleHolder.getJSIModule()); } public void registerModules(List jsiModules) { for (JSIModuleSpec spec : jsiModules) { - mModules.put(spec.getJSIModuleClass(), new JSIModuleHolder(spec)); + mModules.put(spec.getJSIModuleType(), new JSIModuleHolder(spec)); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModuleSpec.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModuleSpec.java index 1178921292e..2e8379f7fcd 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModuleSpec.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModuleSpec.java @@ -12,7 +12,7 @@ package com.facebook.react.bridge; */ public interface JSIModuleSpec { - Class getJSIModuleClass(); + JSIModuleType getJSIModuleType(); JSIModuleProvider getJSIModuleProvider(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModuleType.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModuleType.java new file mode 100644 index 00000000000..99fb9b3e8c5 --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/JSIModuleType.java @@ -0,0 +1,17 @@ +/** + * 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. + */ + +package com.facebook.react.bridge; + +/** + * A list of support JSIModules. These are usually core infra pieces, so there + * should be an explicit list. + */ +public enum JSIModuleType { + TurboModuleManager, + UIManager, +} diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java index 6c4219e1e6e..406989e1576 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java @@ -12,6 +12,7 @@ import static com.facebook.react.uimanager.common.UIManagerType.DEFAULT; import static com.facebook.react.uimanager.common.ViewUtil.getUIManagerType; import com.facebook.react.bridge.CatalystInstance; +import com.facebook.react.bridge.JSIModuleType; import com.facebook.react.bridge.ReactContext; import com.facebook.react.bridge.UIManager; import com.facebook.react.uimanager.common.UIManagerType; @@ -34,7 +35,7 @@ public class UIManagerHelper { public static UIManager getUIManager(ReactContext context, @UIManagerType int uiManagerType) { CatalystInstance catalystInstance = context.getCatalystInstance(); return uiManagerType == FABRIC ? - catalystInstance.getJSIModule(UIManager.class) : + (UIManager)catalystInstance.getJSIModule(JSIModuleType.UIManager) : catalystInstance.getNativeModule(UIManagerModule.class); }