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

Update minimum node version required #552

Merged
merged 1 commit into from
Dec 15, 2018
Merged

Conversation

ralphtheninja
Copy link
Member

@ralphtheninja ralphtheninja commented Dec 15, 2018

Closes #551

I started with v8.0.0 and moved upwards. The problem is related to napi_throw_error() which was added in v8.0.0 but the function signature has changed from two parameters to three.

https://nodejs.org/dist/latest-v10.x/docs/api/n-api.html#n_api_napi_throw_error

Update:

There are more issues, not just related to napi_throw_error().

@ralphtheninja ralphtheninja changed the title Update minimum node version required [wip] Update minimum node version required Dec 15, 2018
@ralphtheninja
Copy link
Member Author

ralphtheninja commented Dec 15, 2018

Going to let this hang for a while. Maybe we can tweak this to make it work on v8.0.0.

We can do this by introducing #ifdef code (checking different node versions) but it would be painful and kind of defeats the point of n-api. Lets go with v8.6.0.

@ralphtheninja ralphtheninja changed the title [wip] Update minimum node version required Update minimum node version required Dec 15, 2018
@ralphtheninja ralphtheninja merged commit d08c48f into master Dec 15, 2018
@ralphtheninja ralphtheninja deleted the min-node-version branch December 15, 2018 20:58
@rvagg rvagg mentioned this pull request Dec 18, 2018
wolfy1339 added a commit to hellomouse/GNS that referenced this pull request Jun 20, 2019
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 this pull request may close these issues.

2 participants