Skip to content
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

Bundles node_api.h is outdated #463

Closed
ghost opened this issue Mar 29, 2019 · 9 comments
Closed

Bundles node_api.h is outdated #463

ghost opened this issue Mar 29, 2019 · 9 comments

Comments

@ghost
Copy link

ghost commented Mar 29, 2019

I am writing a large C++ addon and would like to use the node-addon-api but it lacks some of the "C" API's features (thread safe functions in particular) so I need to fallback on the "C" N-API.

Unfortunately, the node-addon-api bundles a too old version of "node_api.h", so I can't mix and match.

Could node-addon-api be updated to use latest N-API C header files ?

@legendecas
Copy link
Member

AFAIK, you could directly include node-api.h in /usr/local/include/node (assuming installed node.js with headers).

@ghost
Copy link
Author

ghost commented Mar 29, 2019

@legendecas But I would like to do this in a platform independent way (that also works on Windows).

@ghost ghost mentioned this issue Mar 29, 2019
4 tasks
@ghost
Copy link
Author

ghost commented Mar 29, 2019

@legendecas Also, I suspect the linker might yield problems if the same c++ code is using two different versions of the same header file in the same build ?

@gabrielschulhof
Copy link
Contributor

@extmchristensen you only need the bundled N-API implementation if you're planning on building against a version of Node.js which does not itself have an implementation of N-API (i.e. pre-6.14.2, IIRC). node-addon-api avoids using the built-in headers if the version of Node.js against which it's building has its own headers.

The reason node-addon-api does not have thread-safe function is not that it's missing from the bundled headers, but that it hasn't been implemented in the C++ wrapper – however, as you noticed, we have #442 that seeks to implement it.

@ghost
Copy link
Author

ghost commented Apr 1, 2019

@gabrielschulhof I don't understand the point that node-addon-api is unneeded for recent node versions.... If I don't add node-addon-api to package.json + binding.gyp, I get compilation errors about napi.h being missing.

@gabrielschulhof
Copy link
Contributor

@extmchristensen node-addon-api is needed if you wish to use Napi::* in your native addon. On recent versions of Node.js, src/node_api.h, src/node_api.cc and all other files under node-addon-api's src/ directory are unneeded, because they are part of Node.js itself.

What versions of Node.js do you plan to support with your native addon?

@ghost
Copy link
Author

ghost commented Apr 3, 2019

@gabrielschulhof On my node 11 windows installation, I can not find the headers in the node installation. itself, but they ("C" headers only, not the C++ nap.h header) are present under node-gyp at .node-gyp\version\include\node

As for your question, I need to support at minimum v10 and v11 with additional support for v8 and v9 as nice to have. I am currently using the C++ node-addon wrapper + a 3rd party napi-thread-safe-callback library to allow async callback from 3rd party thread.

@gabrielschulhof
Copy link
Contributor

@extmchristensen for this range of Node.js versions N-API is built-in. Therefore node-addon-api will not compile the version of N-API it ships. This means that the files in its src/ subdirectory are OK to be out-of-date because they will not be compiled and linked into your add-on. Rather, your addon will link to the N-API implementation provided by Node.js.

@gabrielschulhof
Copy link
Contributor

This issue, along with #509 needs to be closed when Node.js v8.x goes out of scope and we will have removed the src/-provided implementation of N-API.

gabrielschulhof pushed a commit to gabrielschulhof/node-addon-api that referenced this issue Jan 1, 2020
Remove the files associated with the external implementation of N-API.
Making this move is possible because all supported versions of Node.js
now have an internal implementation of N-API.

Fixes: nodejs#463
Fixes: nodejs#509
gabrielschulhof pushed a commit to gabrielschulhof/node-addon-api that referenced this issue Jan 1, 2020
Remove the files associated with the external implementation of N-API
and the v8.x Travis CI testing. This move is possible because of v8.x
EOL, which means that all supported versions of Node.js now have an
internal implementation of N-API.

