Skip to content

Update code to support N-API on older versions of node #25

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

Merged
jasongin merged 2 commits into from
Apr 20, 2017
Merged

Update code to support N-API on older versions of node #25

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0d8dea4
Update to current sources
jasongin Apr 17, 2017
57d0f9b
Enable back-compat with Node v4
jasongin Apr 17, 2017
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
Prev Previous commit
Enable back-compat with Node v4 
Recent changes introduced dependencies on internal node APIs:
 - Environment::GetCurrent()
 - FatalError()
 - arraysize
To enable building this code as an external module, compatible back to Node v4, the internal APIs must be avoided.

Also replace use of v8::Maybe::ToChecked() (available in Node >= 7) with FromJust(), which does excatly the same thing and works in Node v4.
  • Loading branch information
@jasongin
jasongin committed Apr 20, 2017
commit 57d0f9be8532796b7ee9e3490da7700e9f564f37
29 changes: 16 additions & 13 deletions src/node_api.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@
#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"

napi_status napi_set_last_error(napi_env env, napi_status error_code,
uint32_t engine_error_code = 0,
Expand Down Expand Up @@ -43,9 +44,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 @@ -570,8 +571,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 @@ -730,7 +730,8 @@ napi_status napi_get_last_error_info(napi_env env,
const napi_extended_error_info** result) {
CHECK_ENV(env);

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 @@ -1634,7 +1635,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_ok;
}
Expand All @@ -1653,7 +1654,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_ok;
}
Expand All @@ -1678,7 +1679,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_ok;
Expand Down Expand Up @@ -2684,9 +2685,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();
Copy link
Member

@addaleax addaleax Apr 18, 2017

Choose a reason for hiding this comment

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

I’m not sure, this might not work when there are native modules that use a different loop, or Node embedders that use multiple loops… but nan does it too so it can’t be so terribly bad I guess?

Copy link
Member Author

@jasongin jasongin Apr 18, 2017

Choose a reason for hiding this comment

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

Right, nan does this now. Other native modules are free to use non-default loops, if they call the libuv APIs directly, and that won't cause any problems with this.

Eventually, we might wrap more of the libuv APIs in N-API, and that would support non-default loops.

Copy link

@kkoopa kkoopa Apr 18, 2017

Choose a reason for hiding this comment

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

nan supports pick-and-mix. If the default loop is too restrictive, you can fall back to libuv. If the point here is to abstract away libuv, there needs to be a way of specifying a non-default loop without having to call the libuv API directly.

Copy link
Member Author

@jasongin jasongin Apr 18, 2017

Choose a reason for hiding this comment

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

As I said just above, these APIs are not intended to entirely abstract away libuv, at least not at this time.


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

Expand Down