Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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: #21586

Reviewed By: shergin

Differential Revision: D10520204

Pulled By: fkgozali

fbshipit-source-id: bcb2dbd91cba3c1db987dc18960247218fdbc032
  • Loading branch information
REDMOND\acoates authored and facebook-github-bot committed Nov 14, 2018
1 parent 11d4512 commit 8826d8b
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 6 deletions.
41 changes: 35 additions & 6 deletions ReactCommon/cxxreact/CxxModule.h
Expand Up @@ -67,13 +67,14 @@ class CxxModule {
std::string name;

size_t callbacks;
bool isPromise;
std::function<void(folly::dynamic, Callback, Callback)> func;

std::function<folly::dynamic(folly::dynamic)> syncFunc;

const char *getType() {
assert(func || syncFunc);
return func ? (callbacks == 2 ? "promise" : "async") : "sync";
return func ? (isPromise ? "promise" : "async") : "sync";
}

// std::function/lambda ctors
Expand All @@ -82,24 +83,36 @@ class CxxModule {
std::function<void()>&& afunc)
: name(std::move(aname))
, callbacks(0)
, isPromise(false)
, func(std::bind(std::move(afunc))) {}

Method(std::string aname,
std::function<void(folly::dynamic)>&& 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<void(folly::dynamic, Callback)>&& 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<void(folly::dynamic, Callback, Callback)>&& afunc)
: name(std::move(aname))
, callbacks(2)
, isPromise(true)
, func(std::move(afunc)) {}

Method(std::string aname,
std::function<void(folly::dynamic, Callback, Callback)>&& afunc,
AsyncTagType)
: name(std::move(aname))
, callbacks(2)
, isPromise(false)
, func(std::move(afunc)) {}

// method pointer ctors
Expand All @@ -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 <typename T>
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 <typename T>
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 <typename T>
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 <typename T>
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

Expand All @@ -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(); })
{}
Expand All @@ -148,6 +176,7 @@ class CxxModule {
SyncTagType)
: name(std::move(aname))
, callbacks(0)
, isPromise(false)
, syncFunc(std::move(afunc))
{}
};
Expand Down
19 changes: 19 additions & 0 deletions ReactCommon/cxxreact/SampleCxxModule.cpp
Expand Up @@ -114,6 +114,25 @@ auto SampleCxxModule::getMethods() -> std::vector<Method> {
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),

};
}

Expand Down

0 comments on commit 8826d8b

Please sign in to comment.