Skip to content

Commit

Permalink
n-api: Sync with back-compat changes
Browse files Browse the repository at this point in the history
Background: 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>
  • Loading branch information
jasongin authored and MylesBorins committed Apr 16, 2018
1 parent bc25250 commit 49d74c6
Showing 1 changed file with 16 additions and 12 deletions.
28 changes: 16 additions & 12 deletions src/node_api.cc
Expand Up @@ -12,8 +12,10 @@
#include <node_object_wrap.h>
#include <string.h>
#include <algorithm>
#include <cassert>
#include <cmath>
#include <vector>
#include "uv.h"
#include "node_api.h"
#include "env-inl.h"
#include "node_api_backport.h"
Expand Down Expand Up @@ -45,9 +47,9 @@ struct napi_env__ {
} \
} while (0)

#define CHECK_ENV(env) \
if ((env) == nullptr) { \
node::FatalError(__func__, "environment(env) must not be null"); \
#define CHECK_ENV(env) \
if ((env) == nullptr) { \
return napi_invalid_arg; \
}

#define CHECK_ARG(env, arg) \
Expand Down Expand Up @@ -581,8 +583,7 @@ class SetterCallbackWrapper

/*virtual*/
void SetReturnValue(napi_value value) override {
node::FatalError("napi_set_return_value",
"Cannot return a value from a setter callback.");
// Ignore any value returned from a setter callback.
}

private:
Expand Down Expand Up @@ -745,7 +746,8 @@ napi_status napi_get_last_error_info(napi_env env,
CHECK_ENV(env);
CHECK_ARG(env, result);

static_assert(node::arraysize(error_messages) == napi_status_last,
static_assert(
(sizeof (error_messages) / sizeof (*error_messages)) == napi_status_last,
"Count of error messages must match count of error values");
assert(env->last_error.error_code < napi_status_last);

Expand Down Expand Up @@ -1697,7 +1699,7 @@ napi_status napi_get_value_int32(napi_env env,

v8::Isolate* isolate = env->isolate;
v8::Local<v8::Context> context = isolate->GetCurrentContext();
*result = val->Int32Value(context).ToChecked();
*result = val->Int32Value(context).FromJust();

return napi_clear_last_error(env);
}
Expand All @@ -1716,7 +1718,7 @@ napi_status napi_get_value_uint32(napi_env env,

v8::Isolate* isolate = env->isolate;
v8::Local<v8::Context> context = isolate->GetCurrentContext();
*result = val->Uint32Value(context).ToChecked();
*result = val->Uint32Value(context).FromJust();

return napi_clear_last_error(env);
}
Expand All @@ -1741,7 +1743,7 @@ napi_status napi_get_value_int64(napi_env env,
} else {
v8::Isolate* isolate = env->isolate;
v8::Local<v8::Context> context = isolate->GetCurrentContext();
*result = val->IntegerValue(context).ToChecked();
*result = val->IntegerValue(context).FromJust();
}

return napi_clear_last_error(env);
Expand Down Expand Up @@ -2822,9 +2824,11 @@ napi_status napi_queue_async_work(napi_env env, napi_async_work work) {
CHECK_ENV(env);
CHECK_ARG(env, work);

// Consider: Encapsulate the uv_loop_t into an opaque pointer parameter
uv_loop_t* event_loop =
node::Environment::GetCurrent(env->isolate)->event_loop();
// Consider: Encapsulate the uv_loop_t into an opaque pointer parameter.
// Currently the environment event loop is the same as the UV default loop.
// Someday (if node ever supports multiple isolates), it may be better to get
// the loop from node::Environment::GetCurrent(env->isolate)->event_loop();
uv_loop_t* event_loop = uv_default_loop();

uvimpl::Work* w = reinterpret_cast<uvimpl::Work*>(work);

Expand Down

0 comments on commit 49d74c6

Please sign in to comment.