Trying to use `SWIG_NAPI_Callback` for `std::function<void(std::shared_ptr<RpcDevice>)>`

[mateusz-plociennik]

Hello again!

I am trying to use swig_napi_callback.i in my private project. I bumped into couple of issues:

• missing const next to std::vector<napi_value> &js_args that triggers candidate template ignored: could not match: https://github.com/mmomtchev/swig-napi-example-project/blob/614fcbcebe6821659048ae06c9899b2c70f16d1e/src/swig_napi_callback.i#L29

• when RET is void then:

  error: variable has incomplete type 'void'
  RET c_ret;

• c_ret is re-used as a std::string storage:

  c_ret = e.what();
  c_ret = info[0].ToString();
  throw std::runtime_error{c_ret}

• $typemap(out, std::shared_ptr<RpcDevice>, 1=device, result=js_args.at(0), argnum=callback argument 1); doesn't work with SWIG_NAPI_SetFinalizer, I had to convert napi_value to Napi::Value using Napi::Value(env, js_args.at(0)):

  rpcJAVASCRIPT_wrap.cxx:17128:15: error: no matching function for call to 'SWIG_NAPI_SetFinalizer'
            SWIG_NAPI_SetFinalizer(env, js_args.at(0), finalizer);
            ^~~~~~~~~~~~~~~~~~~~~~
  rpcJAVASCRIPT_wrap.cxx:661:17: note: candidate function not viable: cannot convert argument of incomplete type 'value_type' (aka 'napi_value__ *') to 'Napi::Value' for 2nd argument
  SWIGRUNTIME int SWIG_NAPI_SetFinalizer(Napi::Env env, Napi::Value val, SWIG_NAPI_Finalizer *finalizer);

[mateusz-plociennik]

The last issue with SWIG_NAPI_SetFinalizer I sorted out by inlining the typemap and changing the 2nd argument from js_args.at(0) to Napi::Value(env, js_args.at(0)):

#define ASYNC_CALLBACK_SUPPORT
%{
#define ASYNC_CALLBACK_SUPPORT
%}

%include "swig_napi_callback.i"

%typemap(in, fragment="SWIG_NAPI_Callback") std::function<void(std::shared_ptr<BlamOSRPC::RpcDevice>)> {
    if (! $input.IsFunction()) {
        %argument_fail(SWIG_TypeError, $type, $symname, $argnum);
    }

    $1 = SWIG_NAPI_Callback<void, std::shared_ptr<BlamOSRPC::RpcDevice>>(
        $input,
        std::function<void(Napi::Env, std::vector<napi_value>&, std::shared_ptr<BlamOSRPC::RpcDevice>)>(
            [](Napi::Env env, std::vector<napi_value>& js_args, std::shared_ptr<BlamOSRPC::RpcDevice> device) -> void {
                /* $typemap(out, std::shared_ptr<BlamOSRPC::RpcDevice>, 1=device, result=js_args.at(0), argnum=callback argument 1); */
                {
                    js_args.at(0) = SWIG_NewPointerObj(const_cast<BlamOSRPC::RpcDevice*>((&device)->get()), SWIGTYPE_p_BlamOSRPC__RpcDevice, SWIG_POINTER_OWN | 0);
                    auto* owner = new std::shared_ptr<BlamOSRPC::RpcDevice>(*&device);
                    auto* finalizer = new SWIG_NAPI_Finalizer([owner](){ delete owner; });
                    SWIG_NAPI_SetFinalizer(env, Napi::Value(env, js_args.at(0)), finalizer);
                }
            }
        ),
        [](Napi::Env env, Napi::Value js_ret) -> void {}
    );
}

%typemap(ts) std::function<void(std::shared_ptr<BlamOSRPC::RpcDevice>)> "(device: RpcDevice) => Promise<void> | void";

[mateusz-plociennik]

TBH rather than using ASYNC_CALLBACK_SUPPORT flag, I would prefer to pick the SWIG_NAPI_Callback variant myself according to the needs. What do you think about adding SWIG_NAPI_Callback_Async variant dedicated for async callbacks?

[mmomtchev]

Thank you for your feedback, I still haven't included this in an official SWIG JSE release because I suspected there were more cases. I wonder if there is a more elegant solution than explicitly constructing a Napi::Value for SWIG_NAPI_SetFinalizer. I will take your additions and will transform this into an official SWIG JSE feature.

Napi::Value can be constructed from napi_value - but this is a C++ object construction, while Napi::Value can simply be cast to napi_value by a built-in operator - and this requires only retrieving an existing pointer. On the other hand, I prefer to avoid excessive mixing of C Node-API and C++ node-addon-api.

This is why, very often, in node-addon-api, they specify the arguments to be the C napi_value instead of a C++ Node::Value. However the operator does not work with std::vector.

Going fully C Node-API is something I considered early in the development - as this would generate faster to compile code and it allow the C support in SWIG to work - but I would have lost C++ inheritance which is not trivial to implement from scratch.

[mateusz-plociennik]

If your template can accept std::vector<Napi::Value> instead of std::vector<napi_value>, then the $typemap() should work. It's just a matter of conversion where needed, like: std::vector<napi_value>(js_args.begin(), js_args.end()), correct?

[mmomtchev]

Yes, but the conversion will still be required somewhere at some point - because Node.js expects std::vector<napi_value>.

[mmomtchev]

https://github.com/nodejs/node-addon-api/blob/main/doc/function.md

Function::Call method