[DRAFT] Add Node-API to Hermes #1377
Conversation
|
Thanks for doing this @vmoroz!!! |
|
@tmikov has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
Thanks so much, @vmoroz! Really excited to play with this. |
|
Huge effort @vmoroz thank you ❤️ |
|
Hey @vmoroz, thanks for putting this together! Two high level requests:
|
@neildhar , thank you for the feedback and suggestions! |
|
Is this the main place to follow work on this topic? Is there still some appetite to support NAPI in React Native? |
|
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 Win 8000 dows (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! |
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. |
|
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.
…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: ***@***.***>
|
This is awesome! In the video clip you say that you target first of all the mobile platforms: iOS and Android. 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. |
|
@wcandillon when you say that you are implementing WebGPU, do you literally mean the WebGPU spec, 1 to 1, with shader language, etc? |
|
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. |
|
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. |
…ontEndDefs/Builtins.h +1 Summary: `-Wduplicate-enum` identifies instances where enum categories are assigned the same value. When this is unintentional it is a bug. This diff likely suppresses such an instance using clang diagnostic suppression or `FB_DUPLICATE_ENUM_OKAY`. For questions/comments, contact r-barnes. - If you approve of this diff, please use the "Accept & Ship" button :-) Reviewed By: palmje Differential Revision: D59818696 fbshipit-source-id: 5561a1ff852f31b7d1da9d8689afd4f13eaa87c7
Summary: Release version 0.23.0 Reviewed By: gkz Differential Revision: D59864810 fbshipit-source-id: ac8409d4a0706dbf9a2078bb4a3636f9eb53e8e9
Summary: X-link: facebook/react-native#45507 Changelog: [Internal] Reviewed By: SamChou19815 Differential Revision: D59891129 fbshipit-source-id: c1da0730d6c1b52cce5de730b47b3f3c854dff6b
Summary: Make the CDP domain handler and corresponding tests gracefully handle the case where Hermes is built without memory instrumentation. Reviewed By: dannysu Differential Revision: D59543260 fbshipit-source-id: 6f7fc28a240045b4e1dc441d55dd420c054eb41d
Summary: Mark local functions as static to fix some build configurations. Reviewed By: dannysu Differential Revision: D59561408 fbshipit-source-id: ee0a6dc6d2265ca88af6039fd2a387bd4dcbae1f
Summary: Changelog [flow-api-translator]: Added support for `class A extends (Foo as Bar)` in the flow-api-translator D58577282 added support for the legacy `TypeCastExpression`s, but did not add support for `AsExpression`s. Add support here. Reviewed By: SamChou19815 Differential Revision: D59928177 fbshipit-source-id: 4a1d745428fc016da9dd7004aa8e26d223d09deb
|
For anyone who wants to try out this PR with react-native use version |
|
I was able to link and run a react-native app with this hermes NodeAPI PR. However after calling raw is a hermes/include/hermes/ADT/CompactArray.h Lines 85 to 87 in 175d85f hermes/include/hermes/ADT/CompactArray.h Lines 53 to 57 in 175d85f hermes/include/hermes/ADT/CompactArray.h Lines 131 to 133 in 175d85f hermes/lib/VM/IdentifierTable.cpp Line 331 in 175d85f 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? |
|
After building hermes in debug mode I get a different error, however without any stacktrace: and after retrying a few times a different error: Does anyone know how I can get the stacktrace of these errors to find the cause?? |
Summary: If a program body has no statements, there is no need to use prettier to print it. Instead just return the docblock comment or an empty string. This is needed due to an issue with prettier printing empty files with comments. With the hermes-transform infra today we perform comment attachment during "parse" so that comments are attached when transforming the AST. Typically however with prettier the attachment is done during printing, but this can be skipped by removing the `comments` array off the program node (so there are not comments to attach). This works great for normal nodes as it prints based off each nodes `comments` array. But for the docblock if there is no statement in the body to attach it to we need to add it to the program comments array but this triggers attachment which fails with our AST format. Reviewed By: gkz Differential Revision: D60191972 fbshipit-source-id: 31975a285e940af0ec0ce95363e8943a291996fb
Summary: Dynamic cast requires ensuring that we generate RTTI for these records, which means we need to keep more careful track of how they are defined. Since we never need an actual dynamic cast, just replace it with an asserting static cast. Reviewed By: avp Differential Revision: D60260766 fbshipit-source-id: bacce14fd5dee08b591ab7271b67feff0d3186cd
Reviewed By: alexmckenley Differential Revision: D60294907 fbshipit-source-id: c7b4a83329140049a59f02f1a370306caad36d65
Summary: Original Author: neildhar@meta.com Original Git: 6b69a06 Original Reviewed By: avp Original Revision: D59072005 The register allocator has the ability to honour a memory limit that is proportional to the product of the number of instructions and basic blocks in the function being allocated. Unfortunately, functions that hit this limit by definition have a lot of instructions Even in the most degenerate case where every block has one instruction, you need 4000 instructions to hit the 10M limit. This diff tries to improve the quality of generated code in cases where most values are used within the basic block they are defined in. In such cases, we currently make the register available after the end of the block. With this diff, the registers become available after their last use in the block. This is useful for functions with extremely large basic blocks, where the current approach would end up allocating a huge number of registers since the registers cannot be used within the same block. Closes facebook#1448 Reviewed By: avp Differential Revision: D60241766 fbshipit-source-id: 5196333862517cd546d675cf8fe005eb1ed5a790
Summary: Move the stack accessors on Errors to the prototype. This makes them easy to globally replace, which is useful for synth traces. Reviewed By: avp Differential Revision: D60323579 fbshipit-source-id: 283795a4dd5f4863b3a87b978da5991e48c1d92f
Summary: X-link: facebook/react-native#50041 Changelog: [Internal] Reviewed By: panagosg7 Differential Revision: D71200832 fbshipit-source-id: a2f267c5b2beabe527f958bfc4557bafc04d27de
Summary: X-link: facebook/react-native#50083 Changelog: [Internal] Reviewed By: gkz Differential Revision: D71354083 fbshipit-source-id: 33fe2c11ad8cba88d3d206a15976510991d3663e
Summary: Split CrashManager out to organize differently. Reviewed By: lavenzg Differential Revision: D71136469 fbshipit-source-id: e3398020c1e4928d877944ae43d0dff3bf9b3989
Reviewed By: lavenzg Differential Revision: D71164778 fbshipit-source-id: 464e62b0a9c3d182fe57c13ac766fe7fa5bb5bb8
Summary: X-link: facebook/react-native#50373 Changelog: [Internal] Reviewed By: jbrown215, SamChou19815, panagosg7 Differential Revision: D72061660 fbshipit-source-id: a6862fe0e1f62992b76145006b3f8bc3ad788258
|
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:
|
|
@kraenhansen a couple of quick points:
|
Summary: X-link: facebook/react-native#50387 Changelog: [Internal] Reviewed By: gkz Differential Revision: D72133412 fbshipit-source-id: f7719284db67537a2c652ea56d9f9050210da7d4
5d46568 to
4780209
Compare
|
@vmoroz has updated the pull request. You must reimport the pull request before landing. |
|
|
@vmoroz has updated the pull request. You must reimport the pull request before landing. |
|
@vmoroz has updated the pull request. You must reimport the pull request before landing. |
8092309 to
dc09086
Compare
|
@vmoroz has updated the pull request. You must reimport the pull request before landing. |
|
@vmoroz has updated the pull request. You must reimport the pull request before landing. |
|
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? |
|
Hey @neildhar, great to hear from you! It was very easy to re-target the PR for the |
|
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! :) |
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:
Test Plan
Unit tests for Node-API are to be added.