search

nodejs / package-maintenance

Repository for work for discussion of helping with maintenance of key packages in the ecosystem.
Other
407 stars 145 forks source link

Suggestion: Promote use of N-API for native addons #52

Open gabrielschulhof opened 5 years ago

gabrielschulhof commented 5 years ago

In light of https://github.com/nodejs/node/pull/22754#issuecomment-425072975 we may want to reach out to native addon maintainers to remind them of the benefits of porting their addon to N-API. We at @nodejs/n-api can support their efforts.

mcollina commented 5 years ago

Absolutely, big big +1. This could be a good item in the todo list.

Have you got a list of popular modules that are going to break because of that, and how their maintainers would feel about a rewrite-to-napi PR?

gabrielschulhof commented 5 years ago

@mcollina https://github.com/nodejs/abi-stable-node/issues/346#issuecomment-426135449

gabrielschulhof commented 5 years ago

@mcollina we need to follow through on some of the things we've already ported. leveldown, for example, was ported early on in the N-API design process, so it basically needs to be ported again.

mcollina commented 5 years ago

My main concern is not about doing the work, but rather in asking the maintainer if they would accept such PR. We should not overstep them and help them only if they are ok with it.

ralphtheninja commented 5 years ago

@gabrielschulhof @mcollina I'm currently working on a n-api implementation of leveldown. Gonna need a few days more before review. I'm guessing accepting such a PR would be quite overwhelming as a maintainer. I prefer to do it myself and maybe instead ask for help/assistance if I run into dragons. As a maintainer it gives me good incentive to learn more about the code base and the c++ side has always been sort of off-limits for me and other maintainers.

mhdawson commented 5 years ago

+1 to this suggestion as well (full disclosure I have been a supporter/participant of N-API from the start :)).

mogill commented 5 years ago

Porting from NAN to N-API is probably not an awareness issue but more of a cost-benefit problem. NAN and N-API are functionally equivalent: unless the module succumbs to NAN code-rot there is no value to investing time porting from NAN to N-API, especially if the port is non-trivial.

Of course NAN modules are already breaking, which leads back to the core issue of this package-maintenance issue: if a module's maintainer already isn't keeping up with functionality issues, the NAN to N-API conversion is, at best, a treadmill. It makes little sense for an unpaid, uninterested package maintainer to spend hours or days learning N-API when N-API experts are already available to help. For those modules awareness of where to get help doing the work is far more valuable than awareness of the tools and details of the work itself.

The fact Node.js' maintainers already support NAN/N-API conversion for some native modules is news to me. What makes a module eligible for support from @nodejs/n-api? Can package maintainers request help? If so, what is that process?

ghinks commented 5 years ago

I think I'm echoing @mcollina and @mhdawson in that we need to identify the modules first. But this is great, I may at last be able to use some of my lost decades of C++ skills again. Maybe we should just identify the top compiled modules and offer assistance to convert them as previously suggested. It always has to start with diplomacy.

gabrielschulhof commented 5 years ago

@ghinks we have a list of the biggest hitters in https://github.com/nodejs/abi-stable-node/issues/346#issuecomment-426135449.

ghinks commented 5 years ago

excellent, I took a look at npm time (as it looked easier, and its been a while since I wrote C++, so I wanted easier). I forked and created a branch and migrated it to napi here. It seems to be all working on napi but as we form teams I'm sure we can get better at reviewing the api changes. I'll need help setting it up on travis for all the different OSs. But it is building and passing all the tests on linux on various node versions. When we have our next meeting I'm sure we can get organized about how to do this efficiently going forwards, like contacting owners etc.

zeke commented 5 years ago

Here's a(n outdated) list of top dependents of nan by dependent count: https://github.com/nice-registry/native-modules#readme -- This could be a useful list for prioritizing which modules authors to reach out to.

There was an effort among to native module maintainers a while back to consolidate work into an org: https://github.com/prebuild -- all the discussion around it happened in this issue: https://github.com/prebuild/welcome/issues/1

ghinks commented 5 years ago

I have taken a suggestion from @mhdawson and contacted a friendly at the serialport package and am looking into converting that NAN interfaces to use NAPI. I have exchanged emails with the maintainer and they are a candidate customer.

ghinks commented 5 years ago

I have additional findings which I am going to follow up with the n-api group and the maintainers group. There are some particular findings I have about updating via node-addon-api to replace nan and direct lib uv usage.

jonchurch commented 3 years ago

There's some really good stuff in this issue, marking as stale? to hopefully get someone to come dust this off.