[DRAFT] Add Node-API to Hermes

[vmoroz]

Summary

This is an initial implementation of Node-API for Hermes.
The code is taken from the hermes-windows repo.

Node-API is an ABI-safe that is originally implemented for Node.js addons, and then adopted by all major JS runtimes.

This is a draft PR. Before removing the draft status I would like to ask a few questons:

• What must be the library that exposes the Node-API? In hermes-windows we expose it for the hermes.dll. It is the libhermes for Windows. In this repo libhermes is not a shared library.
• Please advise on the source file names and their locations.
• What must be file headers?
• I am going to add the test files after figuring out what must be the library that exposes Node-API.

Test Plan

Unit tests for Node-API are to be added.

[tmikov]

Thanks for doing this @vmoroz!!!

[facebook-github-bot]

@tmikov has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[shirakaba]

Thanks so much, @vmoroz! Really excited to play with this.

[NathanWalker]

Huge effort @vmoroz thank you ❤️

[neildhar]

Hey @vmoroz, thanks for putting this together!

Two high level requests:

1. Could you move all the new code into the node-api directory? It can produce a static library that we link into libhermes, but I think keeping the implementation of this API separate makes more sense (similar to how the hermes_abi implementation is separate).
2. As a first step, could we just include the implementations of the actual NAPI APIs? That is, we should exclude the extensions for providing things like CDP integration and access to Hermes specific functionality. We want to support these things eventually, but we haven't decided on what the ABI boundary should look like.

[vmoroz]

Hey @vmoroz, thanks for putting this together!

Two high level requests:

1. Could you move all the new code into the node-api directory? It can produce a static library that we link into libhermes, but I think keeping the implementation of this API separate makes more sense (similar to how the hermes_abi implementation is separate).
2. As a first step, could we just include the implementations of the actual NAPI APIs? That is, we should exclude the extensions for providing things like CDP integration and access to Hermes specific functionality. We want to support these things eventually, but we haven't decided on what the ABI boundary should look like.

@neildhar , thank you for the feedback and suggestions!
Extracting the Node API related code into the node-api folder similar to the hermes_abi code makes perfect sense. I will do that.

[wcandillon]

Is this the main place to follow work on this topic? Is there still some appetite to support NAPI in React Native?
We are eyeing at some node modules that we would love to use out of the box in React Native.

[shirakaba]

I'm still very interested in it and have even submitted conference talk proposals on the topic so would love it to become a reality!

I have some native Node addons that I've written for Electron, and it's a shame that I can use them in React Native Windows (as it supports Node-API) yet can't use them in any other flavour of React Native currently.

Let me know if there's any way I could help!

[vmoroz]

Is this the main place to follow work on this topic? Is there still some appetite to support NAPI in React Native? We are eyeing at some node modules that we would love to use out of the box in React Native.

One of the Discord RN channels or a discussion topic in one of the Meta's or Microsoft's related repo could be a better place to discuss all the details, but we can start it here.

The goal of supporting Node-API for Hermes, V8, and RN was to provide the industry standard ABI-safe API. It should enable versioning support for modules, support for multiple programming languages, and reuse of native code written for Node.js.

I wonder what is your scenario in terms of tergeting platforms (Windows, Mac, Android, etc.) and if you own the code that uses Node-API? It may be not possible to reuse binary Node.js modules, but reusing the majority of the code and recompiling it for the RN is a more achievable scenario.

[wcandillon]

We are implementing WebGPU for React Native ( https://x.com/appjsconf/status/1806332497783058900) and right now we are virtually migrating a webgpu node module to JSI manually. So of course the thought of being able to use that node module directly is appealing to us.

…

On Thu 11 Jul 2024 at 18:09, Vladimir Morozov ***@***.***> wrote: Is this the main place to follow work on this topic? Is there still some appetite to support NAPI in React Native? We are eyeing at some node modules that we would love to use out of the box in React Native. One of the Discord RN channels or a discussion topic in one of the Meta's or Microsoft's related repo could be a better place to discuss all the details, but we can start it here. The goal of supporting Node-API for Hermes, V8, and RN was to provide the industry standard ABI-safe API. It should enable versioning support for modules, support for multiple programming languages, and reuse of native code written for Node.js. I wonder what is your scenario in terms of tergeting platforms (Windows, Mac, Android, etc.) and if you own the code that uses Node-API? It may be not possible to reuse binary Node.js modules, but reusing the majority of the code and recompiling it for the RN is a more achievable scenario. — Reply to this email directly, view it on GitHub <#1377 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACKXVUXFX6NCBLYT5AIRILZL2U2FAVCNFSM6AAAAABGMIZABGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRTGMZTKMBYGM> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[vmoroz]

We are implementing WebGPU for React Native ( https://x.com/appjsconf/status/1806332497783058900) and right now we are virtually migrating a webgpu node module to JSI manually. So of course the thought or being able to use that node module directly is appealing to us.

This is awesome! In the video clip you say that you target first of all the mobile platforms: iOS and Android.
It means that you cannot use the precompiled binaries and targeting the Node-API is mostly the matter of the convenience of reusing the existing code. IMHO, it is an achievable goal and @shirakaba's team is already using the Hermes Node-API for their project. The only problem is that to use it we must modify the Hermes code to integrate with the jsi::Runtime.
This PR targets to eliminate this issue and to enable Node-API out of the main Hermes repo.

The current version of the Node-API for Hermes is in the hermes-windows repo: https://github.com/microsoft/hermes-windows/ . There we are starting to factor out the Node-API code into its own folder.

Yesterday we had a long informative face-to-face meeting with Hermes team.
The Hermes team seems to be still in the favor of supporting Node-API. There was an interesting discussion about the interoperability between Node-API and jsi::Runtine/AbiRuntime. There are a few issues to address there. Hopefully we can do it soon.

[tmikov]

@wcandillon when you say that you are implementing WebGPU, do you literally mean the WebGPU spec, 1 to 1, with shader language, etc?

[wcandillon]

We use Google Dawn for the native WebGPU implementation (Dawn is also a supported Skia backend), we code generate most of the bindings from its TS definition (which is itself generated from the W3C spec, we also match TS methods to C++ methods, Dawn provides a json model for that), and for the manual part of the work we just look at the node module and migrate it. This is why being able to use that node binding directly (which is already conformant to the official WebGPU testsuite) feels appealing.

[tmikov]

May I emphatically object to using Discord channels, as they inherently make discussions exclusionary, unfriendly to different timezones, transient, difficult to search, and impossible to reference.

[SamuelScheit]

For anyone who wants to try out this PR with react-native use version 0.75.0-rc.5 (or newer).

[SamuelScheit]

I was able to link and run a react-native app with this hermes NodeAPI PR.

However after calling hermes_create_napi_env it fails with EXC_BAD_ACCESS and the following stacktrace:

hermes::CompactArray::Fixed<unsigned char>::get(char*, unsigned int) [inlined]
hermes::CompactArray::get(unsigned int) const [inlined]
hermes::CompactTable::isEmpty(unsigned int) const [inlined]
unsigned int hermes::vm::detail::IdentifierHashTable::lookupString<char>(llvh::ArrayRef<char>, unsigned int, bool) consthermes_node_api/lib/VM/detail/IdentifierHashTable.cpp:41
hermes::vm::SymbolID hermes::vm::IdentifierTable::registerLazyIdentifierImpl<char>(llvh::ArrayRef<char>, unsigned int)hermes_node_api/lib/VM/IdentifierTable.cpp:393
hermes::napi::NapiEnvironment::NapiEnvironment(hermes::vm::Runtime&, bool, std::__1::shared_ptr<facebook::jsi::PreparedScriptStore>, hermes::vm::RuntimeConfig const&)Users/user/Developer/respond/hermes/hermes_node_api/API/hermes/hermes_napi.cpp:3330
hermes::napi::NapiEnvironment::NapiEnvironment(hermes::vm::Runtime&, bool, std::__1::shared_ptr<facebook::jsi::PreparedScriptStore>, hermes::vm::RuntimeConfig const&) [inlined]
::hermes_create_napi_env(void *, napi_env *)

raw is a NULL pointer and therefore results in EXC_BAD_ACCESS

hermes/include/hermes/ADT/CompactArray.h

Lines 85 to 87 in 175d85f

	static uint32_t get(char *raw, uint32_t idx) {
	return reinterpret_cast<T *>(raw)[idx];
	}

hermes/include/hermes/ADT/CompactArray.h

Lines 53 to 57 in 175d85f

	uint32_t get(uint32_t idx) const {
	assert(idx < size_);
	switch (scale_) {
	case UINT8:
	return Fixed<uint8_t>::get(raw_, idx);

hermes/include/hermes/ADT/CompactArray.h

Lines 131 to 133 in 175d85f

	bool isEmpty(uint32_t idx) const {
	return CompactArray::get(idx) == EMPTY;
	}

hermes/lib/VM/detail/IdentifierHashTable.cpp

Line 41 in 175d85f

if (table_.isEmpty(idx)) {

hermes/lib/VM/IdentifierTable.cpp

Line 331 in 175d85f

auto idx = hashTable_.lookupString(str, hash);

https://github.com/vmoroz/hermes-windows/blob/5d4656887af7397700006e1beec28dc9abd3333f/API/hermes/hermes_napi.cpp#L3327-L3331

so what I'm currently trying to figure out is: why is raw a null pointer and what is the source of this issue

Does anybody else tried to use the NAPI in an app and if yes were you successful or did you encounter errors?

[SamuelScheit]

After building hermes in debug mode I get a different error, however without any stacktrace:

libc++abi: terminating due to uncaught exception of type std::length_error: vector

and after retrying a few times a different error:

libc++abi: terminating due to uncaught exception of type std::bad_alloc

Does anyone know how I can get the stacktrace of these errors to find the cause??

[kraenhansen]

For anyone following along, I want to share a bit of context on the progress related to my previous comments on this PR. A group of developers (including @vmoroz, @mani3xis, @matthargett, @grabbou, @okwasniewski, @shirakaba, @ammarahm-ed and @teodorciuraru) has been persuing an effort to implement the runtime specific parts of Node-API for React Native, coordinated through the #react-native-node-api-modules channel on the Callstack discord and on the Callstack incubator's GitHub org: https://github.com/callstackincubator/react-native-node-api-modules

Many of us has had weekly meetings to collaborate: First drafting an RFC for React Native, later transforming it into a technical design and docs in the repository.

We choose to work without submitting a proposal for comments, mainly from the realization that very few (if any) changes was actually needed in React Native core, in a desire to keep the core lean.

In the short/mid term plan for this PR that we've agreed on:

1. @vmoroz will rebase and force-push this PR branch on hermes-2025-03-03-RNv0.79.0-bc17d964d03743424823d7dd1a9f37633459c5c5
2. @vmoroz will also push any outstanding commits that he feels are fairly stable
3. We will then PRs against this PR branch (for the sake of transparency and to limit rework) and allow discussion on each proposed changes in isolation. To be clear those PRs will be created on his fork: https://github.com/vmoroz/hermes-windows/pulls
4. As the PR of this branch gets closer to addressing the outstanding comments and todos, it will be moved out of draft.

[tmikov]

@kraenhansen a couple of quick points:

1. With the current design, simultaneous n-api and JSI usage in the same runtime is not entirely safe and can cause native crashes.
2. Is there a goal to eventually land n-api in "mainline" Hermes? See Adding Node-API support to Static Hermes #1610. It makes more sense to target the "static_h" branch.

[facebook-github-bot]

@vmoroz has updated the pull request. You must reimport the pull request before landing.

[vmoroz]

@tmikov

1. We are going to refactor the Node-API for Hermes code that it must be safe to use it along with the JSI and the ABI JSI code. The key is that the node_api instances must be created for an existing instance of Hermes runtime. We will remove all APIs that are not part of the standard Node-API. E.g. any APIs that are responsible for creating the Hermes runtime. I wonder what are the other unsafe patterns that you see.
2. As for the requirements in Adding Node-API support to Static Hermes #1610: and the "static_h" branch:
   • I had moved the Node-API implementation into the separate folder: API/node_api_hermes to follow the same style as the other folders in the API folder.
   • Before we go out of the [DRAFT] mode we are going to add unit tests. IMHO, we must not merge this PR without unit tests. While we have unit tests for Node-API in hermes-windows, I believe we must improve them for this repo and this work is currently in progress.
   • Since it is not possible to change the PR target (at least I do not know how to do it), we will create another one for the "static_h" branch. I would propose to do it after we add tests and address all major outstanding TODOs.

[facebook-github-bot]

@vmoroz has updated the pull request. You must reimport the pull request before landing.

[facebook-github-bot]

@vmoroz has updated the pull request. You must reimport the pull request before landing.

[facebook-github-bot]

@vmoroz has updated the pull request. You must reimport the pull request before landing.

[facebook-github-bot]

@vmoroz has updated the pull request. You must reimport the pull request before landing.

[neildhar]

Hey @vmoroz, we're excited about the possibility of extending Hermes with NAPI and are eager to accept this PR! How close are you to having it ready for review?

[vmoroz]

Hey @neildhar, great to hear from you!
My main showstopper is the set of the unit tests, and I am so close to their completion.
My estimate is to be done with them in a couple of weeks or so.
They find some issues that must be fixed along the way.

It was very easy to re-target the PR for the static_h branch instead of main.
I will do the proper rebase as soon as the testing suite is ready.

[tmikov]

For some additional context, we think n-api might be great for implementing builtin functionality like the new Temporal proposal, or even Intl, etc. In other words, we are eager to adopt it and use it ourselves for library functionality.

It should also make it dramatically easier for people to contribute library code to Hermes using a well known and stable API. (JSI is also an option for this, but it somewhat less suited because it relies on exceptions and RTTI, while Hermes itself is compiled without them.)

[vmoroz]

For some additional context, we think n-api might be great for implementing builtin functionality like the new Temporal proposal, or even Intl, etc. In other words, we are eager to adopt it and use it ourselves for library functionality.

It should also make it dramatically easier for people to contribute library code to Hermes using a well known and stable API. (JSI is also an option for this, but it somewhat less suited because it relies on exceptions and RTTI, while Hermes itself is compiled without them.)

Oh, wow! It sounds exciting. OK, It is going to be my top priority until I complete it then. Thank you! :)