Skip to content

Commit c21bdcd

Browse files
authored
Migrate nan to node-addon-api for implementing the C++ addon (#1090)
The day when `rclnodejs` was initialized, [nan](https://github.com/nodejs/nan) was the only library designed to simplify the development of native addons, providing a consistent interface for interacting with the V8 JavaScript engine and Node.js APIs across different Node.js versions. [N-API](https://nodejs.org/api/n-api.html#node-api) was introduced in Node.js v8.0.0 as an experimental feature to provide a stable Node API for native modules, it's officially supported by Node and allows developing native addons that are independent of the underlying JavaScript engine, and ensures Application Binary Interface (ABI) stability across different Node.js versions and compiler levels. so it is recommended to migrate to N-API for existing projects. This patch leverages [node-addon-api](https://github.com/nodejs/node-addon-api) based on [Node-API](https://nodejs.org/api/n-api.html), which provides a C++ object model and exception handling semantics with low overhead, to rewrite the existing C++ bindings, including: 1. Add `node-addon-api` into dependencies. 2. Replace the `Nan` headers with `node-addon-api` headers. 3. Replace `nan` APIs with `node-addon-api` equivalents. 4. Use `Napi::Env` for environment handling. 5. Replace `Nan::ObjectWrap` with `Napi::ObjectWrap`. 6. Replace `Nan::Persistent` with `Napi::FunctionReference`. 7. Update error handling and buffer management. After this patch, we will remove the explicit usage of classes from `v8::`. Fix: #1036
1 parent 3d3eb17 commit c21bdcd

22 files changed

+1650
-1635
lines changed

binding.gyp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@
3030
],
3131
'include_dirs': [
3232
'.',
33-
"<!(node -e \"require('nan')\")",
3433
'<(ros_include_root)',
34+
"<!@(node -p \"require('node-addon-api').include\")",
3535
],
3636
'cflags!': [
3737
'-fno-exceptions'

lib/logging.js

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,31 @@ let LoggingSeverity = {
3939

4040
class Caller {
4141
constructor() {
42-
this._info = {};
43-
let frame = new Error().stack.split('\n').slice(4, 5)[0];
44-
let results = frame.match(/at\s+(.*)\s+\((.*):(\d*):(\d*)\)/i);
45-
46-
if (results && results.length === 5) {
47-
this._info['functionName'] = results[1];
48-
this._info['lineNumber'] = results[3];
49-
this._info['fileName'] = path.basename(results[2]);
42+
this._info = {
43+
functionName: 'unknown',
44+
fileName: 'unknown',
45+
lineNumber: 'unknown',
46+
};
47+
48+
const stackLines = new Error().stack.split('\n');
49+
50+
// Adjust the index (usually 3 or 4) to correctly point to the caller frame.
51+
const callerFrame = stackLines[4] || stackLines[3];
52+
// Match both named and anonymous function stack frames.
53+
const frameRegex = /^\s*at\s+(?:(.+)\s+\()?(.+):(\d+):(\d+)\)?$/;
54+
const match = callerFrame.match(frameRegex);
55+
if (match && match.length === 5) {
56+
this._info.functionName = match[1] || '(anonymous)';
57+
this._info.fileName = path.basename(match[2]);
58+
this._info.lineNumber = match[3];
59+
} else {
60+
// Handle anonymous functions or different stack formats.
61+
const altMatch = callerFrame.match(/at\s+(.*):(\d+):(\d+)/i);
62+
if (altMatch && altMatch.length >= 4) {
63+
this._info.functionName = '(anonymous)';
64+
this._info.fileName = path.basename(altMatch[1]);
65+
this._info.lineNumber = altMatch[2];
66+
}
5067
}
5168
}
5269

@@ -156,7 +173,7 @@ class Logging {
156173
severity,
157174
message,
158175
caller.functionName,
159-
caller.lineNumber,
176+
parseInt(caller.lineNumber, 10),
160177
caller.fileName
161178
);
162179
}

package.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
"docs": "cd docs && make",
2727
"test": "nyc node --expose-gc ./scripts/run_test.js && tsd",
2828
"lint": "eslint && node ./scripts/cpplint.js",
29-
"format": "clang-format -i -style=file ./src/*.cpp ./src/*.hpp && npx --yes prettier --write \"{lib,rosidl_gen,rostsd_gen,rosidl_parser,types,example,test,scripts,benchmark,rostsd_gen}/**/*.{js,md,ts}\" ./*.{js,md,ts}",
29+
"format": "clang-format -i -style=file ./src/*.cpp ./src/*.h && npx --yes prettier --write \"{lib,rosidl_gen,rostsd_gen,rosidl_parser,types,example,test,scripts,benchmark,rostsd_gen}/**/*.{js,md,ts}\" ./*.{js,md,ts}",
3030
"prepare": "husky",
3131
"coverage": "cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js"
3232
},
@@ -79,9 +79,9 @@
7979
"fs-extra": "^11.2.0",
8080
"is-close": "^1.3.3",
8181
"json-bigint": "^1.0.0",
82-
"nan": "^2.22.0",
8382
"terser": "^5.39.0",
84-
"walk": "^2.3.15"
83+
"walk": "^2.3.15",
84+
"node-addon-api": "^8.3.1"
8585
},
8686
"husky": {
8787
"hooks": {

src/addon.cpp

Lines changed: 26 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -12,29 +12,25 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
#include <nan.h>
15+
#include <node_api.h>
16+
#include <rcutils/logging.h>
1617

17-
#include "macros.hpp"
18-
#include "rcl_action_bindings.hpp"
19-
#include "rcl_bindings.hpp"
20-
#include "rcl_handle.hpp"
21-
#include "rcl_lifecycle_bindings.hpp"
22-
#include "rcutils/logging.h"
23-
#include "rcutils/macros.h"
24-
#include "shadow_node.hpp"
18+
#include "macros.h"
19+
#include "rcl_action_bindings.h"
20+
#include "rcl_bindings.h"
21+
#include "rcl_handle.h"
22+
#include "rcl_lifecycle_bindings.h"
23+
#include "rcl_utilities.h"
24+
#include "shadow_node.h"
2525

26-
bool IsRunningInElectronRenderer() {
27-
auto global = Nan::GetCurrentContext()->Global();
28-
auto process =
29-
Nan::To<v8::Object>(Nan::Get(global, Nan::New("process").ToLocalChecked())
30-
.ToLocalChecked())
31-
.ToLocalChecked();
32-
auto process_type =
33-
Nan::Get(process, Nan::New("type").ToLocalChecked()).ToLocalChecked();
34-
return process_type->StrictEquals(Nan::New("renderer").ToLocalChecked());
26+
bool IsRunningInElectronRenderer(const Napi::Env& env) {
27+
Napi::Object global = env.Global();
28+
Napi::Object process = global.Get("process").As<Napi::Object>();
29+
Napi::Value processType = process.Get("type");
30+
return processType.StrictEquals(Napi::String::New(env, "renderer"));
3531
}
3632

37-
void InitModule(v8::Local<v8::Object> exports) {
33+
Napi::Object InitModule(Napi::Env env, Napi::Object exports) {
3834
// workaround process name mangling by chromium
3935
//
4036
// rcl logging uses `program_invocation_name` to determine the log file,
@@ -43,52 +39,29 @@ void InitModule(v8::Local<v8::Object> exports) {
4339
// occurence of ' -' with the null terminator. see:
4440
// https://unix.stackexchange.com/questions/432419/unexpected-non-null-encoding-of-proc-pid-cmdline
4541
#if defined(__linux__) && defined(__GLIBC__)
46-
if (IsRunningInElectronRenderer()) {
42+
if (IsRunningInElectronRenderer(env)) {
4743
auto prog_name = program_invocation_name;
4844
auto end = strstr(prog_name, " -");
4945
assert(end);
5046
prog_name[end - prog_name] = 0;
5147
}
5248
#endif
5349

54-
v8::Local<v8::Context> context = exports->GetIsolate()->GetCurrentContext();
55-
56-
for (uint32_t i = 0; i < rclnodejs::binding_methods.size(); i++) {
57-
Nan::Set(
58-
exports, Nan::New(rclnodejs::binding_methods[i].name).ToLocalChecked(),
59-
Nan::New<v8::FunctionTemplate>(rclnodejs::binding_methods[i].function)
60-
->GetFunction(context)
61-
.ToLocalChecked());
62-
}
63-
64-
for (uint32_t i = 0; i < rclnodejs::action_binding_methods.size(); i++) {
65-
Nan::Set(
66-
exports,
67-
Nan::New(rclnodejs::action_binding_methods[i].name).ToLocalChecked(),
68-
Nan::New<v8::FunctionTemplate>(
69-
rclnodejs::action_binding_methods[i].function)
70-
->GetFunction(context)
71-
.ToLocalChecked());
72-
}
73-
74-
for (uint32_t i = 0; i < rclnodejs::lifecycle_binding_methods.size(); i++) {
75-
Nan::Set(
76-
exports,
77-
Nan::New(rclnodejs::lifecycle_binding_methods[i].name).ToLocalChecked(),
78-
Nan::New<v8::FunctionTemplate>(
79-
rclnodejs::lifecycle_binding_methods[i].function)
80-
->GetFunction(context)
81-
.ToLocalChecked());
82-
}
83-
84-
rclnodejs::ShadowNode::Init(exports);
85-
rclnodejs::RclHandle::Init(exports);
50+
// Init the C++ bindings.
51+
rclnodejs::StoreEnv(env);
52+
rclnodejs::InitBindings(env, exports);
53+
rclnodejs::InitAction(env, exports);
54+
rclnodejs::InitLifecycle(env, exports);
55+
rclnodejs::ShadowNode::Init(env, exports);
56+
rclnodejs::RclHandle::Init(env, exports);
8657

8758
#ifdef DEBUG_ON
8859
int result = rcutils_logging_set_logger_level(PACKAGE_NAME,
8960
RCUTILS_LOG_SEVERITY_DEBUG);
9061
RCUTILS_UNUSED(result);
9162
#endif
63+
64+
return exports;
9265
}
9366

94-
NODE_MODULE(rclnodejs, InitModule);
67+
NODE_API_MODULE(rclnodejs, InitModule)

src/executor.cpp

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,16 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
#include "executor.hpp"
15+
#include "executor.h"
1616

1717
#include <rcl/error_handling.h>
1818

1919
#include <stdexcept>
2020
#include <string>
2121

22-
#include "handle_manager.hpp"
23-
#include "macros.hpp"
24-
#include "rcl_bindings.hpp"
22+
#include "handle_manager.h"
23+
#include "macros.h"
24+
#include "rcl_bindings.h"
2525

2626
#ifdef WIN32
2727
#define UNUSED
@@ -41,12 +41,14 @@ struct RclResult {
4141
std::string error_msg;
4242
};
4343

44-
Executor::Executor(HandleManager* handle_manager, Delegate* delegate)
44+
Executor::Executor(Napi::Env env, HandleManager* handle_manager,
45+
Delegate* delegate)
4546
: async_(nullptr),
4647
main_thread_(uv_thread_self()),
4748
handle_manager_(handle_manager),
4849
delegate_(delegate),
49-
context_(nullptr) {
50+
context_(nullptr),
51+
env_(env) {
5052
running_.store(false);
5153
}
5254

@@ -74,20 +76,27 @@ void Executor::SpinOnce(rcl_context_t* context, int32_t time_out) {
7476
rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
7577
rcl_ret_t ret = rcl_wait_set_init(&wait_set, 0, 0, 0, 0, 0, 0, context,
7678
rcl_get_default_allocator());
77-
if (ret != RCL_RET_OK) Nan::ThrowError(rcl_get_error_string().str);
79+
if (ret != RCL_RET_OK) {
80+
Napi::Error::New(env_, rcl_get_error_string().str)
81+
.ThrowAsJavaScriptException();
82+
return;
83+
}
7884

7985
RclResult wait_result = WaitForReadyCallbacks(&wait_set, time_out);
8086

81-
if (wait_result.ret != RCL_RET_OK)
82-
Nan::ThrowError(wait_result.error_msg.c_str());
87+
if (wait_result.ret != RCL_RET_OK) {
88+
Napi::Error::New(env_, wait_result.error_msg.c_str())
89+
.ThrowAsJavaScriptException();
90+
return;
91+
}
8392

8493
if (handle_manager_->ready_handles_count() > 0) ExecuteReadyHandles();
8594

8695
if (rcl_wait_set_fini(&wait_set) != RCL_RET_OK) {
8796
std::string error_message =
8897
std::string("Failed to destroy guard waitset:") +
8998
std::string(rcl_get_error_string().str);
90-
Nan::ThrowError(error_message.c_str());
99+
Napi::Error::New(env_, error_message.c_str()).ThrowAsJavaScriptException();
91100
}
92101
}
93102

src/executor.hpp renamed to src/executor.h

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,18 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
#ifndef SRC_EXECUTOR_HPP_
16-
#define SRC_EXECUTOR_HPP_
15+
#ifndef SRC_EXECUTOR_H_
16+
#define SRC_EXECUTOR_H_
1717

18+
#include <napi.h>
1819
#include <rcl/wait.h>
1920
#include <uv.h>
2021

2122
#include <atomic>
2223
#include <exception>
2324
#include <vector>
2425

25-
#include "rcl_handle.hpp"
26+
#include "rcl_handle.h"
2627

2728
namespace rclnodejs {
2829

@@ -37,7 +38,7 @@ class Executor {
3738
virtual void CatchException(std::exception_ptr e_ptr) = 0;
3839
};
3940

40-
Executor(HandleManager* handle_manager, Delegate* delegate);
41+
Executor(Napi::Env env, HandleManager* handle_manager, Delegate* delegate);
4142
~Executor();
4243

4344
void Start(rcl_context_t* context, int32_t time_out);
@@ -68,10 +69,11 @@ class Executor {
6869
Delegate* delegate_;
6970
rcl_context_t* context_;
7071
int32_t time_out_;
72+
Napi::Env env_;
7173

7274
std::atomic_bool running_;
7375
};
7476

7577
} // namespace rclnodejs
7678

77-
#endif // SRC_EXECUTOR_HPP_
79+
#endif // SRC_EXECUTOR_H_

0 commit comments

Comments
 (0)