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.

Sign up for GitHub

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

doc: clarify N-API version 1 #34344

Closed
wants to merge 9 commits into from
Closed

doc: clarify N-API version 1 #34344

Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
71c7a51
doc: clarify N-API version 1
mhdawson Jul 13, 2020
6904325
squash: fixup formatting of note
mhdawson Jul 13, 2020
9455cdc
Update doc/api/n-api.md
mhdawson Jul 16, 2020
4bc4c13
Update doc/api/n-api.md
mhdawson Jul 16, 2020
6713a61
Update doc/api/n-api.md
mhdawson Jul 16, 2020
7d81104
Update doc/api/n-api.md
mhdawson Jul 20, 2020
c9ad6f1
Update doc/api/n-api.md
mhdawson Jul 21, 2020
127f153
Update doc/api/n-api.md
mhdawson Jul 22, 2020
2ece38b
Update doc/api/n-api.md
mhdawson Jul 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions doc/api/n-api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,19 +240,25 @@ from version 3 with some additions. This means that it is not necessary
to recompile for new versions of Node.js which are
listed as supporting a later version.

| | 1 | 2 | 3 | 4 | 5 | 6 |
|-------|---------|----------|----------|----------|-----------|-----------|
| v6.x | | | v6.14.2* | | | |
| v8.x | v8.0.0* | v8.10.0* | v8.11.2 | v8.16.0 | | |
| v9.x | v9.0.0* | v9.3.0* | v9.11.0* | | | |
| v10.x | v10.0.0 | v10.0.0 | v10.0.0 | v10.16.0 | v10.17.0 | v10.20.0 |
| v11.x | v11.0.0 | v11.0.0 | v11.0.0 | v11.8.0 | | |
| v12.x | v12.0.0 | v12.0.0 | v12.0.0 | v12.0.0 | v12.11.0 | v12.17.0 |
| v13.x | v13.0.0 | v13.0.0 | v13.0.0 | v13.0.0 | v13.0.0 | |
| v14.x | v14.0.0 | v14.0.0 | v14.0.0 | v14.0.0 | v14.0.0 | v14.0.0 |
| | 1 | 2 | 3 | 4 | 5 | 6 |
|-------|----------|----------|----------|----------|-----------|-----------|
| v6.x | | | v6.14.2* | | | |
| v8.x | v8.6.0** | v8.10.0* | v8.11.2 | v8.16.0 | | |
| v9.x | v9.0.0* | v9.3.0* | v9.11.0* | | | |
| v10.x | v10.0.0 | v10.0.0 | v10.0.0 | v10.16.0 | v10.17.0 | v10.20.0 |
| v11.x | v11.0.0 | v11.0.0 | v11.0.0 | v11.8.0 | | |
| v12.x | v12.0.0 | v12.0.0 | v12.0.0 | v12.0.0 | v12.11.0 | v12.17.0 |
| v13.x | v13.0.0 | v13.0.0 | v13.0.0 | v13.0.0 | v13.0.0 | |
| v14.x | v14.0.0 | v14.0.0 | v14.0.0 | v14.0.0 | v14.0.0 | v14.0.0 |

\* Indicates that the N-API version was released as experimental

\*\* First version which matches version 1 in later releases. While v8.0.0
included N-API as experimental, version 1 continued to evolve until
v8.7.0 and therefore the shape of the API in earlier versions is not
mhdawson marked this conversation as resolved.
Show resolved Hide resolved
truly version 1 (in hindsight we should have called it version 0).
We recommend version 3 or later.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
We recommend version 3 or later.
This also means that our ABI stability guarantee starts at version v8.7.0.
We recommend version 3 or later.

Feel free to rephrase or even discard. The objective I'm trying to achieve with the suggestion is that the final phrasing include a reference to ABI stability and the earliest version which is ABI-compatible with what we have today.

Is v8.7.0 truly the first version of Node.js whose N-API implementation is ABI-compatible with today's? IIRC we had the final ABI in place even while we were still in experimental. Of course, it may not be worth pushing back the documented number a couple of minor versions just to "maximize our coverage".

Copy link
Member Author

Choose a reason for hiding this comment

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

@gabrielschulhof 8.7.0 is the earliest level that @devsnek reported as being compatible so believe it was the earliest.

Copy link
Member Author

Choose a reason for hiding this comment

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

@gabrielschulhof I think I'd also prefer to leave out the extra sentence as I think it restates what the earlier lines say in a bit more detail.

Copy link
Member

Choose a reason for hiding this comment

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

I don't know exactly. I gauged it based on when node-addon-api headers didn't have undefined symbols and such. I don't exhaustively test if it actually works.

Copy link
Member Author

@mhdawson mhdawson Jul 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

The diff between 8.7.0 and the last 8.x version (8.17.0) for node_api.h is:

iff --git a/src/node_api.h b/src/node_api.h
index a3a07a6..f683f8d 100644
--- a/src/node_api.h
+++ b/src/node_api.h
@@ -1,19 +1,23 @@
-/******************************************************************************
- * Experimental prototype for demonstrating VM agnostic and ABI stable API
- * for native modules to use instead of using Nan and V8 APIs directly.
- *
- * The current status is "Experimental" and should not be used for
- * production applications.  The API is still subject to change
- * and as an experimental feature is NOT subject to semver.
- *
- ******************************************************************************/
 #ifndef SRC_NODE_API_H_
 #define SRC_NODE_API_H_

 #include <stddef.h>
 #include <stdbool.h>
+
+#ifndef NAPI_VERSION
+#ifdef NAPI_EXPERIMENTAL
+// Use INT_MAX, this should only be consumed by the pre-processor anyway.
+#define NAPI_VERSION 2147483647
+#else
+// The baseline version for N-API
+#define NAPI_VERSION 4
+#endif
+#endif
+
 #include "node_api_types.h"

+struct uv_loop_s;  // Forward declaration.
+
 #ifdef _WIN32
   #ifdef BUILDING_NODE_EXTENSION
     #ifdef EXTERNAL_NAPI
@@ -98,7 +102,7 @@ typedef struct {
   EXTERN_C_END

 #define NAPI_MODULE(modname, regfunc) \
-  NAPI_MODULE_X(modname, regfunc, NULL, 0)
+  NAPI_MODULE_X(modname, regfunc, NULL, 0)  // NOLINT (readability/null_usage)

 #define NAPI_AUTO_LENGTH SIZE_MAX

@@ -175,7 +179,7 @@ NAPI_EXTERN napi_status napi_create_range_error(napi_env env,
                                                 napi_value msg,
                                                 napi_value* result);

-// Methods to get the the native napi_value from Primitive type
+// Methods to get the native napi_value from Primitive type
 NAPI_EXTERN napi_status napi_typeof(napi_env env,
                                     napi_value value,
                                     napi_valuetype* result);
@@ -581,6 +585,76 @@ NAPI_EXTERN napi_status napi_run_script(napi_env env,
                                         napi_value script,
                                         napi_value* result);

+#if NAPI_VERSION >= 2
+
+// Return the current libuv event loop for a given environment
+NAPI_EXTERN napi_status napi_get_uv_event_loop(napi_env env,
+                                               struct uv_loop_s** loop);
+
+#endif  // NAPI_VERSION >= 2
+
+#if NAPI_VERSION >= 3
+
+NAPI_EXTERN napi_status napi_open_callback_scope(napi_env env,
+                                                 napi_value resource_object,
+                                                 napi_async_context context,
+                                                 napi_callback_scope* result);
+
+NAPI_EXTERN napi_status napi_close_callback_scope(napi_env env,
+                                                  napi_callback_scope scope);
+
+NAPI_EXTERN napi_status napi_fatal_exception(napi_env env, napi_value err);
+
+NAPI_EXTERN napi_status napi_add_env_cleanup_hook(napi_env env,
+                                                  void (*fun)(void* arg),
+                                                  void* arg);
+
+NAPI_EXTERN napi_status napi_remove_env_cleanup_hook(napi_env env,
+                                                     void (*fun)(void* arg),
+                                                     void* arg);
+
+#endif  // NAPI_VERSION >= 3
+
+#if NAPI_VERSION >= 4
+
+// Calling into JS from other threads
+NAPI_EXTERN napi_status
+napi_create_threadsafe_function(napi_env env,
+                                napi_value func,
+                                napi_value async_resource,
+                                napi_value async_resource_name,
+                                size_t max_queue_size,
+                                size_t initial_thread_count,
+                                void* thread_finalize_data,
+                                napi_finalize thread_finalize_cb,
+                                void* context,
+                                napi_threadsafe_function_call_js call_js_cb,
+                                napi_threadsafe_function* result);
+
+NAPI_EXTERN napi_status
+napi_get_threadsafe_function_context(napi_threadsafe_function func,
+                                     void** result);
+
+
+NAPI_EXTERN napi_status
+napi_call_threadsafe_function(napi_threadsafe_function func,
+                              void* data,
+                              napi_threadsafe_function_call_mode is_blocking);
+
+NAPI_EXTERN napi_status
+napi_acquire_threadsafe_function(napi_threadsafe_function func);
+
+NAPI_EXTERN napi_status
+napi_release_threadsafe_function(napi_threadsafe_function func,
+                                 napi_threadsafe_function_release_mode mode);
+
+NAPI_EXTERN napi_status
+napi_unref_threadsafe_function(napi_env env, napi_threadsafe_function func);
+
+NAPI_EXTERN napi_status
+napi_ref_threadsafe_function(napi_env env, napi_threadsafe_function func);
+
+#endif  // NAPI_VERSION >= 4
+
 EXTERN_C_END

 #endif  // SRC_NODE_API_H_

Copy link
Member Author

Choose a reason for hiding this comment

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

Looking at the additions I don't see any API changes from 8.7.0 and the last 8.x version in the diff above (+struct uv_loop_s; // Forward declaration. was added as part of the NAPI_VERSION_2 addition, not sure why we did that but that's a separate issue)

Copy link
Member Author

Choose a reason for hiding this comment

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

git diff v8.6.0..v8.7.0 src/node_api.h shows no changes

Copy link
Member Author

Choose a reason for hiding this comment

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

git diff v8.5.0..v8.6.0 src/node_api.h show a number of changes in the API.

I think that would likely say that v8.6.0 is the right one to specify. There don't seem to be any changes in the api file between v8.6.0 that and the last 8.x version which would affect the API shape. It was still experimental so there would be no guarantees about it working properly or internal behavior changing but that is not relevant to us adding the comment about when the shape of V1 was finalized.

@devsnek I'll update to v8.6.0 unless what you saw contradicts that.

Copy link
Member

Choose a reason for hiding this comment

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

checking this again, it appears to be 8.6.0, not 8.7.0. 8.5.0 is missing napi_async_context and NAPI_AUTO_LENGTH which are both in version 1.

Copy link
Member Author

Choose a reason for hiding this comment

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

@devsnek thanks for confirming, we must have both been investigating at the same time.


The N-APIs associated strictly with accessing ECMAScript features from native
code can be found separately in `js_native_api.h` and `js_native_api_types.h`.
The APIs defined in these headers are included in `node_api.h` and
Expand Down