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

Mention emnapi in documentation #17515

Merged
merged 4 commits into from Feb 23, 2023
Merged

Conversation

toyobayashi
Copy link
Contributor

Would you mind mentioning emnapi in documentation?

emnapi gives Emscripten users another way to write binding code, and also gives users the ability to port Node.js addons written in Node-API to WebAssembly.

Currently emnapi has a documentation site and has successfully ported most of the official examples in node-addon-examples repository.

I think it is time to let more developers know emnapi. The community can help make it better.

Sorry for my bad English, great appreciate if you approve this.

@@ -798,6 +799,16 @@ for defining the binding:
of one tool over the other will usually be based on which is the most
natural fit for the project and its build system.

.. _interacting-with-code-emnapi:

Binding C/C++ and JavaScript — Emnapi (Node-API for Emscripten)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that - some kind of unicode - ? It looks different to the other ones in this file? Can we stick the to ascii -?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks different to the other ones in this file

It was copied from here

Binding C++ and JavaScript — WebIDL Binder and Embind
======================================================

I use Ctrl + F in VSCode, and then find there are many in this file. I think I should leave them the same.

@@ -17,6 +17,7 @@ JavaScript and compiled C or C++:
created with:

- :ref:`Embind or WebIDL-Binder<interacting-with-code-binding-cpp>`
- :ref:`Emnapi (Node-API)<interacting-with-code-emnapi>`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it makes sense to list it here. Emnapi is specific set of bindings for node API, right? Not a method of writing new binding?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my understanding, developers cannot use Node-API to write bindings in emscripten before emnapi exists, in a sense it is a new way to write bindings in emscripten, so it is listed here like embind.

@toyobayashi
Copy link
Contributor Author

Hi, @sbc100 @kripken

Could you reconsider this PR? With the great help of @RReverser , now emnapi is more and more mature. It is used in lovell/sharp#3522 and TryGhost/node-sqlite3#1674. I have confidence to say now emnapi is the only project which has so complete Node-API implementation for wasm. I believe it is not a toy project and it can make contribution to wasm community.

@sbc100
Copy link
Collaborator

sbc100 commented Feb 10, 2023

Sure, I don't see why we shouldn't mention this in the docs.

I'm not sure the title "Binding C/C++ and JavaScript — Emnapi (Node-API for Emscripten)" makes sense though, at least not to me. This isn't like embind in that sense I don't think. How about "Writing node extensions using emscripten (Node-API)" or "Porting node extensions to emscripten (Node-API)`? Or maybe you can come up with a better one?

@toyobayashi
Copy link
Contributor Author

If I understand correctly, what embind doing is

void native_function(/* ... */) { /* ... */ }

emscripten::val native_function_binding(/* ... */) {
  // ...
  native_function(/* ... */);
  // ...
}

EMSCRIPTEN_BINDINGS(mod) {
  emscripten::function("nativeFunction", native_function_binding);
}

Then I can use it in JavaScript:

Module.nativeFunction(/* ... */);

And what emnapi doing is

void native_function(/* ... */) { /* ... */ }

napi_value native_function_binding(napi_env env, napi_callback_info info) {
  // ...
  native_function(/* ... */);
  // ...
}

#define EXPORT_FUNCTION(env, exports, name, f) \
  do { \
    napi_value f##_fn; \
    napi_create_function((env), NULL, NAPI_AUTO_LENGTH, (f), NULL, &(f##_fn)); \
    napi_set_named_property((env), (exports), (name), (f##_fn)); \
  } while (0)

NAPI_MODULE_INIT() {
  EXPORT_FUNCTION(env, exports, "nativeFunction", native_function_binding);
  return exports;
}

Then I can use it in JavaScript:

const binding = Module.emnapiInit(/* ... */);
binding.nativeFunction(/* ... */);

So what is the difference do you think in binding C/C++ and JavaScript between embind and emnapi? It does give emscripten a new way of writing bindings, at least to me. So I think there is nothing wrong with this title.

Except binding ability, emnapi provide ability of porting Node-API addon to wasm additionally. From this point, "Porting node extensions to emscripten (Node-API)" is also good :)

@toyobayashi
Copy link
Contributor Author

@sbc100 As I explained above, how about these titles?

  • Binding C/C++ and JavaScript - Node-API
  • Using Node-API to write binding or port native Node.js addon to Emscripten
  • Node-API binding on Emscripten

@sbc100
Copy link
Collaborator

sbc100 commented Feb 10, 2023

I don't really have much insight I'm afraid. Perhaps @RReverser would be better placed to review here?

@RReverser
Copy link
Collaborator

Oh, sorry, almost missed this one. I think N-API does make sense in the JS binding section.

While a few of its APIs are Node-specific, most of it was designed as a generic way to interact with JS values from native code - and in that context it doesn't matter whether it's V8, ChakraCore, SpiderMonkey, or something like Emscripten that represents the native side.

@toyobayashi
Copy link
Contributor Author

Yeah, sorry for my bad English expression, and thanks @RReverser to give us a more clear explanation about Node-API. Now I have updated the title and some content, is there anything else that needs to be changed? Waiting for your approvement.

@@ -17,6 +17,7 @@ JavaScript and compiled C or C++:
created with:

- :ref:`Embind or WebIDL-Binder<interacting-with-code-binding-cpp>`
- :ref:`Emnapi (Node-API)<interacting-with-code-emnapi>`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the mention below is ok, but I would remove this one. The main part of this doc is for officially supported things in the emscripten repo. But adding a mention near the end is ok for unofficial stuff (while mentioning it is unofficial, as I see that you do).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@kripken
Copy link
Member

kripken commented Feb 23, 2023

Not sure what happened with the other tests, but the docs test passed and this only changed docs...

@kripken kripken merged commit 65f36f3 into emscripten-core:main Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants