We did not have test coverage for using a napi_value
pointing to a string or symbol for the name when
creating a property. Add that coverage.
PR-URL: https://github.com/nodejs/node/pull/13510
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
It takes time to build each of the addons used to test n-api.
Consolidate a few of the smaller ones to save build time.
Get rid of one more small addon.
PR-URL: https://github.com/nodejs/node/pull/13380
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
It takes time to build each of the addons used to test n-api.
Consolidate a few of the smaller ones to save build time.
PR-URL: https://github.com/nodejs/node/pull/13317
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Previously, napi_wrap() would only work with objects created from a
constructor returned by napi_define_class(). While the N-API team
was aware of this limitation, it was not clearly documented and is
likely to cause confusion anyway. It's much simpler if addons are
allowed to use any JS object. Also, the specific behavior of the
limitation is difficult to reimplement on other VMs that work
differently from V8.
V8 requires object internal fields to be declared on the object
prototype (which napi_define_class() used to do). Since it's too
late to modify the object prototype by the time napi_wrap() is
called, napi_wrap() now inserts a new object (with the internal
field) into the supplied object's prototype chain. Then it can be
retrieved from there later by napi_unwrap().
This change also includes improvements to the documentation for
napi_create_external(), partly to explain how it is different from
napi_wrap().
PR-URL: https://github.com/nodejs/node/pull/13250
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
One of the N-API weak-reference test cases already had to be made
asynchronous to handle different behavior in a newer V8 version:
https://github.com/nodejs/node/pull/12864
When porting N-API to Node-ChakraCore, we found more of the test
cases needed similar treatment:
https://github.com/nodejs/node-chakracore/issues/246 So to make
thes tests more robust (and avoid having differences in the test code
for Node-ChakraCore), I am refactoring the tests in this file to
insert a `setImmedate()` callback before every call to `gc()` and
assertions about the effects of the GC.
PR-URL: https://github.com/nodejs/node/pull/13121
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Add testing for all types of typed arrays.
Add testing for napi_is_arraybuffer.
PR-URL: https://github.com/nodejs/node/pull/13244
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Kunal Pathak <kunal.pathak@microsoft.com>
Reviewed-By: Hitesh Kanwathirtha <digitalinfinity@gmail.com>
Add napi_get_version function so that addons can
query the level of N-API supported.
PR-URL: https://github.com/nodejs/node/pull/13207
Fixes: https://github.com/nodejs/abi-stable-node/issues/231
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
Add test to cover napi_has_named_property
PR-URL: https://github.com/nodejs/node/pull/13178
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
Add tests to validate that properties marked as static
are available through the class as opposed to instances
PR-URL: https://github.com/nodejs/node/pull/13124
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Hitesh Kanwathirtha <digitalinfinity@gmail.com>
Unlike most N-API functions, `napi_get_last_error_info()` should not
clear the last error code when successful, because a pointer to (not
a copy of) the error info structure is returned via an out parameter.
PR-URL: https://github.com/nodejs/node/pull/13087
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
- add coverage for napi_get_prototype
- add coverage for napi_strict_equals
PR-URL: https://github.com/nodejs/node/pull/13044
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
Add tests to cover functions that return globals
PR-URL: https://github.com/nodejs/node/pull/13006
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
Use `common.mustCall()` to confirm that function is invoked.
PR-URL: https://github.com/nodejs/node/pull/12959
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
- add coverage for napi_has_element
- add coverage for napi_create_array_with_length
PR-URL: https://github.com/nodejs/node/pull/12890
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
- Create a handle scope before invoking the async completion
callback, because it is basically always needed, easy for user
code to forget, and this makes it more consistent with ordinary
N-API function callbacks.
- Check for an unhandled JS exception after invoking an async
completion callback, and report it via `node::FatalException()`.
- Add a corresponding test case for an exception in async callback.
Previously, any unhandled JS exception thrown from a
`napi_async_complete_callback` would be silently ignored. Among other
things this meant assertions in some test cases could be undetected.
PR-URL: https://github.com/nodejs/node/pull/12838
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
When the number of args requested is greater than the actual number of
args supplied to the function call, the remainder of the args array
should be filled in with `undefined` values. Because of this bug, the
remainder of the array was left uninitialized, which could cause a
crash.
Refer to the documentation for the `argv` parameter at
https://github.com/nodejs/node/blob/master/doc/api/n-api.md#napi_get_cb_info
PR-URL: https://github.com/nodejs/node/pull/12863
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Improved test coverage for napi_make_callback by porting the
existing addons/make_callback test to n-api
PR-URL: https://github.com/nodejs/node/pull/12409
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Add coverage for N-API functions related to
throwing and creating errors. A number of these
are currently showing as not having any
coverage in the nightly code coverage reports.
PR-URL: https://github.com/nodejs/node/pull/12729
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Add cast to avoid warning during build of addon.
PR-URL: https://github.com/nodejs/node/pull/12730
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Replace function expressions with function declarations in preparation
for a lint rule requiring function declarations.
PR-URL: https://github.com/nodejs/node/pull/12711
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Start the transition to Array.prototype.includes() and
String.prototype.includes(). This commit refactors most of the
comparisons of Array.prototype.indexOf() and String.prototype.indexOf()
return values with -1 to the former methods in tests.
PR-URL: https://github.com/nodejs/node/pull/12604
Refs: https://github.com/nodejs/node/issues/12586
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
- Add a test project to addons-napi that covers the
N-API reference and external APIs
- Fix a bug in napi_typeof that was found by the new tests
PR-URL: https://github.com/nodejs/node/pull/12551
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
adding test coverage for napi_cancel_async_work based
on coverage report
PR-URL: https://github.com/nodejs/node/pull/12575
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
We chose to document this in the docs as there are
different possible behaviours. Adding this test to validate
that all vm implementations do it the same way.
PR-URL: https://github.com/nodejs/node/pull/12633
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
N-API is somewhat strict about blocking calls to many APIs while there
is a pending exception. The NAPI_PREAMBLE macro at the beginning of
many API implementations checks for a pending exception. However, a
subset of the APIs (which don't call back into JavaScript) still need
to work while in a pending-exception state. This changes the reference
APIs (equivalent to v8::Persistent) and handle scope APIs so that they
can be used for cleanup up while an exception is pending.
We may decide to similarly enable a few other APIs later, (which would
be a non-breaking change) but we know at least these are needed now
to unblock some specific scenarios.
Fixes: https://github.com/nodejs/abi-stable-node/issues/122
Fixes: https://github.com/nodejs/abi-stable-node/issues/228
PR-URL: https://github.com/nodejs/node/pull/12524
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
We left out null-checks for many of the parameters passed to our APIs.
In particular, arguments of type `napi_value` were often accepted
without a null-check, even though they should never be null.
Additionally, many APIs simply returned `napi_ok` on success. This
leaves in place an error that may have occurred in a previous N-API
call. Others (those which perform `NAPI_PREAMBLE(env)` at the top)
OTOH explicitly clear the last error before proceeding. With this
modification all APIs explicitly clear the last error on success.
Fixes: https://github.com/nodejs/abi-stable-node/issues/227
PR-URL: https://github.com/nodejs/node/pull/12539
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This API doesn't serve much purpose, and is only likely to cause
confusion and bugs. The intention was that this would return the
number of characters in a string independent of encoding, but
that's not generally useful. In almost all cases, one of the
encoding-specific napi_get_value_string_* APIs is more correct.
(Pass a null buffer if only the encoded length is desired.)
Anyway the current implementation of napi_get_value_string_length()
is technically wrong: it returns the number of 2-byte code units of
the UTF-16 encoding, but there are actually some characters that
are encoded as two UTF-16 code units.
Note the JavaScript String.prototype.length property returns the
number of UTF-16 code units, which may be different from the number
of characters. So, getting the true character count is not common
with JavaScript, and is probably best left to specialized
internationalization libraries.
PR-URL: https://github.com/nodejs/node/pull/12496
Fixes: https://github.com/nodejs/abi-stable-node/issues/226
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Improved test coverage for napi_make_callback by porting the
existing addons/make_callback test to n-api
PR-URL: https://github.com/nodejs/node/pull/12409
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/12368
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
Missed in ca786c3734. This does
not actually affect the outcome because returning `nullptr` or
`this` from a constructor has the same effect.
PR-URL: https://github.com/nodejs/node/pull/12318
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
This adds RegExp or error constructor arguments to the remaining places
where it is missing in preparation for the commit that will enforce the
presence of at least two arguments.
PR-URL: https://github.com/nodejs/node/pull/12270
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Add basic tests for handle scopes as code coverage
reports that we are not covering these with the existing
tests.
PR-URL: https://github.com/nodejs/node/pull/12327
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Based on the async methods we had in abi-stable-node before the napi
feature landed in node/master. Changed this set of APIs to handle
error cases and removed a lot of the extra methods we had for setting
all the pieces of napi_work opting instead to pass all of those as
arguments to napi_create_async_work as none of those parameters are
optional except for the complete callback, anyway.
Renamed the napi_work struct to napi_async_work and replace the
struct itself with a class which can better encapsulate the object
lifetime and uv_work_t that we're trying to wrap anyway.
Added a napi_async_callback type for the async helper callbacks
instead of taking raw function pointers and make this callback take a
napi_env parameter as well as the void* data it was already taking.
Call the complete handler for the async work item with a napi_status
code translated from the uvlib error code.
The execute callback is required for napi_create_async_work, though
complete callback is still optional.
Also added some async unit tests for addons-napi based on the
addons/async_hello_world test.
PR-URL: https://github.com/nodejs/node/pull/12250
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Hitesh Kanwathirtha <hiteshk@microsoft.com>
Change `napi_callback` to return `napi_value` directly instead of
requiring `napi_set_return_value`.
When we invoke the callback, we will check the return value and
call `SetReturnValue` ourselves. If the callback returns `NULL`,
we don't set the return value in v8 which would have the same
effect as previously if the callback didn't call
`napi_set_return_value`. Seems to be a more natural way
to handle return values from callbacks. As a consequence,
remove `napi_set_return_value`.
Add a `napi_value` to `napi_property_descriptor` to support string
values which couldn't be passed in the `utf8name` parameter or
symbols as property names. Class names, however, cannot be symbols
so this `napi_value` must be a string type in that case.
Remove all of the `napi_callback_info` helpers except for
`napi_get_cb_info` and make all the parameters to
`napi_get_cb_info` optional except for argc.
Update all the test collateral according to these changes.
Also add `test/addons-napi/common.h` to house some common macros
for wrapping N-API calls and error handling.
PR-URL: https://github.com/nodejs/node/pull/12248
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
The napi_property_attributes enum used names and values from
v8::PropertyAttribute, but those negative flag names were outdated
along with the default behavior of a property being writable,
enumerable, and configurable unless otherwise specified. To match the
ES5 standard property descriptor those attributes should be positive
flags and should default to false unless otherwise specified.
PR-URL: https://github.com/nodejs/node/pull/12240
Fixes: https://github.com/nodejs/abi-stable-node/issues/221
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
1. We define struct napi_env__ to include the isolate, the last
exception, and the info about the last error.
2. We instantiate one struct napi_env__ during module registration and
we pass it into the FunctionCallbackInfo for all subsequent entries into
N-API when we create functions/accessors/finalizers.
Once module unloading will be supported we shall have to delete the
napi_env we create during module init.
There is a clear separation between public and private API wrt. env:
1. Public APIs assert that env is not nullptr as their first action.
2. Private APIs need not validate env. They assume it's not nullptr.
PR-URL: https://github.com/nodejs/node/pull/12195
Fixes: https://github.com/nodejs/abi-stable-node/issues/198
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Add support for abi stable module API (N-API) as "Experimental feature".
The goal of this API is to provide a stable Node API for native
module developers. N-API aims to provide ABI compatibility guarantees
across different Node versions and also across different
Node VMs - allowing N-API enabled native modules to just work
across different versions and flavors of Node.js without recompilation.
A more detailed introduction is provided in:
https://github.com/nodejs/node-eps/blob/master/005-ABI-Stable-Module-API.md
and https://github.com/nodejs/abi-stable-node/blob/doc/VM%20Summit.pdf.
The feature, during its experimental state, will be guarded by a runtime
flag "--napi-modules". Only when this flag is added to the command line
will N-API modules along with regular non N-API modules be supported.
The API is defined by the methods in "src/node_api.h" and
"src/node_api_types.h". This is the best
starting point to review the API surface. More documentation will follow.
In addition to the implementation of the API using V8, which is included
in this PR, the API has also been validated against chakracore and that
port is available in
https://github.com/nodejs/abi-stable-node/tree/api-prototype-chakracore-8.x.
The current plan is to provide N-API support in versions 8.X and 6.X
directly. For older versions, such as 4.X or pre N-API versions of 6.X,
we plan to create an external npm module to provide a migration path
that will allow modules targeting older Node.js versions to use the API,
albeit without getting the advantage of not having to recompile.
In addition, we also plan an external npm package with C++ sugar to
simplify the use of the API. The sugar will be in-line only and will
only use the exported N-API methods but is not part of the N-API
itself. The current version is in:
https://github.com/nodejs/node-api.
This PR is a result of work in the abi-stable-node repo:
https://github.com/nodejs/abi-stable-node/tree/doc,
with this PR being the cumulative work on the api-prototype-8.x
branch with the following contributors in alphabetical order:
Author: Arunesh Chandra <arunesh.chandra@microsoft.com>
Author: Gabriel Schulhof <gabriel.schulhof@intel.com>
Author: Hitesh Kanwathirtha <hiteshk@microsoft.com>
Author: Ian Halliday <ianhall@microsoft.com>
Author: Jason Ginchereau <jasongin@microsoft.com>
Author: Michael Dawson <michael_dawson@ca.ibm.com>
Author: Sampson Gao <sampsong@ca.ibm.com>
Author: Taylor Woll <taylor.woll@microsoft.com>
PR-URL: https://github.com/nodejs/node/pull/11975
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Roman Reiss
Michael Dawson
Jason Ginchereau
Rich Trott
Michael Dawson
Michaël Zasso
Hitesh Kanwathirtha
gwer
Gabriel Schulhof
James M Snell
Sampson Gao
Anna Henningsen
taylor.woll
Taylor Woll