[6.x] n-api: backport to v6.x by gabrielschulhof · Pull Request #19447 · nodejs/node
added
c++
v6.x labels
Mar 19, 2018
MylesBorins
added
the
semver-minor
label
Mar 20, 2018MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018This 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. Backport-PR-URL: #19447 PR-URL: #12270 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018Missed in ca786c3. This does not actually affect the outcome because returning `nullptr` or `this` from a constructor has the same effect. Backport-PR-URL: #19447 PR-URL: #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>
MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018This 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. Backport-PR-URL: #19447 PR-URL: #12496 Fixes: nodejs/abi-stable-node#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>
MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018We 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: nodejs/abi-stable-node#227 Backport-PR-URL: #19447 PR-URL: #12539 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018N-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: nodejs/abi-stable-node#122 Fixes: nodejs/abi-stable-node#228 Backport-PR-URL: #19447 PR-URL: #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>
MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018- 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 Backport-PR-URL: #19447 PR-URL: #12551 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018Background: To enable N-API support for node versions back to v4, the
N-API code can also be built as an external addon. To make maintenance
easier, a single codebase needs to support both built-in and external
scenarios, along with Node versions >= 4 (and corresponding V8
versions).
This change includes several minor fixes to avoid using node
internal APIs and support older V8 versions:
- Expand node::arraysize
- In the CHECK_ENV() macro, return an error code instead of calling
node::FatalError(). This is more consistent with how other invalid
arguments to N-API functions are handled.
- In v8impl::SetterCallbackWrapper::SetReturnValue(), do nothing
instead of calling node::FatalError(). This is more consistent
with JavaScript setter callbacks, where any returned value is
silently ignored.
- When queueing async work items, get the uv default loop instead of
getting the loop from node::Environment::GetCurrent(). Currently
that returns the same loop anyway. If/when node supports multiple
environments, it should have a public API for getting the
environment & event loop, and we can update this implementation
then.
- Use v8::Maybe::FromJust() instead of the newer alias ToChecked()
Backport-PR-URL: #19447
PR-URL: #12674
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018Add the initial documentation for the N-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 documentation sections in that repo 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: 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> Backport-PR-URL: #19447 PR-URL: #12549 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018Start 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. Backport-PR-URL: #19447 PR-URL: #12604 Refs: #12586 Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018These APIs do not need a try-catch around their body, because no exceptions are thrown in their implementation: - `napi_is_array()` - `napi_get_value_string_latin1()` - `napi_get_value_string_utf8()` - `napi_get_value_string_utf16()` - `napi_get_value_external()` - `napi_is_buffer()` - `napi_is_arraybuffer()` - `napi_get_arraybuffer_info()` - `napi_is_typedarray()` - `napi_get_typedarray_info()` Fixes: nodejs/abi-stable-node#238 Backport-PR-URL: #19447 PR-URL: #12705 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018Add 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. Backport-PR-URL: #19447 PR-URL: #12729 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018Improved test coverage for napi_make_callback by porting the existing addons/make_callback test to n-api Backport-PR-URL: #19447 PR-URL: #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>
MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018Backport-PR-URL: #19447 PR-URL: #12864 Ref: #12551 (comment) Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018When 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 Backport-PR-URL: #19447 PR-URL: #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>
MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018- 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. Backport-PR-URL: #19447 PR-URL: #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>
MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018- fix 2 broken links - fix capitalization in description of napi_create_array-with-length Backport-PR-URL: #19447 PR-URL: #12889 Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: MichaëZasso <targos@protonmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Hitesh Kanwathirtha <digitalinfinity@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018Backport-PR-URL: #19447 PR-URL: #12898 Fixes: #7129 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018Use `common.mustCall()` to confirm that function is invoked. Backport-PR-URL: #19447 PR-URL: #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>
MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018Backport-PR-URL: #19447 PR-URL: #12974 Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
MylesBorins pushed a commit that referenced this pull request
Apr 16, 2018This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters