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

N-API support #56

Closed
NickNaso opened this issue Mar 14, 2018 · 15 comments
Closed

N-API support #56

NickNaso opened this issue Mar 14, 2018 · 15 comments

Comments

@NickNaso
Copy link
Contributor

Node 8 has a new experimental feature called N-API which is aimed at reducing maintenance cost for node native addons. Check out this blog for more details on its benefits.

microtime is a popular module, and in order to help the Node.js community make the important transition to N-API, we are hoping you will be able to work with us in order to migrate microtime to N-API. Your support and feedback is important in making this effort a success.

We (N-API team) have built a preliminary port of microtime on top of N-API as part of our effort to understand the API surface used by native modules. We used this port and the port of some other modules to determine which areas to focus on converting to N-API.
This port has all of the NAN code removed and replaced by N-API equivalents. While it passes all the unit tests, it might need a bit more refinement before getting merged in.
Please take a look if you're interested. I will be happy to answer questions about the port, point to documentation/code, etc. I'm also happy to open a PR if you're happy with the port in it's current shape.

If you like, you can join our WG meeting which happens every Thursday at 1:30 Eastern / 10:30 Pacific US time, to discuss any issues or ask us any questions.

If you don't want to take a dependency on a feature that still has "experimental" status, we understand, and a good option might be to maintain a branch that uses N-API, that is kept mostly up to date with development occurring in microtime/master. We have some precedence to follow with node.bcrypt.js which recently published a N-API version of their module. Of course we'd appreciate feedback about anything we can do to improve the development or migration experience.

You can find my work here: https://github.com/NickNaso/node-microtime/tree/napi

I will be happy to contribute and help in any way.

@wadey
Copy link
Owner

wadey commented Mar 15, 2018

Thank you for your time in starting this patch! I have been looking into N-API a bit but haven’t had the time to fully research it and start working on a port. I will try to review over your branch soon and give you some feedback and keep the ball rolling.

A quick question, how does backwards compatibility with versions of Node prior to 8 work?

@NickNaso
Copy link
Contributor Author

NickNaso commented Mar 15, 2018

Hi @wadey ,
it's compatible with 4 5 6 7 8. Today we landed the PR to get N-API out from the experimental and in the next months we will work to investigate and solve any compatibility problems with the LTS versions.
I think that we need to create a branch called for example "napi" so I can execute the PR on it. After that we can iterate to over it and at the end publish a tagged version of microtime like reported here: https://nodejs.org/en/docs/guides/publishing-napi-modules/
Let me know if you are agree and how you want to proceed.

@mhdawson
Copy link

Compatibility has several dimensions. Using node-addon-api it can compile and run with matching 4 6 and 8 (I'd have to check on the 5 and 7 but @NickNaso probably already did that). To get the compile once/run with later versions that is currently in master, in process for 8.x and 6.x (some backports are required). We aim to get to the point were you can compile on 6.x and then run on later versions.

@wadey
Copy link
Owner

wadey commented Mar 16, 2018

@NickNaso I've created a branch napi that you can open your PR against. We can do the code review there. Thanks!

@NickNaso
Copy link
Contributor Author

NickNaso commented Mar 21, 2018

@wadey Hi all seems work fine if you want take a look at PR #57

@NickNaso
Copy link
Contributor Author

NickNaso commented Jan 24, 2019

microtime is one of the most important native add-on and it is tracked by citgm, but in this issue (nodejs/node#25060) seems that it will not work well in the future version of Node.js (v12).

N-API could help to manage this kind of issues. In general using N-API maintainers do not take care of the changes that will happen in V8 or node.
Please consider the idea to work together on N-API version and release it.

@wadey
Copy link
Owner

wadey commented Jan 27, 2019

ok I would like to resume the work on this and get it released. What is the best doc with the most up to date process for releasing a add-on with N-API support? Or can you summarize what I need to do? Thanks for your help with this.

@wadey
Copy link
Owner

wadey commented Jan 27, 2019

Is this still the best reference? https://nodejs.org/en/docs/guides/publishing-napi-modules/

@wadey
Copy link
Owner

wadey commented Jan 28, 2019

I have published microtime v3.0.0-0 (with tag n-api) with N-API support. Can you please test this and confirm?

Branch is here: https://github.com/wadey/node-microtime/tree/napi

@NickNaso
Copy link
Contributor Author

NickNaso commented Jan 28, 2019

@wadey Sorry if I did not answer at your previous question. I just tried to install and use the tagged version (n-api) and it worked well.
Thanks for your work.

@mhdawson
Copy link

mhdawson commented Jan 28, 2019

So I guess the next question is the steps/possibility of updating so that the main version that people get when they download being the n-api version by default? (if that is not already the case)

@wadey
Copy link
Owner

wadey commented Jan 31, 2019

I wanted to see if I could maintain compatibility with Node v4 and v6 with the new n-api version, and I was able to with 3.0.0-1! I am going to do a little more testing with this and then release without a tag. Thank you!

@mhdawson
Copy link

@wadey thanks!

@BonesyWonesy
Copy link

Thanks!

@wadey
Copy link
Owner

wadey commented Feb 4, 2019

N-API released as v3.0.0 with no tag! Let me know if you have any issues.

@wadey wadey closed this as completed Feb 4, 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

No branches or pull requests

4 participants