Fixes: nodejs#463
Fixes: nodejs#509
gabrielschulhof pushed a commit to gabrielschulhof/node-addon-api that referenced this issue Jan 2, 2020
Remove the files associated with the external implementation of N-API
and the v8.x Travis CI testing. This move is possible because of v8.x
EOL, which means that all supported versions of Node.js now have an
internal implementation of N-API.

Fixes: nodejs#463
Fixes: nodejs#509
Fixes: nodejs#640
gabrielschulhof pushed a commit to gabrielschulhof/node-addon-api that referenced this issue Jan 3, 2020
Remove the followings:

* the files associated with the external implementation of N-API
* Travis CI jobs for v8.x and v6.x
* documentation instructing users to add the external N-API
  implementation to their dependencies.
* conversion tool code that adds the external N-API implementation as a
  dependency to the user's addon.

This move is possible because of v8.x EOL, which means that all
supported versions of Node.js now have an internal implementation of
N-API.

Fixes: nodejs#463
Fixes: nodejs#509
Fixes: nodejs#640
kevindavies8 added a commit to kevindavies8/node-addon-api-Develop that referenced this issue Aug 24, 2022
Remove the followings:

* the files associated with the external implementation of N-API
* Travis CI jobs for v8.x and v6.x
* documentation instructing users to add the external N-API
  implementation to their dependencies.
* conversion tool code that adds the external N-API implementation as a
  dependency to the user's addon.

This move is possible because of v8.x EOL, which means that all
supported versions of Node.js now have an internal implementation of
N-API.

Fixes: nodejs/node-addon-api#463
Fixes: nodejs/node-addon-api#509
Fixes: nodejs/node-addon-api#640
PR-URL: nodejs/node-addon-api#643
Reviewed-By: Nicola Del Gobbo <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Marlyfleitas added a commit to Marlyfleitas/node-api-addon-Development that referenced this issue Aug 26, 2022
Remove the followings:

* the files associated with the external implementation of N-API
* Travis CI jobs for v8.x and v6.x
* documentation instructing users to add the external N-API
  implementation to their dependencies.
* conversion tool code that adds the external N-API implementation as a
  dependency to the user's addon.

This move is possible because of v8.x EOL, which means that all
supported versions of Node.js now have an internal implementation of
N-API.

Fixes: nodejs/node-addon-api#463
Fixes: nodejs/node-addon-api#509
Fixes: nodejs/node-addon-api#640
PR-URL: nodejs/node-addon-api#643
Reviewed-By: Nicola Del Gobbo <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
wroy7860 added a commit to wroy7860/addon-api-benchmark-node that referenced this issue Sep 19, 2022
Remove the followings:

* the files associated with the external implementation of N-API
* Travis CI jobs for v8.x and v6.x
* documentation instructing users to add the external N-API
  implementation to their dependencies.
* conversion tool code that adds the external N-API implementation as a
  dependency to the user's addon.

This move is possible because of v8.x EOL, which means that all
supported versions of Node.js now have an internal implementation of
N-API.

Fixes: nodejs/node-addon-api#463
Fixes: nodejs/node-addon-api#509
Fixes: nodejs/node-addon-api#640
PR-URL: nodejs/node-addon-api#643
Reviewed-By: Nicola Del Gobbo <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this issue Aug 11, 2023
Remove the followings:

* the files associated with the external implementation of N-API
* Travis CI jobs for v8.x and v6.x
* documentation instructing users to add the external N-API
  implementation to their dependencies.
* conversion tool code that adds the external N-API implementation as a
  dependency to the user's addon.

This move is possible because of v8.x EOL, which means that all
supported versions of Node.js now have an internal implementation of
N-API.

Fixes: nodejs/node-addon-api#463
Fixes: nodejs/node-addon-api#509
Fixes: nodejs/node-addon-api#640
PR-URL: nodejs/node-addon-api#643
Reviewed-By: Nicola Del Gobbo <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants