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

New Node.js NAPI #202

Closed
RReverser opened this issue Apr 5, 2017 · 17 comments
Closed

New Node.js NAPI #202

RReverser opened this issue Apr 5, 2017 · 17 comments

Comments

@RReverser
Copy link
Contributor

RReverser commented Apr 5, 2017

Now that Node.js has brought support for a native API, ABI-compatible across future Node.js versions or even different Node.js engines, are there any plans to use it instead of direct V8 API for Neon's bindings?

It looks like it could benefit the project, although unclear if it covers all the functionality Neon requires yet.

@Qard
Copy link

Qard commented Apr 12, 2017

It should be noted that it's feature flagged for now, but I'd definitely say it's worth looking into since it'll be a stable target soon and can be used in pure C, so neon could make better use of Rust FFI rather than having to maintain neon.cc. 😸

@Awk34
Copy link

Awk34 commented May 31, 2017

It was released with Node 8.0.0 yesterday :D
https://nodejs.org/en/docs/guides/publishing-napi-modules/

@Qard
Copy link

Qard commented May 31, 2017

Released, but behind a --napi-modules flag.

@RReverser
Copy link
Contributor Author

@Qard Doesn't prevent from playing it (with using cfg(feature) for conditional compilation on Neon side).

@Qard
Copy link

Qard commented May 31, 2017

Yep, just wanted to be clear it's still behind a flag, so not production-ready yet, and could change at any time.

@dherman
Copy link
Collaborator

dherman commented Oct 11, 2017

I agree this is something we should eventually support, and I'd welcome experimentation! We can't switch to it overnight, but if NAPI pans out I would absolutely support switching to it when the time is right.

@Qard
Copy link

Qard commented Oct 11, 2017

N-API is no longer flagged as of Node.js 8.6.0, by the way. 🎉

https:/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V8.md#2017-09-26-version-860-current-jasnell

@dherman
Copy link
Collaborator

dherman commented Oct 11, 2017

@Qard Nice! It's still marked as experimental and I don't really know how much it's likely to change in practice. It looks like 8.x will become LTS in a few weeks if I'm reading the schedule right, so we could technically switch to that soon. But it's still pretty new so I think it doesn't hurt to give it some time.

Anyway, none of that precludes us experimenting with this! I don't think it's the best use of my time right now, since we have bigger issues to fix (especially the safety holes, the high level macro syntax, and fleshing out the API) first. But if anyone is interested in working on this I will be happy to discuss it with them and actively mentor them.

@RReverser
Copy link
Contributor Author

I could try to start implementing this after the next week unless someone wants to do it earlier.

@RReverser
Copy link
Contributor Author

I would definitely not mark this as beginner friendly, looks like it won't be that easy to preserve API compatibility without hacks.

@muditsaurabh
Copy link

@RReverser Any update on this issue.

@fluxxu
Copy link

fluxxu commented Mar 8, 2018

For reference:
https:/atom/xray/tree/master/napi

@devsnek
Copy link

devsnek commented Mar 14, 2018

napi is considered stable at this point: nodejs/node@cd7d7b1

@RReverser
Copy link
Contributor Author

I did look at extending Neon back in the day, but it seemed relying on V8 implementation details too much to have both implementations for V8 and NAPI. Now that NAPI is stable, full rewrite to get away from V8 API should be likely easier.

@devsnek
Copy link

devsnek commented Mar 5, 2019

any word on this?

@amilajack
Copy link
Member

There are plans for this but it will likely be a while support for it gets added. The main focus as of now is to improve the existing API and DX

@amilajack
Copy link
Member

Duplicate of #444

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants