-
Notifications
You must be signed in to change notification settings - Fork 459
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue 602: Add template methods to ObjectWrap #604
Issue 602: Add template methods to ObjectWrap #604
Conversation
ObjectWrap<T> was enhanced to support template methods for defining properties and methods of JS class. Now C++ methods and functions may be passed as template parameters for ObjectWrap<T>::InstanceMethod, ObjectWrap<T>::StaticAccessor, etc. There are several benefits: - no need to allocate extra memory for passing C++ function napi callback and use add_finalizer() to free memory; - a compiler can see whole chain of calls up to napi callback that may allow better optimisation. Some examples: ```cpp // Method InstanceMethod<&MyClass::method>("method"); // Read-write property InstanceAccessor<&MyClass::get, &MyClass::set>("rw_prop"); // Read-only property InstanceAccessor<&MyClass::get>("ro_prop"); ``` Closes #602.
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.
LGTM
We should consider moving the existing methods to
@nodejs/n-api what do y'all think? |
IMO there are no really breaking changes landed on most recent major version v2. I'd prefer the latter less aggressive pattern for the common usage on |
I'm good with the move to napi-inl.deprecated.h, removing completely we can discuss later. We talked about adding a new deprecation guard so that people who already have napi_deprecated are not affected, but people can opt-in to the the higher level of deprecation. In this case we'll use: napi_disable_deprecated_1 napi_disable_deprecated will map to napi_disabled_deprecated_0 |
@mhdawson I think it would OK to land this PR as is, and then leave the move to deprecated to another PR. |
CI against v8.x: https://ci.nodejs.org/job/node-test-node-addon-api-new/1217/ |
Here it is again: CI against v8.x: https://ci.nodejs.org/job/node-test-node-addon-api-new/1218/ |
ObjectWrap<T> was enhanced to support template methods for defining properties and methods of JS class. Now C++ methods and functions may be passed as template parameters for ObjectWrap<T>::InstanceMethod, ObjectWrap<T>::StaticAccessor, etc. There are several benefits: - no need to allocate extra memory for passing C++ function napi callback and use add_finalizer() to free memory; - a compiler can see whole chain of calls up to napi callback that may allow better optimisation. Some examples: ```cpp // Method InstanceMethod<&MyClass::method>("method"); // Read-write property InstanceAccessor<&MyClass::get, &MyClass::set>("rw_prop"); // Read-only property InstanceAccessor<&MyClass::get>("ro_prop"); ``` Fixes: #602 PR-URL: #604 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Landed in ce91e14. |
ObjectWrap<T> was enhanced to support template methods for defining properties and methods of JS class. Now C++ methods and functions may be passed as template parameters for ObjectWrap<T>::InstanceMethod, ObjectWrap<T>::StaticAccessor, etc. There are several benefits: - no need to allocate extra memory for passing C++ function napi callback and use add_finalizer() to free memory; - a compiler can see whole chain of calls up to napi callback that may allow better optimisation. Some examples: ```cpp // Method InstanceMethod<&MyClass::method>("method"); // Read-write property InstanceAccessor<&MyClass::get, &MyClass::set>("rw_prop"); // Read-only property InstanceAccessor<&MyClass::get>("ro_prop"); ``` Fixes: nodejs/node-addon-api#602 PR-URL: nodejs/node-addon-api#604 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
ObjectWrap<T> was enhanced to support template methods for defining properties and methods of JS class. Now C++ methods and functions may be passed as template parameters for ObjectWrap<T>::InstanceMethod, ObjectWrap<T>::StaticAccessor, etc. There are several benefits: - no need to allocate extra memory for passing C++ function napi callback and use add_finalizer() to free memory; - a compiler can see whole chain of calls up to napi callback that may allow better optimisation. Some examples: ```cpp // Method InstanceMethod<&MyClass::method>("method"); // Read-write property InstanceAccessor<&MyClass::get, &MyClass::set>("rw_prop"); // Read-only property InstanceAccessor<&MyClass::get>("ro_prop"); ``` Fixes: nodejs/node-addon-api#602 PR-URL: nodejs/node-addon-api#604 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
ObjectWrap<T> was enhanced to support template methods for defining properties and methods of JS class. Now C++ methods and functions may be passed as template parameters for ObjectWrap<T>::InstanceMethod, ObjectWrap<T>::StaticAccessor, etc. There are several benefits: - no need to allocate extra memory for passing C++ function napi callback and use add_finalizer() to free memory; - a compiler can see whole chain of calls up to napi callback that may allow better optimisation. Some examples: ```cpp // Method InstanceMethod<&MyClass::method>("method"); // Read-write property InstanceAccessor<&MyClass::get, &MyClass::set>("rw_prop"); // Read-only property InstanceAccessor<&MyClass::get>("ro_prop"); ``` Fixes: nodejs/node-addon-api#602 PR-URL: nodejs/node-addon-api#604 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
ObjectWrap<T> was enhanced to support template methods for defining properties and methods of JS class. Now C++ methods and functions may be passed as template parameters for ObjectWrap<T>::InstanceMethod, ObjectWrap<T>::StaticAccessor, etc. There are several benefits: - no need to allocate extra memory for passing C++ function napi callback and use add_finalizer() to free memory; - a compiler can see whole chain of calls up to napi callback that may allow better optimisation. Some examples: ```cpp // Method InstanceMethod<&MyClass::method>("method"); // Read-write property InstanceAccessor<&MyClass::get, &MyClass::set>("rw_prop"); // Read-only property InstanceAccessor<&MyClass::get>("ro_prop"); ``` Fixes: nodejs/node-addon-api#602 PR-URL: nodejs/node-addon-api#604 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
ObjectWrap was enhanced to support template methods for
defining properties and methods of JS class. Now C++ methods and
functions may be passed as template parameters for
ObjectWrap::InstanceMethod, ObjectWrap::StaticAccessor, etc.
There are several benefits:
napi callback and use add_finalizer() to free memory;
that may allow better optimisation.
Some examples:
Closes #602.
Use squash method.