src: factor out Node.js-agnostic N-APIs #23786

gabrielschulhof · 2018-10-20T13:18:59Z

Split the Node.js ECMAScript API (N-EAPI?) into its own header and
implementation files. The motivation is that the ECMAScript API stand
on its own so it might be embedded separately, implementation and all.

Portions of the implementation used by both files are stored in
node_api_impl.h.

The checked boxes below indicate that the given API remains in
node_api.h, whereas the lack of a checkbox indicates that the API was
moved to node_ecma_api.h.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

nodejs-github-bot · 2018-10-20T13:19:01Z

@gabrielschulhof build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1318/pipeline

gabrielschulhof · 2018-10-20T13:20:39Z

CI: https://ci.nodejs.org/job/node-test-pull-request/18009/

gabrielschulhof · 2018-10-20T15:01:18Z

@nodejs/n-api ping!

refack

Interesting design idea 👍

refack · 2018-10-20T15:21:16Z

src/node_api_impl.h

@@ -0,0 +1,195 @@
+#ifndef SRC_NODE_API_IMPL_H_


I believe it's more of a convention to call such files *-inl.h

True, I guess most of the stuff has ended up inline. I'll rename it.

gabrielschulhof · 2018-10-20T15:54:16Z

@refack yeah, there seems to be demand out there to have a header that basically describes the interaction between a native code base and a JavaScript VM, irrespective of the entry point (a dynamically loaded addon in our case). So, it looks like it makes sense to separate out the entry point from the description of the interaction.

gabrielschulhof · 2018-10-20T16:01:30Z

CI failure is unrelated.

joyeecheung · 2018-10-20T18:24:32Z

Is there a particular reason it is named ECMAScript API instead of JavaScript API? Somehow to me it implies that the concepts/lingos in this subset have equivalents in ECMA-262, but it doesn't seem to always be the case (e.g. handle scopes)?

It also seems a bit strange to see the name of an organization (ecma) in the header file when they are not behind the implementation (are they? AFAIK ECMA does not do any technical work.) Also, can we use their name there legally, when the Node.js foundation is not a member of ECMA?

One may also have an impression that this somehow has something to do with ECMA TC53 that works on a ECMAScript-based wearable platform..or, there has also been talks about spinning a new TC in ECMA to specify an environment like Node.js (though I am still somewhat puzzled about that suggestion).

gabrielschulhof · 2018-10-20T19:26:22Z

@joyeecheung I don't have a strong opinion about whether to call things ECMAScript or not. I guess I was influenced by the fact that most of the JerryScript implementation is called ecma-this.c and ecma-that.c. I totally don't mind renaming the files.

gabrielschulhof · 2018-10-20T19:29:56Z

@joyeecheung also, in our docs, we often refer to sections in the spec:

etc.

joyeecheung · 2018-10-20T20:04:42Z

@gabrielschulhof I see, thanks for the pointers. Maybe it makes sense to replace ecma with js, vm, or lang here? (BTW, considering C# and CLI are also specified under ECMA, it seems somewhat strange to name files like ecma-array-object.c, although I guess that's not that ambiguous when in a JerryScript checkout...)

gabrielschulhof · 2018-10-20T22:30:23Z

@joyeecheung "js" sounds good.

gabrielschulhof · 2018-10-21T00:05:29Z

@joyeecheung I would love to see a TC that specifies how a JavaScript engine is to be used from the native side. node_js_api.h is heading in that direction de facto.

gabrielschulhof · 2018-10-21T00:23:34Z

@joyeecheung renamed.

gabrielschulhof · 2018-10-21T13:46:54Z

@bghgary PTAL

bghgary · 2018-10-21T18:17:56Z

@gabrielschulhof Thanks for working on this. I'm excited to see these changes.

There are still a number of issues to fix in order for the js part to be self-contained. For example, node::Persistent and node::Environment are still referenced in the factored out js code. Another example is that napi_env__ has a reference to uv_loop_t which needs to be separated somehow. I will make a complete list and post them here once I have them.

mhdawson · 2018-10-22T21:46:35Z

Will wait to see follow on comments from @bghgary before doing an in-depth review but looks like a great start.

gabrielschulhof · 2018-10-23T04:47:23Z

@bghgary I've removed the last remaining node references from node_js_api.cc. If you copy node_js_api.cc and node_js_api.h verbatim and supply your own node_api_internals.h you should be able to build the two files into your project.

gabrielschulhof · 2018-10-23T06:10:26Z

@bghgary after a bit more refactoring, you may copy node_js_api.cc, node_js_api.h, and node_api_common.h verbatim, and supply node_api_internals.h.

bghgary · 2018-10-23T22:39:46Z

Thanks @gabrielschulhof! I will look at this as soon as I can.

bghgary

I think it's getting closer. The only work items left are:

Need a way to create a napi_env object
Need to factor the C++ wrapper code to not include node specific wrappers.

bghgary · 2018-10-24T22:29:39Z

src/node_api_internals.h

+#include "env.h"
+#include "node_internals.h"
+
+#define NAPI_ARRAYSIZE(array) \


This seems overkill to me. It is only used by one static_assert in the code. I would rather duplicate the code for simplicity sake.

bghgary · 2018-10-24T22:33:39Z

src/node_api_internals.h

+#define NAPI_ARRAYSIZE(array) \
+  node::arraysize((array))
+
+#define NAPI_FIXED_ONE_BYTE_STRING(isolate, string) \


This is only used twice within one function. It seems like we can easily just call the correct v8 function instead of using this macro.

bghgary · 2018-10-24T22:37:35Z

src/node_api_internals.h

+#define NAPI_FIXED_ONE_BYTE_STRING(isolate, string) \
+  node::FIXED_ONE_BYTE_STRING((isolate), (string))
+
+#define NAPI_PRIVATE_KEY(context, suffix) \


This is only ever used as NAPI_PRIVATE_KEY(context, wrapper) in a few functions. It would make more sense to me if the value of this is stored in napi_env__ as a v8::Private. Then this macro would not be necessary anymore.

bghgary · 2018-10-24T22:42:46Z

src/node_api_internals.h

+namespace v8impl {
+
+template <typename T>
+using Persistent = node::Persistent<T>;


I'm not sure I understand the implications of this. According to node_persistent.h, it looks like this is a workaround to prevent people from calling a member function that has bugs in V8. I'm not convinced yet that this is required in this context. If it is required, can we duplicate the tiny amount of code in node_persistent.h instead?

bghgary · 2018-10-24T22:45:34Z

src/node_api_internals.h

@@ -0,0 +1,24 @@
+#ifndef SRC_NODE_API_INTERNALS_H_


I don't think node_api_internals.h is necessary. See the other comments I have for the code defined in this file.

bghgary · 2018-10-24T23:05:14Z

src/node_api.cc

-  CHECK_ARG(env, scope);
-  CHECK_ARG(env, escapee);
-  CHECK_ARG(env, result);
+NAPI_NO_RETURN void napi_fatal_error(const char* location,


This function is used by the C++ wrapper in the addon API. One possible solution is to move this to node_js_api.h and make it such that by default it throws an exception but can be overridden to do something else. In a node environment, it will call node::FatalError.

bghgary · 2018-10-24T23:14:19Z

src/node_api_internals.h

+
+#include "node_version.h"
+#include "env.h"
+#include "node_internals.h"


This is bringing in a few macros (CHECK, CHECK_EQ, CHECK_LE) that are used by the code. I suggest doing something like this so that it will work in standalone and in node.

#include <cassert> // ... #ifndef CHECK #define CHECK(expr) assert(expr) #endif #ifndef CHECK_EQ #define CHECK_EQ(a, b) CHECK((a) == (b)) #endif #ifndef CHECK_LE #define CHECK_LE(a, b) CHECK((a) <= (b)) #endif

Sorry, just realized that CHECK needs to work in release builds as well, so using assert directly won't work.

bghgary · 2018-10-24T23:24:47Z

test/addons-napi/7_factory_wrap/myobject.h

@@ -1,7 +1,7 @@
 #ifndef TEST_ADDONS_NAPI_7_FACTORY_WRAP_MYOBJECT_H_
 #define TEST_ADDONS_NAPI_7_FACTORY_WRAP_MYOBJECT_H_

-#include <node_api.h>
+#include <node_js_api.h>


Is there a way to add a test that only includes the standalone js files and maybe include v8 directly, such that we can ensure changes don't break the standalone js scenario?

bghgary · 2018-10-24T23:36:10Z

src/node_api.h

@@ -46,7 +24,6 @@ struct uv_loop_s;  // Forward declaration.
 #define NAPI_NO_RETURN


NAPI_NO_RETURN is being used by the C++ wrappers. This should move to the common header or we should remove the macro usage from the C++ wrappers.

I believe we shall eventually have to similarly partition the C++ wrappers such that only the Node.js-specific header uses NAPI_NO_RETURN.

bghgary · 2018-10-25T00:08:25Z

@gabrielschulhof @joyeecheung Would it be better if we named the files node_api_js instead node_js_api, just so that node_api sorts together with node_api_js?

bghgary · 2018-10-25T00:10:52Z

@gabrielschulhof I have locally hacked around most of the issues I mentioned in the comments. Let me know if I can help with the implementation in any way.

gabrielschulhof · 2018-10-25T03:29:45Z

@bghgary the creation of the environment should be up to the runtime, because it may involve more than just V8. In Node.js it certainly does. That's why node_api.cc subclasses napi_env__. Also, in Node.js the environment is created during the initialization of the first N-API module that gets loaded. This may not be true for other environments. If you need nothing further than what struct napi_env__ provides, you can always create one with new napi_env__(isolate) and then pass it to the code using N-API.

I'd also like to keep the ties to Node.js via node_api_internals.h. If anything, having an escape hatch via this include file ensures that adaptations can be made for a variety of V8 environments.

I'm not yet ready to consider the C++ wrappers in node-addon-api. I think we should concentrate first on getting N-API to look right.

bghgary · 2018-10-25T18:47:29Z

@gabrielschulhof

the creation of the environment should be up to the runtime

Agreed. I'm not suggesting otherwise.

you can always create one with new napi_env__(isolate) and then pass it to the code using N-API

That is what I'm doing right now. Here are my thoughts. napi_api_common.h seems like it is an internal header specific to a v8 implementation. For the standalone js api usage, is it the right contract to have to include this header and create their own function to create an napi_env? If so, should it still be called napi_env__? What is the right way to target a different VM using this contract? Sorry for so many questions. 😅

I'd also like to keep the ties to Node.js via node_api_internals.h. If anything, having an escape hatch via this include file ensures that adaptations can be made for a variety of V8 environments.

I think that's fine for the api that are node specific, but I think it's strange that js api also has a dependency on node_api_internals.h.

I'm not yet ready to consider the C++ wrappers in node-addon-api. I think we should concentrate first on getting N-API to look right.

Sure, up to you. I find that looking at the C++ wrappers provides a bigger end-to-end picture so that we don't miss anything.

mhdawson · 2018-11-15T17:58:22Z

With respect to:

@yorkie I think we'd have to figure out if moving the headers would be breaking or not. My feeling is we should land this without doing that and then figure that out.

Thinking about it more, I think it is probably ok if just the js-native-XX headers were moved out.

mhdawson · 2018-11-15T18:00:07Z

Other than the few comments I left the rest looks good to me and I'm happy to sign off once we address the question of moving some headers out and respond to the rest of the outstanding comments.

gabrielschulhof · 2018-11-16T16:57:02Z

@yorkie @mhdawson

If we move the implementation to src/napi in order to be able to make test-addons-napi we need the following modification:

diff --git a/deps/npm/node_modules/node-gyp/addon.gypi b/deps/npm/node_modules/node-gyp/addon.gypi
index 55fb321118..c0513cd592 100644
--- a/deps/npm/node_modules/node-gyp/addon.gypi
+++ b/deps/npm/node_modules/node-gyp/addon.gypi
@@ -18,6 +18,7 @@
     'include_dirs': [
       '<(node_root_dir)/include/node',
       '<(node_root_dir)/src',
+      '<(node_root_dir)/src/napi',
       '<(node_root_dir)/deps/openssl/config',
       '<(node_root_dir)/deps/openssl/openssl/include',
       '<(node_root_dir)/deps/uv/include',

This means we need to change node-gyp, and then pull in a new version via npm. I'm not sure that's worth the trouble at this point. Maybe in a later PR.

gabrielschulhof · 2018-11-16T19:13:09Z

@mhdawson not included in this latest commit is my effort to replace references to node_api.h with js_native_api.h in all N-API tests, and to give the tests a common entry point at test/addons-napi/entry_point.c which can be referred as 'sources': [ ..., '../entry_point.c' ] from the tests' binding.gyp. That way, the reference to node_api.h is restricted to a single file under test/addons-napi for all tests covering js_native_api.h.

mhdawson · 2018-11-16T21:56:36Z

@gabrielschulhof I think landing this and then thinking about moving the files to a different directory later makes sense to me.

mhdawson

LGTM

Trott · 2018-11-17T01:10:27Z

CI: https://ci.nodejs.org/job/node-test-pull-request/18693/

gabrielschulhof · 2018-11-17T20:51:01Z

Another CI: https://ci.nodejs.org/job/node-test-pull-request/18703/

gabrielschulhof · 2018-11-17T22:41:10Z

Landed in 596bd5f.

Split the Node.js ECMAScript API (N-EAPI?) into its own header and implementation files. The motivation is that the ECMAScript API stand on its own so it might be embedded separately, implementation and all. Portions of the implementation used by both files are stored in `node_api_impl.h`. The checked boxes below indicate that the given API remains in `node_api.h`, whereas the lack of a checkbox indicates that the API was moved to `node_ecma_api.h`. * [x] NAPI_MODULE * [x] NAPI_MODULE_INIT * [x] napi_acquire_threadsafe_function * [x] napi_add_env_cleanup_hook * [x] napi_async_destroy * [x] napi_async_init * [x] napi_call_threadsafe_function * [x] napi_cancel_async_work * [x] napi_close_callback_scope * [x] napi_create_async_work * [x] napi_create_buffer * [x] napi_create_buffer_copy * [x] napi_create_external_buffer * [x] napi_create_threadsafe_function * [x] napi_delete_async_work * [x] napi_fatal_error * [x] napi_fatal_exception * [x] napi_get_buffer_info * [x] napi_get_node_version * [x] napi_get_threadsafe_function_context * [x] napi_get_uv_event_loop * [x] napi_is_buffer * [x] napi_make_callback * [x] napi_module_register * [x] napi_open_callback_scope * [x] napi_queue_async_work * [x] napi_ref_threadsafe_function * [x] napi_release_threadsafe_function * [x] napi_remove_env_cleanup_hook * [x] napi_unref_threadsafe_function * [ ] napi_add_finalizer * [ ] napi_adjust_external_memory * [ ] napi_call_function * [ ] napi_close_escapable_handle_scope * [ ] napi_close_handle_scope * [ ] napi_coerce_to_bool * [ ] napi_coerce_to_number * [ ] napi_coerce_to_object * [ ] napi_coerce_to_string * [ ] napi_create_array * [ ] napi_create_arraybuffer * [ ] napi_create_array_with_length * [ ] napi_create_bigint_int64 * [ ] napi_create_bigint_uint64 * [ ] napi_create_bigint_words * [ ] napi_create_dataview * [ ] napi_create_double * [ ] napi_create_error * [ ] napi_create_external * [ ] napi_create_external_arraybuffer * [ ] napi_create_function * [ ] napi_create_int32 * [ ] napi_create_int64 * [ ] napi_create_object * [ ] napi_create_promise * [ ] napi_create_range_error * [ ] napi_create_reference * [ ] napi_create_string_latin1 * [ ] napi_create_string_utf16 * [ ] napi_create_string_utf8 * [ ] napi_create_symbol * [ ] napi_create_typedarray * [ ] napi_create_type_error * [ ] napi_create_uint32 * [ ] napi_define_class * [ ] napi_define_properties * [ ] napi_delete_element * [ ] napi_delete_property * [ ] napi_delete_reference * [ ] napi_escape_handle * [ ] napi_get_and_clear_last_exception * [ ] napi_get_arraybuffer_info * [ ] napi_get_array_length * [ ] napi_get_boolean * [ ] napi_get_cb_info * [ ] napi_get_dataview_info * [ ] napi_get_element * [ ] napi_get_global * [ ] napi_get_last_error_info * [ ] napi_get_named_property * [ ] napi_get_new_target * [ ] napi_get_null * [ ] napi_get_property * [ ] napi_get_property_names * [ ] napi_get_prototype * [ ] napi_get_reference_value * [ ] napi_get_typedarray_info * [ ] napi_get_undefined * [ ] napi_get_value_bigint_int64 * [ ] napi_get_value_bigint_uint64 * [ ] napi_get_value_bigint_words * [ ] napi_get_value_bool * [ ] napi_get_value_double * [ ] napi_get_value_external * [ ] napi_get_value_int32 * [ ] napi_get_value_int64 * [ ] napi_get_value_string_latin1 * [ ] napi_get_value_string_utf16 * [ ] napi_get_value_string_utf8 * [ ] napi_get_value_uint32 * [ ] napi_get_version * [ ] napi_has_element * [ ] napi_has_named_property * [ ] napi_has_own_property * [ ] napi_has_property * [ ] napi_instanceof * [ ] napi_is_array * [ ] napi_is_arraybuffer * [ ] napi_is_dataview * [ ] napi_is_error * [ ] napi_is_exception_pending * [ ] napi_is_promise * [ ] napi_is_typedarray * [ ] napi_new_instance * [ ] napi_open_escapable_handle_scope * [ ] napi_open_handle_scope * [ ] napi_reference_ref * [ ] napi_reference_unref * [ ] napi_reject_deferred * [ ] napi_remove_wrap * [ ] napi_resolve_deferred * [ ] napi_run_script * [ ] napi_set_element * [ ] napi_set_named_property * [ ] napi_set_property * [ ] napi_strict_equals * [ ] napi_throw * [ ] napi_throw_error * [ ] napi_throw_range_error * [ ] napi_throw_type_error * [ ] napi_typeof * [ ] napi_unwrap * [ ] napi_wrap PR-URL: #23786 Reviewed-By: Yazhong Liu <yorkiefixer@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

bghgary · 2018-11-19T19:06:12Z

@gabrielschulhof Thanks for all the work on this!

A couple of small issues were not resolved in this PR. Specifically, this issue and this issue with the header includes. Should I file a separate issue for them or I can submit a PR? I am currently modifying the headers when I bring them into my project, but it seems like they should be fixed at the source.

mhdawson · 2018-11-19T20:52:05Z

@bghgary please go ahead and submit at PR and copy @gabrielschulhof and @mhdawson (me). Sorry we missed getting these in, but should be easier to land small incremental changes now.

bghgary · 2018-11-19T20:57:43Z

Okay, sounds good. Thanks!

bghgary · 2018-11-20T00:41:12Z

PR created here: #24498

Split the Node.js ECMAScript API (N-EAPI?) into its own header and implementation files. The motivation is that the ECMAScript API stand on its own so it might be embedded separately, implementation and all. Portions of the implementation used by both files are stored in `node_api_impl.h`. The checked boxes below indicate that the given API remains in `node_api.h`, whereas the lack of a checkbox indicates that the API was moved to `node_ecma_api.h`. * [x] NAPI_MODULE * [x] NAPI_MODULE_INIT * [x] napi_acquire_threadsafe_function * [x] napi_add_env_cleanup_hook * [x] napi_async_destroy * [x] napi_async_init * [x] napi_call_threadsafe_function * [x] napi_cancel_async_work * [x] napi_close_callback_scope * [x] napi_create_async_work * [x] napi_create_buffer * [x] napi_create_buffer_copy * [x] napi_create_external_buffer * [x] napi_create_threadsafe_function * [x] napi_delete_async_work * [x] napi_fatal_error * [x] napi_fatal_exception * [x] napi_get_buffer_info * [x] napi_get_node_version * [x] napi_get_threadsafe_function_context * [x] napi_get_uv_event_loop * [x] napi_is_buffer * [x] napi_make_callback * [x] napi_module_register * [x] napi_open_callback_scope * [x] napi_queue_async_work * [x] napi_ref_threadsafe_function * [x] napi_release_threadsafe_function * [x] napi_remove_env_cleanup_hook * [x] napi_unref_threadsafe_function * [ ] napi_add_finalizer * [ ] napi_adjust_external_memory * [ ] napi_call_function * [ ] napi_close_escapable_handle_scope * [ ] napi_close_handle_scope * [ ] napi_coerce_to_bool * [ ] napi_coerce_to_number * [ ] napi_coerce_to_object * [ ] napi_coerce_to_string * [ ] napi_create_array * [ ] napi_create_arraybuffer * [ ] napi_create_array_with_length * [ ] napi_create_bigint_int64 * [ ] napi_create_bigint_uint64 * [ ] napi_create_bigint_words * [ ] napi_create_dataview * [ ] napi_create_double * [ ] napi_create_error * [ ] napi_create_external * [ ] napi_create_external_arraybuffer * [ ] napi_create_function * [ ] napi_create_int32 * [ ] napi_create_int64 * [ ] napi_create_object * [ ] napi_create_promise * [ ] napi_create_range_error * [ ] napi_create_reference * [ ] napi_create_string_latin1 * [ ] napi_create_string_utf16 * [ ] napi_create_string_utf8 * [ ] napi_create_symbol * [ ] napi_create_typedarray * [ ] napi_create_type_error * [ ] napi_create_uint32 * [ ] napi_define_class * [ ] napi_define_properties * [ ] napi_delete_element * [ ] napi_delete_property * [ ] napi_delete_reference * [ ] napi_escape_handle * [ ] napi_get_and_clear_last_exception * [ ] napi_get_arraybuffer_info * [ ] napi_get_array_length * [ ] napi_get_boolean * [ ] napi_get_cb_info * [ ] napi_get_dataview_info * [ ] napi_get_element * [ ] napi_get_global * [ ] napi_get_last_error_info * [ ] napi_get_named_property * [ ] napi_get_new_target * [ ] napi_get_null * [ ] napi_get_property * [ ] napi_get_property_names * [ ] napi_get_prototype * [ ] napi_get_reference_value * [ ] napi_get_typedarray_info * [ ] napi_get_undefined * [ ] napi_get_value_bigint_int64 * [ ] napi_get_value_bigint_uint64 * [ ] napi_get_value_bigint_words * [ ] napi_get_value_bool * [ ] napi_get_value_double * [ ] napi_get_value_external * [ ] napi_get_value_int32 * [ ] napi_get_value_int64 * [ ] napi_get_value_string_latin1 * [ ] napi_get_value_string_utf16 * [ ] napi_get_value_string_utf8 * [ ] napi_get_value_uint32 * [ ] napi_get_version * [ ] napi_has_element * [ ] napi_has_named_property * [ ] napi_has_own_property * [ ] napi_has_property * [ ] napi_instanceof * [ ] napi_is_array * [ ] napi_is_arraybuffer * [ ] napi_is_dataview * [ ] napi_is_error * [ ] napi_is_exception_pending * [ ] napi_is_promise * [ ] napi_is_typedarray * [ ] napi_new_instance * [ ] napi_open_escapable_handle_scope * [ ] napi_open_handle_scope * [ ] napi_reference_ref * [ ] napi_reference_unref * [ ] napi_reject_deferred * [ ] napi_remove_wrap * [ ] napi_resolve_deferred * [ ] napi_run_script * [ ] napi_set_element * [ ] napi_set_named_property * [ ] napi_set_property * [ ] napi_strict_equals * [ ] napi_throw * [ ] napi_throw_error * [ ] napi_throw_range_error * [ ] napi_throw_type_error * [ ] napi_typeof * [ ] napi_unwrap * [ ] napi_wrap PR-URL: #23786 Reviewed-By: Yazhong Liu <yorkiefixer@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

codebytere · 2019-01-12T23:28:27Z

@gabrielschulhof do you think this can/should be backported to v10.x? I've added a label, but feel free to remove it!

Split the Node.js ECMAScript API (N-EAPI?) into its own header and implementation files. The motivation is that the ECMAScript API stand on its own so it might be embedded separately, implementation and all. Portions of the implementation used by both files are stored in `node_api_impl.h`. The checked boxes below indicate that the given API remains in `node_api.h`, whereas the lack of a checkbox indicates that the API was moved to `node_ecma_api.h`. * [x] NAPI_MODULE * [x] NAPI_MODULE_INIT * [x] napi_acquire_threadsafe_function * [x] napi_add_env_cleanup_hook * [x] napi_async_destroy * [x] napi_async_init * [x] napi_call_threadsafe_function * [x] napi_cancel_async_work * [x] napi_close_callback_scope * [x] napi_create_async_work * [x] napi_create_buffer * [x] napi_create_buffer_copy * [x] napi_create_external_buffer * [x] napi_create_threadsafe_function * [x] napi_delete_async_work * [x] napi_fatal_error * [x] napi_fatal_exception * [x] napi_get_buffer_info * [x] napi_get_node_version * [x] napi_get_threadsafe_function_context * [x] napi_get_uv_event_loop * [x] napi_is_buffer * [x] napi_make_callback * [x] napi_module_register * [x] napi_open_callback_scope * [x] napi_queue_async_work * [x] napi_ref_threadsafe_function * [x] napi_release_threadsafe_function * [x] napi_remove_env_cleanup_hook * [x] napi_unref_threadsafe_function * [ ] napi_add_finalizer * [ ] napi_adjust_external_memory * [ ] napi_call_function * [ ] napi_close_escapable_handle_scope * [ ] napi_close_handle_scope * [ ] napi_coerce_to_bool * [ ] napi_coerce_to_number * [ ] napi_coerce_to_object * [ ] napi_coerce_to_string * [ ] napi_create_array * [ ] napi_create_arraybuffer * [ ] napi_create_array_with_length * [ ] napi_create_bigint_int64 * [ ] napi_create_bigint_uint64 * [ ] napi_create_bigint_words * [ ] napi_create_dataview * [ ] napi_create_double * [ ] napi_create_error * [ ] napi_create_external * [ ] napi_create_external_arraybuffer * [ ] napi_create_function * [ ] napi_create_int32 * [ ] napi_create_int64 * [ ] napi_create_object * [ ] napi_create_promise * [ ] napi_create_range_error * [ ] napi_create_reference * [ ] napi_create_string_latin1 * [ ] napi_create_string_utf16 * [ ] napi_create_string_utf8 * [ ] napi_create_symbol * [ ] napi_create_typedarray * [ ] napi_create_type_error * [ ] napi_create_uint32 * [ ] napi_define_class * [ ] napi_define_properties * [ ] napi_delete_element * [ ] napi_delete_property * [ ] napi_delete_reference * [ ] napi_escape_handle * [ ] napi_get_and_clear_last_exception * [ ] napi_get_arraybuffer_info * [ ] napi_get_array_length * [ ] napi_get_boolean * [ ] napi_get_cb_info * [ ] napi_get_dataview_info * [ ] napi_get_element * [ ] napi_get_global * [ ] napi_get_last_error_info * [ ] napi_get_named_property * [ ] napi_get_new_target * [ ] napi_get_null * [ ] napi_get_property * [ ] napi_get_property_names * [ ] napi_get_prototype * [ ] napi_get_reference_value * [ ] napi_get_typedarray_info * [ ] napi_get_undefined * [ ] napi_get_value_bigint_int64 * [ ] napi_get_value_bigint_uint64 * [ ] napi_get_value_bigint_words * [ ] napi_get_value_bool * [ ] napi_get_value_double * [ ] napi_get_value_external * [ ] napi_get_value_int32 * [ ] napi_get_value_int64 * [ ] napi_get_value_string_latin1 * [ ] napi_get_value_string_utf16 * [ ] napi_get_value_string_utf8 * [ ] napi_get_value_uint32 * [ ] napi_get_version * [ ] napi_has_element * [ ] napi_has_named_property * [ ] napi_has_own_property * [ ] napi_has_property * [ ] napi_instanceof * [ ] napi_is_array * [ ] napi_is_arraybuffer * [ ] napi_is_dataview * [ ] napi_is_error * [ ] napi_is_exception_pending * [ ] napi_is_promise * [ ] napi_is_typedarray * [ ] napi_new_instance * [ ] napi_open_escapable_handle_scope * [ ] napi_open_handle_scope * [ ] napi_reference_ref * [ ] napi_reference_unref * [ ] napi_reject_deferred * [ ] napi_remove_wrap * [ ] napi_resolve_deferred * [ ] napi_run_script * [ ] napi_set_element * [ ] napi_set_named_property * [ ] napi_set_property * [ ] napi_strict_equals * [ ] napi_throw * [ ] napi_throw_error * [ ] napi_throw_range_error * [ ] napi_throw_type_error * [ ] napi_typeof * [ ] napi_unwrap * [ ] napi_wrap PR-URL: nodejs#23786 Reviewed-By: Yazhong Liu <yorkiefixer@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Oct 20, 2018

gabrielschulhof added the node-api Issues and PRs related to the Node-API. label Oct 20, 2018

refack reviewed Oct 20, 2018

View reviewed changes

gabrielschulhof force-pushed the split-out-n-api branch from db1194f to c759091 Compare October 21, 2018 00:23

bghgary mentioned this pull request Oct 21, 2018

Factor out N-API to expand embeddability use cases nodejs/abi-stable-node#354

Closed

gabrielschulhof force-pushed the split-out-n-api branch from 0995958 to 78733ce Compare October 23, 2018 06:08

bghgary reviewed Oct 25, 2018

View reviewed changes

yorkie approved these changes Nov 16, 2018

View reviewed changes

mhdawson approved these changes Nov 16, 2018

View reviewed changes

address review comments

4b12574

gabrielschulhof force-pushed the split-out-n-api branch from 26461cc to 4b12574 Compare November 17, 2018 20:49

gabrielschulhof closed this Nov 17, 2018

gabrielschulhof deleted the split-out-n-api branch November 17, 2018 22:43

BridgeAR mentioned this pull request Dec 5, 2018

v11.4.0 proposal #24854

Merged

4 tasks

codebytere added the backport-requested-v10.x label Jan 12, 2019

		@@ -46,7 +24,6 @@ struct uv_loop_s; // Forward declaration.
		#define NAPI_NO_RETURN

src: factor out Node.js-agnostic N-APIs #23786

src: factor out Node.js-agnostic N-APIs #23786

Conversation

gabrielschulhof commented Oct 20, 2018

Checklist

nodejs-github-bot commented Oct 20, 2018

gabrielschulhof commented Oct 20, 2018

gabrielschulhof commented Oct 20, 2018

refack left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabrielschulhof commented Oct 20, 2018

gabrielschulhof commented Oct 20, 2018

joyeecheung commented Oct 20, 2018 • edited Loading

gabrielschulhof commented Oct 20, 2018

gabrielschulhof commented Oct 20, 2018

joyeecheung commented Oct 20, 2018 • edited Loading

gabrielschulhof commented Oct 20, 2018

gabrielschulhof commented Oct 21, 2018

gabrielschulhof commented Oct 21, 2018

gabrielschulhof commented Oct 21, 2018

bghgary commented Oct 21, 2018

mhdawson commented Oct 22, 2018 • edited Loading

gabrielschulhof commented Oct 23, 2018

gabrielschulhof commented Oct 23, 2018

bghgary commented Oct 23, 2018

bghgary left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bghgary Oct 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bghgary commented Oct 25, 2018 • edited Loading

bghgary commented Oct 25, 2018

gabrielschulhof commented Oct 25, 2018

bghgary commented Oct 25, 2018

mhdawson commented Nov 15, 2018

mhdawson commented Nov 15, 2018

gabrielschulhof commented Nov 16, 2018

gabrielschulhof commented Nov 16, 2018

mhdawson commented Nov 16, 2018

mhdawson left a comment

Choose a reason for hiding this comment

Trott commented Nov 17, 2018

gabrielschulhof commented Nov 17, 2018

gabrielschulhof commented Nov 17, 2018

bghgary commented Nov 19, 2018

mhdawson commented Nov 19, 2018

bghgary commented Nov 19, 2018

bghgary commented Nov 20, 2018

codebytere commented Jan 12, 2019

joyeecheung commented Oct 20, 2018 •

edited

Loading

joyeecheung commented Oct 20, 2018 •

edited

Loading

mhdawson commented Oct 22, 2018 •

edited

Loading

bghgary Oct 24, 2018 •

edited

Loading

bghgary commented Oct 25, 2018 •

edited

Loading