From b2647ea33543a144000892ad7eeb4329a660507f Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Fri, 7 Apr 2017 09:27:05 -0700 Subject: [PATCH] Allow ModuleRegistry initialization to complete lazily Reviewed By: mhorowitz Differential Revision: D4794794 fbshipit-source-id: f3b7a5d02587b4cd4e214aa6b0368a0d214fb63f --- React/CxxBridge/RCTCxxBridge.mm | 2 +- React/CxxModule/RCTCxxUtils.h | 3 +-- React/CxxModule/RCTCxxUtils.mm | 5 ++--- ReactCommon/cxxreact/JSCNativeModules.cpp | 4 ---- ReactCommon/cxxreact/ModuleRegistry.cpp | 26 +++++++++++++++++++---- ReactCommon/cxxreact/ModuleRegistry.h | 2 ++ 6 files changed, 28 insertions(+), 14 deletions(-) diff --git a/React/CxxBridge/RCTCxxBridge.mm b/React/CxxBridge/RCTCxxBridge.mm index 86bd33b8a68..da803299b46 100644 --- a/React/CxxBridge/RCTCxxBridge.mm +++ b/React/CxxBridge/RCTCxxBridge.mm @@ -398,7 +398,7 @@ struct RCTInstanceCallback : public InstanceCallback { [_performanceLogger markStartForTag:RCTPLNativeModulePrepareConfig]; RCT_PROFILE_BEGIN_EVENT(RCTProfileTagAlways, @"-[RCTCxxBridge buildModuleRegistry]", nil); - auto registry = buildModuleRegistry(_moduleDataByID, self, _reactInstance); + auto registry = std::make_shared(createNativeModules(_moduleDataByID, self, _reactInstance)); [_performanceLogger markStopForTag:RCTPLNativeModulePrepareConfig]; RCT_PROFILE_END_EVENT(RCTProfileTagAlways, @""); diff --git a/React/CxxModule/RCTCxxUtils.h b/React/CxxModule/RCTCxxUtils.h index e84032682d1..41e701dfb7c 100644 --- a/React/CxxModule/RCTCxxUtils.h +++ b/React/CxxModule/RCTCxxUtils.h @@ -12,7 +12,6 @@ #import #include #include -#include #include #include @@ -30,7 +29,7 @@ namespace react { class Instance; -std::shared_ptr buildModuleRegistry(NSArray *modules, RCTBridge *bridge, const std::shared_ptr &instance); +std::vector> createNativeModules(NSArray *modules, RCTBridge *bridge, const std::shared_ptr &instance); JSContext *contextForGlobalContextRef(JSGlobalContextRef contextRef); diff --git a/React/CxxModule/RCTCxxUtils.mm b/React/CxxModule/RCTCxxUtils.mm index 740f5960aa3..331e0bfaf55 100644 --- a/React/CxxModule/RCTCxxUtils.mm +++ b/React/CxxModule/RCTCxxUtils.mm @@ -41,7 +41,7 @@ using namespace facebook::react; namespace facebook { namespace react { -std::shared_ptr buildModuleRegistry(NSArray *modules, RCTBridge *bridge, const std::shared_ptr &instance) +std::vector> createNativeModules(NSArray *modules, RCTBridge *bridge, const std::shared_ptr &instance) { std::vector> nativeModules; for (RCTModuleData *moduleData in modules) { @@ -61,8 +61,7 @@ std::shared_ptr buildModuleRegistry(NSArray *mo nativeModules.emplace_back(std::make_unique(bridge, moduleData)); } } - - return std::make_shared(std::move(nativeModules)); + return nativeModules; } JSContext *contextForGlobalContextRef(JSGlobalContextRef contextRef) diff --git a/ReactCommon/cxxreact/JSCNativeModules.cpp b/ReactCommon/cxxreact/JSCNativeModules.cpp index f55e0049736..013f6ef13fa 100644 --- a/ReactCommon/cxxreact/JSCNativeModules.cpp +++ b/ReactCommon/cxxreact/JSCNativeModules.cpp @@ -45,10 +45,6 @@ folly::Optional JSCNativeModules::createModule(const std::string& name, auto global = Object::getGlobalObject(context); m_genNativeModuleJS = global.getProperty("__fbGenNativeModule").asObject(); m_genNativeModuleJS->makeProtected(); - - // Initialize the module name list, otherwise getModuleConfig won't work - // TODO (pieterdb): fix this in ModuleRegistry - m_moduleRegistry->moduleNames(); } auto result = m_moduleRegistry->getConfig(name); diff --git a/ReactCommon/cxxreact/ModuleRegistry.cpp b/ReactCommon/cxxreact/ModuleRegistry.cpp index 39ef2e0d252..4a7dcb80149 100644 --- a/ReactCommon/cxxreact/ModuleRegistry.cpp +++ b/ReactCommon/cxxreact/ModuleRegistry.cpp @@ -2,6 +2,8 @@ #include "ModuleRegistry.h" +#include + #include "NativeModule.h" #include "SystraceSection.h" @@ -28,6 +30,18 @@ std::string normalizeName(std::string name) { ModuleRegistry::ModuleRegistry(std::vector> modules) : modules_(std::move(modules)) {} +void ModuleRegistry::registerModules(std::vector> modules) { + // TODO: consider relaxing this restriction + CHECK(modulesByName_.empty()) << "Can only register additional modules before NativeModules have been accessed"; + + if (modules_.empty()) { + modules_ = std::move(modules); + } else { + modules_.reserve(modules_.size() + modules.size()); + std::move(modules.begin(), modules.end(), std::back_inserter(modules_)); + } +} + std::vector ModuleRegistry::moduleNames() { std::vector names; for (size_t i = 0; i < modules_.size(); i++) { @@ -40,6 +54,12 @@ std::vector ModuleRegistry::moduleNames() { folly::Optional ModuleRegistry::getConfig(const std::string& name) { SystraceSection s("getConfig", "module", name); + + // Initialize modulesByName_ + if (modulesByName_.empty() && !modules_.empty()) { + moduleNames(); + } + auto it = modulesByName_.find(name); if (it == modulesByName_.end()) { return nullptr; @@ -97,8 +117,7 @@ void ModuleRegistry::callNativeMethod(ExecutorToken token, unsigned int moduleId folly::dynamic&& params, int callId) { if (moduleId >= modules_.size()) { throw std::runtime_error( - folly::to("moduleId ", moduleId, - " out of range [0..", modules_.size(), ")")); + folly::to("moduleId ", moduleId, " out of range [0..", modules_.size(), ")")); } #ifdef WITH_FBSYSTRACE @@ -113,8 +132,7 @@ void ModuleRegistry::callNativeMethod(ExecutorToken token, unsigned int moduleId MethodCallResult ModuleRegistry::callSerializableNativeHook(ExecutorToken token, unsigned int moduleId, unsigned int methodId, folly::dynamic&& params) { if (moduleId >= modules_.size()) { throw std::runtime_error( - folly::to("moduleId ", moduleId, - " out of range [0..", modules_.size(), ")")); + folly::to("moduleId ", moduleId, "out of range [0..", modules_.size(), ")")); } return modules_[moduleId]->callSerializableNativeHook(token, methodId, std::move(params)); } diff --git a/ReactCommon/cxxreact/ModuleRegistry.h b/ReactCommon/cxxreact/ModuleRegistry.h index ef19d28a674..fd5665c5306 100644 --- a/ReactCommon/cxxreact/ModuleRegistry.h +++ b/ReactCommon/cxxreact/ModuleRegistry.h @@ -30,6 +30,8 @@ class ModuleRegistry { // notifyCatalystInstanceDestroy: use RAII instead ModuleRegistry(std::vector> modules); + void registerModules(std::vector> modules); + std::vector moduleNames(); folly::Optional getConfig(const std::string& name);