-
Notifications
You must be signed in to change notification settings - Fork 33
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 #19
Comments
Buon giorno, it looks like N-API has just moved from "experimental" to "stable" ahead of Node 10, which is great news. Thank you for taking the time to demonstrate how easy migrating to it can be. The primary benefit of N-API to end users, and therefore module maintainers, is the avoidance of Is my understanding of the N-API experience for end-users of each major Node LTS version correct?
Other questions:
|
Hi @lovell ,
At the end you will need to write some part of your code. I hope that my answer is exhaustive. About farmhash if you want experiment with N-API you can create a branch and call it "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 farmhash like reported here: https://nodejs.org/en/docs/guides/publishing-napi-modules/ |
That's a very comprehensive reply Nicola, molte grazie. I think I'd like to see the following before a release of
I've created your suggestion of a |
Thanks for the PR. Anyone that would like to test this can do so with: npm install lovell/farmhash#napi I've subscribed to the highlighted Node 8 and prebuild issues so as soon as they're marked as closed we can merge this and release. I also plan to do some performance testing. |
Hi @lovell I want only notify that last week prebuild added the support for N-API https://www.npmjs.com/package/prebuild#n-api-considerations. |
@NickNaso Thanks, that's great news and I saw that N-API is considered stable in Node 8 too. Happy for a PR with the extra prebuild config if you're able or will get around it myself at some point. |
I've merged the |
Great news I'm very happy to hear this. |
@NickNaso Are you able to help with a build failure with N-API and Node v8 (v10+ is OK), e.g. https://travis-ci.org/lovell/farmhash/jobs/548869980 ? |
It looks like the build failure is due to prebuild-ci using node-abi to determine that the value of I think this means that napi-based modules have to use a more recent version in the |
Hi @lovell, |
That makes sense, thanks for confirming, I'll need to add support for |
I want to add this information: |
Thanks for your help Nick, it's been a bit of a learning exercise for me, e.g. I had to update prebuild-ci to add support for the napi runtime at prebuild/prebuild-ci#18 Anyway, v3.0.0 is now published with prebuilt binaries for N-API ABI version 3, supported by both Node 10 and 12 (and some more recent versions of Node 8, but have decided to drop support for that), with a fallback to the Node ABIs should N-API fail for any reason at install time. Woo! |
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.
farmhash 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 farmhash 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 farmhash 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 farmhash/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/farmhash/tree/napi
I will be happy to contribute and help in any way.
The text was updated successfully, but these errors were encountered: