From 9e30c3b218c08afb51ae0e274f15422b9bcca480 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Oghin=C4=83?= Date: Fri, 27 Nov 2015 06:25:35 -0800 Subject: [PATCH] add native module overriding Differential Revision: D2700638 fb-gh-sync-id: a88ffaf864be848e1bba22e443d301e4623f04ec --- .../facebook/react/bridge/BaseJavaModule.java | 5 ++ .../facebook/react/bridge/NativeModule.java | 20 +++++-- .../react/bridge/NativeModuleRegistry.java | 55 +++++++++---------- 3 files changed, 44 insertions(+), 36 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/BaseJavaModule.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/BaseJavaModule.java index 061611e74..1a84c05fa 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/BaseJavaModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/BaseJavaModule.java @@ -327,6 +327,11 @@ public abstract class BaseJavaModule implements NativeModule { // do nothing } + @Override + public boolean canOverrideExistingModule() { + return false; + } + @Override public void onCatalystInstanceDestroy() { // do nothing diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModule.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModule.java index 480e42182..905b5be93 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModule.java @@ -22,7 +22,7 @@ import com.fasterxml.jackson.core.JsonGenerator; * register themselves using {@link CxxModuleWrapper}. */ public interface NativeModule { - public static interface NativeMethod { + interface NativeMethod { void invoke(CatalystInstance catalystInstance, ReadableNativeArray parameters); String getType(); } @@ -31,28 +31,36 @@ public interface NativeModule { * @return the name of this module. This will be the name used to {@code require()} this module * from javascript. */ - public String getName(); + String getName(); /** * @return methods callable from JS on this module */ - public Map getMethods(); + Map getMethods(); /** * Append a field which represents the constants this module exports * to JS. If no constants are exported this should do nothing. */ - public void writeConstantsField(JsonGenerator jg, String fieldName) throws IOException; + void writeConstantsField(JsonGenerator jg, String fieldName) throws IOException; /** * This is called at the end of {@link CatalystApplicationFragment#createCatalystInstance()} * after the CatalystInstance has been created, in order to initialize NativeModules that require * the CatalystInstance or JS modules. */ - public void initialize(); + void initialize(); + + /** + * Return true if you intend to override some other native module that was registered e.g. as part + * of a different package (such as the core one). Trying to override without returning true from + * this method is considered an error and will throw an exception during initialization. By + * default all modules return false. + */ + boolean canOverrideExistingModule(); /** * Called before {CatalystInstance#onHostDestroy} */ - public void onCatalystInstanceDestroy(); + void onCatalystInstanceDestroy(); } 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 80b1b4616..50bdd69bb 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModuleRegistry.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/NativeModuleRegistry.java @@ -13,11 +13,10 @@ import java.io.IOException; import java.io.StringWriter; import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.Map; -import java.util.Set; import com.facebook.react.common.MapBuilder; -import com.facebook.react.common.SetBuilder; import com.facebook.infer.annotation.Assertions; import com.facebook.systrace.Systrace; @@ -30,13 +29,13 @@ import com.fasterxml.jackson.core.JsonGenerator; public class NativeModuleRegistry { private final ArrayList mModuleTable; - private final Map, NativeModule> mModuleInstances; + private final Map, NativeModule> mModuleInstances; private final String mModuleDescriptions; private final ArrayList mBatchCompleteListenerModules; private NativeModuleRegistry( ArrayList moduleTable, - Map, NativeModule> moduleInstances, + Map, NativeModule> moduleInstances, String moduleDescriptions) { mModuleTable = moduleTable; mModuleInstances = moduleInstances; @@ -160,32 +159,24 @@ public class NativeModuleRegistry { public static class Builder { - private ArrayList mModuleDefinitions; - private Map, NativeModule> mModuleInstances; - private Set mSeenModuleNames; - - public Builder() { - mModuleDefinitions = new ArrayList(); - mModuleInstances = MapBuilder.newHashMap(); - mSeenModuleNames = SetBuilder.newHashSet(); - } + private final HashMap mModules = MapBuilder.newHashMap(); public Builder add(NativeModule module) { - ModuleDefinition registration = new ModuleDefinition( - mModuleDefinitions.size(), - module.getName(), - module); - Assertions.assertCondition( - !mSeenModuleNames.contains(module.getName()), - "Module " + module.getName() + " was already registered!"); - mSeenModuleNames.add(module.getName()); - mModuleDefinitions.add(registration); - mModuleInstances.put((Class) module.getClass(), module); + NativeModule existing = mModules.get(module.getName()); + if (existing != null && !module.canOverrideExistingModule()) { + throw new IllegalStateException("Native module " + module.getClass().getSimpleName() + + " tried to override " + existing.getClass().getSimpleName() + " for module name " + + module.getName() + ". If this was your intention, return true from " + + module.getClass().getSimpleName() + "#canOverrideExistingModule()"); + } + mModules.put(module.getName(), module); return this; } public NativeModuleRegistry build() { Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "CreateJSON"); + ArrayList moduleTable = new ArrayList<>(); + Map, NativeModule> moduleInstances = MapBuilder.newHashMap(); String moduleDefinitionJson; try { JsonFactory jsonFactory = new JsonFactory(); @@ -193,19 +184,23 @@ public class NativeModuleRegistry { try { JsonGenerator jg = jsonFactory.createGenerator(writer); jg.writeStartObject(); - for (ModuleDefinition module : mModuleDefinitions) { - jg.writeObjectFieldStart(module.name); - jg.writeNumberField("moduleID", module.id); + int idx = 0; + for (NativeModule module : mModules.values()) { + ModuleDefinition moduleDef = new ModuleDefinition(idx++, module.getName(), module); + moduleTable.add(moduleDef); + moduleInstances.put(module.getClass(), module); + jg.writeObjectFieldStart(moduleDef.name); + jg.writeNumberField("moduleID", moduleDef.id); jg.writeObjectFieldStart("methods"); - for (int i = 0; i < module.methods.size(); i++) { - MethodRegistration method = module.methods.get(i); + for (int i = 0; i < moduleDef.methods.size(); i++) { + MethodRegistration method = moduleDef.methods.get(i); jg.writeObjectFieldStart(method.name); jg.writeNumberField("methodID", i); jg.writeStringField("type", method.method.getType()); jg.writeEndObject(); } jg.writeEndObject(); - module.target.writeConstantsField(jg, "constants"); + moduleDef.target.writeConstantsField(jg, "constants"); jg.writeEndObject(); } jg.writeEndObject(); @@ -217,7 +212,7 @@ public class NativeModuleRegistry { } finally { Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE); } - return new NativeModuleRegistry(mModuleDefinitions, mModuleInstances, moduleDefinitionJson); + return new NativeModuleRegistry(moduleTable, moduleInstances, moduleDefinitionJson); } } }