-
Notifications
You must be signed in to change notification settings - Fork 88
Migrate project from NAN to Node-API for improved compatibility - #1088
Migrate project from NAN to Node-API for improved compatibility #1088SanjayUG wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR migrates the project from NAN to Node-API to improve compatibility and modernize the native addon bindings. Key changes include:
- Replacing NAN header inclusions with Node-API headers.
- Updating class inheritance and method signatures to align with Node-API constructs.
- Modifying build configurations in binding.gyp to integrate Node-API properly.
Comments suppressed due to low confidence (2)
binding.gyp:31
- The removal of '.' from the include_dirs array may affect the lookup of local headers; verify that this change is intentional and that all required directories are still being included.
<!@(node -p "require(\"node-addon-api\").include")
binding.gyp:132
- [nitpick] The addition of 'scripts/config.js' and the Node-API gyp dependency should be verified to ensure they are necessary for the migration; confirm that these dependencies are available and correctly configured in the project.
'dependencies': [
minggangw
commented
Mar 14, 2025
Just wondering if we could leverage AI agent to convert most of the code from NAPI to Node-API 😄
SanjayUG
commented
Mar 14, 2025
Just wondering if we could leverage AI agent to convert most of the code from NAPI to Node-API 😄
Glad for your reply,
I believe using AI in efficient way will help in the problem solving faster.
SanjayUG
commented
Mar 14, 2025
Can you just check it now,
I just modified the following things,
- Header File: Included node_api.h in both files.
- Initialization Function: Updated the initialization function to use Node-API conventions.
- Module Registration: Used NAPI_MODULE macro for module registration.
i welcome you with some feedback to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice that a consistent failure on Actions https://github.com/RobotWebTools/rclnodejs/actions/runs/13857494263/job/38777515813?pr=1088#step:7:25, can you pass the compiling locally?
Hello there, This pr is the contribution to issue #1036,
Here's a summary of the changes made to migrate the project from nan to Node-API:
src/addon.cpp
Replaced #include <nan.h> with #include <node_api.h>.
Updated the IsRunningInElectronRenderer function to use Node-API functions instead of nan functions.
Modified the InitModule function to use Node-API for module initialization.
src/shadow_node.hpp
Replaced #include <nan.h> with #include <node_api.h>.
Changed the class inheritance from Nan::ObjectWrap to Napi::ObjectWrap.
Updated method signatures to use Napi::CallbackInfo instead of Nan::FunctionCallbackInfov8::Value.
Replaced Nan::Persistentv8::Function with Napi::FunctionReference.
src/rcl_handle.cpp
Replaced #include <nan.h> with #include <node_api.h>.
Changed the class inheritance from Nan::ObjectWrap to Napi::ObjectWrap.
Updated method signatures to use Napi::CallbackInfo instead of Nan::FunctionCallbackInfov8::Value.
Replaced Nan functions with their Node-API equivalents in the Init, New, SyncProperties, PropertiesGetter, Release, Dismiss, and NewInstance methods.
src/rcl_bindings.cpp
Replaced #include <nan.h> with #include <node_api.h>.
Updated the Init and CreateNode methods to use Node-API functions instead of Nan functions.
src/rcl_action_bindings.cpp
Updated the ActionCreateServer method to use Node-API instead of nan.
If any feedback feel free to mention.