Repository navigation
N-API Support for node.bcrypt.js #513
Description
The recently announced Node 8 has a new experimental feature called N-API which is aimed at reducing maintenance cost for node native addons. Checkout this blog for more details on its benefits.
node.bcrypt.js is one of the top 30 native modules by dependency count, and in order to help the Node.js community make this important transition to N-API, we are hoping you will be able to work with us in order to port node.bcrypt.js to support N-API. Your support and feedback is important in making this effort a success.
I am part of the N-API working group and and would like to talk to you about how you can can get started with N-API and what help we might be able to provide. I'm available to talk on the phone individually if you'd like. Alternatively, if you prefer, feel free to jump in our WG meeting which happens every Thursday at 1.30 Eastern / 10.30 Pacific US time, to discuss any issues.
Here’s a video of N-API in action
https://www.youtube.com/watch?v=nmXhJ88nZsk
Activity
recrsn commentedon Jun 13, 2017
I tried to get it work. We heavily depend on
AsyncWorkerbut I couldn't find much docs about it.And due to other commitments, I could not spend much time on it.
aruneshchandra commentedon Jun 13, 2017
You are right there is not a very detailed documentation on
AsyncWorker.. however the following header file can help you understand the usagehttps://github.com/nodejs/node-addon-api/blob/master/napi.h#L1362
Alternatively, do you mind talking to us during our Weekly WG meeting which happens every Thursday at 1.30 Eastern / 10.30 Pacific US time, to discuss some ways we can help you to get started on this port.
aruneshchandra commentedon Jul 13, 2017
//cc @kfarnung - please update the documentation for
AsyncWorkerNickNaso commentedon Oct 9, 2017
Hi,
today I completed the porting of bcrypt from NAN to N-API using node-addon-api the code is here https://github.com/NickNaso/node.bcrypt.js/tree/node-addon-api
All tests successfully pass. Maybe you can consider to integrate the code and with some additional work on documentation and package.json we can publish the N-API version of bcrypt I will be happy to collaborate.
aruneshchandra commentedon Oct 9, 2017
//cc: @agathver, @mhdawson, @digitalinfinity
recrsn commentedon Oct 9, 2017
Great work @NickNaso
Please open a PR so that we can continue to iterate there
NickNaso commentedon Oct 9, 2017
Hi @agathver
I think that we need to create a branch so I can execute the PR on it. What do you think?
recrsn commentedon Oct 9, 2017
@NickNaso Yes, created a branch called "napi".
mhdawson commentedon Oct 11, 2017
@NickNaso thanks for doing this.
If you don't mind some info on how long it took you (The commits are over 4-5 days) to do the port along with highlighting any things you found more tricky would be good as data for people looking at doing other ports.
NickNaso commentedon Oct 12, 2017
Hi everyone,
I just returned at home I really happy to share my experience on porting bcrypt to N-API
All the translation process has been taken 5 days of my spare time. In total 10 / 15 hours of work.
The most difficult task was to create and use AsyncWorker in particular I don't know how to call the callback from the OnOk method but I saw at code here https://github.com/nodejs/node-addon-api/blob/master/napi.h#L1433 and to understand how to better use it I looked at this porting https://github.com/sampsongao/leveldown/tree/napi
There aren't examples of usage AsyncWorker here https://github.com/nodejs/abi-stable-node-addon-examples
To understand how to handle the errors I used the examples from the test https://github.com/nodejs/node-addon-api/blob/master/test/error.cc
As reference I followed the N-API documentation here https://nodejs.org/dist/latest-v8.x/docs/api/n-api.html
It was not very difficult but with more useful documentation could be more simple and fast.
NickNaso commentedon Oct 17, 2017
Hi,
today I updated the code and just fixed some compilation error that I received on Windows platform and now all work fine on macOS - linux - Windows with Node.js version 8, but in Node.js version previous the 8 the module is compiled well but I receive an error when I require it.
Just post an example
digitalinfinity commentedon Oct 17, 2017
Hi @NickNaso to clarify- you're using the binary that was compiled against NAPI as implemented in Node 8.6+? On downlevel, I believe the story is you need to use the node-addon-api package for compat with old node versions. Which version of Node do you get the above error in?
10 remaining items
mhdawson commentedon Jan 30, 2018
@agathver thanks!
recrsn commentedon Apr 6, 2018
@NickNaso node-pre-gyp has got napi support. Will you be willing to contribute a patch adding it to the napi branch?
NickNaso commentedon Apr 6, 2018
@agathver I will do it in this weekend and I will open a PR on napi branch.
NickNaso commentedon Jan 24, 2019
Hi @agathver,
I worked on porting the updates on the NAN version of the add-on to the N-API release. I added the support for node-pre-gyp.
Please when you have time see the PR: #698
NickNaso commentedon Feb 18, 2019
Hi everyone,
Is there a plan to switch to N-API for this module? In case how can I help?
recrsn commentedon Mar 8, 2019
The plan is to maintain both versions of the module for now, till Node 8 remains supported.
NickNaso commentedon Mar 8, 2019
Ok I will try to maintain the N-API version in sync with the classical version.
DRoet commentedon Oct 26, 2019
Any plans for a
4.x.xrelease that uses N-API by default? now thatnode 8is almost EOL.recrsn commentedon Oct 29, 2019
That's on the roadmap for this November.
recrsn commentedon Feb 19, 2020
bcryptis now using N-API by defaultmhdawson commentedon Feb 19, 2020
@agathver thanks for the update !