From 8826d8b233c1e3325a575d5012b713c4786e6062 Mon Sep 17 00:00:00 2001 From: "REDMOND\\acoates" Date: Tue, 13 Nov 2018 19:50:27 -0800 Subject: [PATCH] Allow CxxModules to implement methods with two callbacks (#21586) Summary: When writing a native module in C++ using CxxModule, its not currently possible to write async methods which take two callbacks, that shouldn't be projected to JS as a promise. I hit this when trying to implement the AppState module in c++. AppState has a single method on it, getCurrentAppState, which takes two callbacks (a success and a failure callback). This change adds a couple of new CxxModule::Method ctors, which allow devs to specify that they want two callbacks, but not a promise. This is done using an extra tag in the ctor, similar to how you specify that you want to make a synchronous method. I used the existing AsyncTag to indicate that the 2 callback version should be async, and not a promise. Pull Request resolved: https://github.com/facebook/react-native/pull/21586 Reviewed By: shergin Differential Revision: D10520204 Pulled By: fkgozali fbshipit-source-id: bcb2dbd91cba3c1db987dc18960247218fdbc032 --- ReactCommon/cxxreact/CxxModule.h | 41 ++++++++++++++++++++---- ReactCommon/cxxreact/SampleCxxModule.cpp | 19 +++++++++++ 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/ReactCommon/cxxreact/CxxModule.h b/ReactCommon/cxxreact/CxxModule.h index f253c108c2eea7..5a4d330e67f7f3 100644 --- a/ReactCommon/cxxreact/CxxModule.h +++ b/ReactCommon/cxxreact/CxxModule.h @@ -67,13 +67,14 @@ class CxxModule { std::string name; size_t callbacks; + bool isPromise; std::function func; std::function syncFunc; const char *getType() { assert(func || syncFunc); - return func ? (callbacks == 2 ? "promise" : "async") : "sync"; + return func ? (isPromise ? "promise" : "async") : "sync"; } // std::function/lambda ctors @@ -82,24 +83,36 @@ class CxxModule { std::function&& afunc) : name(std::move(aname)) , callbacks(0) + , isPromise(false) , func(std::bind(std::move(afunc))) {} Method(std::string aname, std::function&& afunc) : name(std::move(aname)) , callbacks(0) - , func(std::bind(std::move(afunc), _1)) {} + , isPromise(false) + , func(std::bind(std::move(afunc), std::placeholders::_1)) {} Method(std::string aname, std::function&& afunc) : name(std::move(aname)) , callbacks(1) - , func(std::bind(std::move(afunc), _1, _2)) {} + , isPromise(false) + , func(std::bind(std::move(afunc), std::placeholders::_1, std::placeholders::_2)) {} Method(std::string aname, std::function&& afunc) : name(std::move(aname)) , callbacks(2) + , isPromise(true) + , func(std::move(afunc)) {} + + Method(std::string aname, + std::function&& afunc, + AsyncTagType) + : name(std::move(aname)) + , callbacks(2) + , isPromise(false) , func(std::move(afunc)) {} // method pointer ctors @@ -108,25 +121,39 @@ class CxxModule { Method(std::string aname, T* t, void (T::*method)()) : name(std::move(aname)) , callbacks(0) + , isPromise(false) , func(std::bind(method, t)) {} template Method(std::string aname, T* t, void (T::*method)(folly::dynamic)) : name(std::move(aname)) , callbacks(0) - , func(std::bind(method, t, _1)) {} + , isPromise(false) + , func(std::bind(method, t, std::placeholders::_1)) {} template Method(std::string aname, T* t, void (T::*method)(folly::dynamic, Callback)) : name(std::move(aname)) , callbacks(1) - , func(std::bind(method, t, _1, _2)) {} + , isPromise(false) + , func(std::bind(method, t, std::placeholders::_1, std::placeholders::_2)) {} template Method(std::string aname, T* t, void (T::*method)(folly::dynamic, Callback, Callback)) : name(std::move(aname)) , callbacks(2) - , func(std::bind(method, t, _1, _2, _3)) {} + , isPromise(true) + , func(std::bind(method, t, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)) {} + + template + Method(std::string aname, + T* t, + void (T::*method)(folly::dynamic, Callback, Callback), + AsyncTagType) + : name(std::move(aname)) + , callbacks(2) + , isPromise(false) + , func(std::bind(method, t, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)) {} // sync std::function/lambda ctors @@ -139,6 +166,7 @@ class CxxModule { SyncTagType) : name(std::move(aname)) , callbacks(0) + , isPromise(false) , syncFunc([afunc=std::move(afunc)] (const folly::dynamic&) { return afunc(); }) {} @@ -148,6 +176,7 @@ class CxxModule { SyncTagType) : name(std::move(aname)) , callbacks(0) + , isPromise(false) , syncFunc(std::move(afunc)) {} }; diff --git a/ReactCommon/cxxreact/SampleCxxModule.cpp b/ReactCommon/cxxreact/SampleCxxModule.cpp index d0b3199f869c08..1862c0f08da4bc 100644 --- a/ReactCommon/cxxreact/SampleCxxModule.cpp +++ b/ReactCommon/cxxreact/SampleCxxModule.cpp @@ -114,6 +114,25 @@ auto SampleCxxModule::getMethods() -> std::vector { sample_->hello(); return nullptr; }, SyncTag), + Method("addIfPositiveAsPromise", [](dynamic args, Callback cb, Callback cbError) { + auto a = jsArgAsDouble(args, 0); + auto b = jsArgAsDouble(args, 1); + if (a < 0 || b < 0) { + cbError({"Negative number!"}); + } else { + cb({a + b}); + } + }), + Method("addIfPositiveAsAsync", [](dynamic args, Callback cb, Callback cbError) { + auto a = jsArgAsDouble(args, 0); + auto b = jsArgAsDouble(args, 1); + if (a < 0 || b < 0) { + cbError({"Negative number!"}); + } else { + cb({a + b}); + } + }, AsyncTag), + }; }