-
Notifications
You must be signed in to change notification settings - Fork 465
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
Build fails on macOS and Linux (and possibly Windows), with node release 8.11.1 and lower, for node-addon-api 1.6.0 #387
Comments
I too have ran into this issue while installing |
Having a similar issue with 1.6.0 release
Getting this error with node 8.12.0, 10.9.0, and 11.1.0 |
Not sure if this is what is causing other peoples errors, but the one I was getting was caused by using the src/node-api.h instead of that in node itself. Removing target_include_directories(${PROJECT_NAME}
PRIVATE
remove this---> ${CMAKE_SOURCE_DIR}/node_modules/node-addon-api/src is all that was needed. |
Same issue here - node.js v8.10.0, node-gyp v3.6.2, node-addon-api v1.6.0. |
Hi everyone, |
Hi @gabrielschulhof @mhdawson, |
@NickNaso What about using the In a broader sense we may need to come up with a forward compatibility strategy for node-addon-api. N-API was so new that versioning differences between Node.js and the C++ wrapper weren't a concern until recently. Even if node-addon-api were to be aligned to the same major version strategy as Node.js, new minor version features will still cause compilation failures. Probably should document the minimum version of Node.js that's needed and whenever that bumps increment the major version of node-addon-api. Node.js 8.10 is definitely going to be needed due to AWS Lambda and the way AWS (unfortunately) locks to a specific minor version of Node. |
I think we have the required version management, we just missed guarding the new functionality with the right defines when we added the CallbackScope support. I should have caught that as part of the review. Looking at fixing it now. |
@mhdawson I updated one of our libraries to point to that specific commit and had a team test a built with it. They reported back that it worked with no errors. Dealing with a few different versions of NPM which makes things fun, but looks good. |
@ebickle thanks ! |
CallbackScope support needs to be guarded with N-API version 3, otherwise olders versions of N-API that did not have CallbackScope support will have compile failures. PR-URL: nodejs/node-addon-api#395 Fixes: nodejs/node-addon-api#387 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Nicola Del Gobbo <nicoladelgobbo@NickNaso.local>
CallbackScope support needs to be guarded with N-API version 3, otherwise olders versions of N-API that did not have CallbackScope support will have compile failures. PR-URL: nodejs/node-addon-api#395 Fixes: nodejs/node-addon-api#387 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Nicola Del Gobbo <nicoladelgobbo@NickNaso.local>
CallbackScope support needs to be guarded with N-API version 3, otherwise olders versions of N-API that did not have CallbackScope support will have compile failures. PR-URL: nodejs/node-addon-api#395 Fixes: nodejs/node-addon-api#387 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Nicola Del Gobbo <nicoladelgobbo@NickNaso.local>
CallbackScope support needs to be guarded with N-API version 3, otherwise olders versions of N-API that did not have CallbackScope support will have compile failures. PR-URL: nodejs/node-addon-api#395 Fixes: nodejs/node-addon-api#387 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Nicola Del Gobbo <nicoladelgobbo@NickNaso.local>
Example:
The text was updated successfully, but these errors were encountered: