Skip to content

Conversation

@gabrielschulhof
Copy link
Contributor

This change reduces the places where we declare private functions with
the napi_callback signature for the purpose of using them with C++
callbacks passed as template arguments. We basically have 4 types:

  1. static with void return
  2. static with napi_value return
  3. instance with void return
  4. instance with napi_value return

We can use one of these four calling patterns in the following places
where we accept callbacks as template arguments:

  • Napi::Function (1. and 2.)
  • Napi::PropertyDescriptor (1. for the setter, 2. for the getter)
  • Napi::InstanceWrap<T> (3., 4. for instance methods, 4. for
    instance getters)
  • Napi::ObjectWrap<T> (1., 2. for static methods, 2. for static
    getters)

In the case of InstanceWrap<T> and ObjectWrap<T> instance resp.
static property descriptors we can also remove the infrastructure
designed to allow for optional getters (GetterTag resp.
StaticGetterTag) because the API for specifying instance resp. class
property descriptors does not allow one to omit the getter.

Signed-off-by: @gabrielschulhof

This change reduces the places where we declare private functions with the `napi_callback` signature for the purpose of using them with C++ callbacks passed as template arguments. We basically have 4 types: 1. static with `void` return 2. static with `napi_value` return 3. instance with `void` return 4. instance with `napi_value` return We can use one of these four calling patterns in the following places where we accept callbacks as template arguments: * `Napi::Function` (1. and 2.) * `Napi::PropertyDescriptor` (1. for the setter, 2. for the getter) * `Napi::InstanceWrap<T>` (3., 4. for instance methods, 4. for instance getters) * `Napi::ObjectWrap<T>` (1., 2. for static methods, 2. for static getters) In the case of `InstanceWrap<T>` and `ObjectWrap<T>` instance resp. static property descriptors we can also remove the infrastructure designed to allow for optional getters (`GetterTag` resp. `StaticGetterTag`) because the API for specifying instance resp. class property descriptors does not allow one to omit the getter. Signed-off-by: Gabriel Schulhof <[email protected]>
@gabrielschulhof
Copy link
ContributorAuthor

gabrielschulhof commented Aug 6, 2020

Copy link
Member

@mhdawsonmhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gabrielschulhof
Copy link
ContributorAuthor

gabrielschulhof commented Aug 20, 2020

@mhdawson
Copy link
Member

I saw the AIX problem a few minutes ago while running node-addon-api tests for a different PR. I've pinged @richardlau and @AshCripps to look at getting that back on-line

gabrielschulhof pushed a commit that referenced this pull request Aug 25, 2020
This change reduces the places where we declare private functions with the `napi_callback` signature for the purpose of using them with C++ callbacks passed as template arguments. We basically have 4 types: 1. static with `void` return 2. static with `napi_value` return 3. instance with `void` return 4. instance with `napi_value` return We can use one of these four calling patterns in the following places where we accept callbacks as template arguments: * `Napi::Function` (1. and 2.) * `Napi::PropertyDescriptor` (1. for the setter, 2. for the getter) * `Napi::InstanceWrap<T>` (3., 4. for instance methods, 4. for instance getters) * `Napi::ObjectWrap<T>` (1., 2. for static methods, 2. for static getters) In the case of `InstanceWrap<T>` and `ObjectWrap<T>` instance resp. static property descriptors we can also remove the infrastructure designed to allow for optional getters (`GetterTag` resp. `StaticGetterTag`) because the API for specifying instance resp. class property descriptors does not allow one to omit the getter. Signed-off-by: Gabriel Schulhof <[email protected]> PR-URL: #786 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
@gabrielschulhof
Copy link
ContributorAuthor

Landed in 9aceea7.

@gabrielschulhofgabrielschulhof deleted the simplify-objectwrap-templated-methods-new branch February 1, 2021 16:39
@gabrielschulhofgabrielschulhof restored the simplify-objectwrap-templated-methods-new branch February 1, 2021 16:41
@gabrielschulhofgabrielschulhof deleted the simplify-objectwrap-templated-methods-new branch March 7, 2021 18:44
kevindavies8 added a commit to kevindavies8/node-addon-api-Develop that referenced this pull request Aug 24, 2022
This change reduces the places where we declare private functions with the `napi_callback` signature for the purpose of using them with C++ callbacks passed as template arguments. We basically have 4 types: 1. static with `void` return 2. static with `napi_value` return 3. instance with `void` return 4. instance with `napi_value` return We can use one of these four calling patterns in the following places where we accept callbacks as template arguments: * `Napi::Function` (1. and 2.) * `Napi::PropertyDescriptor` (1. for the setter, 2. for the getter) * `Napi::InstanceWrap<T>` (3., 4. for instance methods, 4. for instance getters) * `Napi::ObjectWrap<T>` (1., 2. for static methods, 2. for static getters) In the case of `InstanceWrap<T>` and `ObjectWrap<T>` instance resp. static property descriptors we can also remove the infrastructure designed to allow for optional getters (`GetterTag` resp. `StaticGetterTag`) because the API for specifying instance resp. class property descriptors does not allow one to omit the getter. Signed-off-by: Gabriel Schulhof <[email protected]> PR-URL: nodejs/node-addon-api#786 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Marlyfleitas added a commit to Marlyfleitas/node-api-addon-Development that referenced this pull request Aug 26, 2022
This change reduces the places where we declare private functions with the `napi_callback` signature for the purpose of using them with C++ callbacks passed as template arguments. We basically have 4 types: 1. static with `void` return 2. static with `napi_value` return 3. instance with `void` return 4. instance with `napi_value` return We can use one of these four calling patterns in the following places where we accept callbacks as template arguments: * `Napi::Function` (1. and 2.) * `Napi::PropertyDescriptor` (1. for the setter, 2. for the getter) * `Napi::InstanceWrap<T>` (3., 4. for instance methods, 4. for instance getters) * `Napi::ObjectWrap<T>` (1., 2. for static methods, 2. for static getters) In the case of `InstanceWrap<T>` and `ObjectWrap<T>` instance resp. static property descriptors we can also remove the infrastructure designed to allow for optional getters (`GetterTag` resp. `StaticGetterTag`) because the API for specifying instance resp. class property descriptors does not allow one to omit the getter. Signed-off-by: Gabriel Schulhof <[email protected]> PR-URL: nodejs/node-addon-api#786 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
wroy7860 added a commit to wroy7860/addon-api-benchmark-node that referenced this pull request Sep 19, 2022
This change reduces the places where we declare private functions with the `napi_callback` signature for the purpose of using them with C++ callbacks passed as template arguments. We basically have 4 types: 1. static with `void` return 2. static with `napi_value` return 3. instance with `void` return 4. instance with `napi_value` return We can use one of these four calling patterns in the following places where we accept callbacks as template arguments: * `Napi::Function` (1. and 2.) * `Napi::PropertyDescriptor` (1. for the setter, 2. for the getter) * `Napi::InstanceWrap<T>` (3., 4. for instance methods, 4. for instance getters) * `Napi::ObjectWrap<T>` (1., 2. for static methods, 2. for static getters) In the case of `InstanceWrap<T>` and `ObjectWrap<T>` instance resp. static property descriptors we can also remove the infrastructure designed to allow for optional getters (`GetterTag` resp. `StaticGetterTag`) because the API for specifying instance resp. class property descriptors does not allow one to omit the getter. Signed-off-by: Gabriel Schulhof <[email protected]> PR-URL: nodejs/node-addon-api#786 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this pull request Aug 11, 2023
This change reduces the places where we declare private functions with the `napi_callback` signature for the purpose of using them with C++ callbacks passed as template arguments. We basically have 4 types: 1. static with `void` return 2. static with `napi_value` return 3. instance with `void` return 4. instance with `napi_value` return We can use one of these four calling patterns in the following places where we accept callbacks as template arguments: * `Napi::Function` (1. and 2.) * `Napi::PropertyDescriptor` (1. for the setter, 2. for the getter) * `Napi::InstanceWrap<T>` (3., 4. for instance methods, 4. for instance getters) * `Napi::ObjectWrap<T>` (1., 2. for static methods, 2. for static getters) In the case of `InstanceWrap<T>` and `ObjectWrap<T>` instance resp. static property descriptors we can also remove the infrastructure designed to allow for optional getters (`GetterTag` resp. `StaticGetterTag`) because the API for specifying instance resp. class property descriptors does not allow one to omit the getter. Signed-off-by: Gabriel Schulhof <[email protected]> PR-URL: nodejs/node-addon-api#786 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@gabrielschulhof@mhdawson@addaleax