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

[DRAFT] Add Node-API to Hermes #1377

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vmoroz
Copy link
Contributor

@vmoroz vmoroz commented Apr 18, 2024

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.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Apr 18, 2024
@tmikov
Copy link
Contributor

tmikov commented Apr 18, 2024

Thanks for doing this @vmoroz!!!

@facebook-github-bot
Copy link
Contributor

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

@shirakaba
Copy link

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

@NathanWalker
Copy link

Huge effort @vmoroz thank you ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants