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.

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

n-api: implement promise #14365

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
138 changes: 138 additions & 0 deletions doc/api/n-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ The documentation for N-API is structured as follows:
* [Working with JavaScript Functions][]
* [Object Wrap][]
* [Asynchronous Operations][]
* [Promises][]

The N-API is a C API that ensures ABI stability across Node.js versions
and different compiler levels. However, we also understand that a C++
Expand Down Expand Up @@ -3395,6 +3396,142 @@ support it:

<!-- it's very convenient to have all the anchors indexed -->
<!--lint disable no-unused-definitions remark-lint-->
## Promises

N-API provides facilities for creating `Promise` objects as described in
[Section 25.4][] of the ECMA specification. It implements promises as a pair of
objects. When a promise is created by `napi_create_promise()`, a "deferred"
object is created and returned alongside the `Promise`. The deferred object is
bound to the created `Promise` and is the only means to resolve or reject the
`Promise` using `napi_resolve_deferred()` or `napi_reject_deferred()`. The
deferred object that is created by `napi_create_promise()` is freed by
`napi_resolve_deferred()` or `napi_reject_deferred()`. The `Promise` object may
be returned to JavaScript where it can be used in the usual fashion.

For example, to create a promise and pass it to an asynchronous worker:
```c
napi_deferred deferred;
napi_value promise;
napi_status status;

// Create the promise.
status = napi_create_promise(env, &deferred, &promise);
if (status != napi_ok) return NULL;

// Pass the deferred to a function that performs an asynchronous action.
do_something_asynchronous(deferred);

// Return the promise to JS
return promise;
```

The above function `do_something_asynchronous()` would perform its asynchronous
action and then it would resolve or reject the deferred, thereby concluding the
promise and freeing the deferred:
```c
napi_deferred deferred;
napi_value undefined;
napi_status status;

// Create a value with which to conclude the deferred.
status = napi_get_undefined(env, &undefined);
if (status != napi_ok) return NULL;

// Resolve or reject the promise associated with the deferred depending on
// whether the asynchronous action succeeded.
if (asynchronous_action_succeeded) {
status = napi_resolve_deferred(env, deferred, undefined);
} else {
status = napi_reject_deferred(env, deferred, undefined);
}
if (status != napi_ok) return NULL;

// At this point the deferred has been freed, so we should assign NULL to it.
deferred = NULL;
```

### napi_create_promise
<!-- YAML
added: REPLACEME
-->
```C
NAPI_EXTERN napi_status napi_create_promise(napi_env env,
napi_deferred* deferred,
napi_value* promise);
```

- `[in] env`: The environment that the API is invoked under.
- `[out] deferred`: A newly created deferred object which can later be passed to
`napi_resolve_deferred()` or `napi_reject_deferred()` to resolve resp. reject
the associated promise.
- `[out] promise`: The JavaScript promise associated with the deferred object.

Returns `napi_ok` if the API succeeded.

This API creates a deferred object and a JavaScript promise.

### napi_resolve_deferred
<!-- YAML
added: REPLACEME
-->
```C
NAPI_EXTERN napi_status napi_resolve_deferred(napi_env env,
napi_deferred deferred,
napi_value resolution);
```

- `[in] env`: The environment that the API is invoked under.
- `[in] deferred`: The deferred object whose associated promise to resolve.
- `[in] resolution`: The value with which to resolve the promise.

This API resolves a JavaScript promise by way of the deferred object
with which it is associated. Thus, it can only be used to resolve JavaScript
promises for which the corresponding deferred object is available. This
effectively means that the promise must have been created using
`napi_create_promise()` and the deferred object returned from that call must
have been retained in order to be passed to this API.

The deferred object is freed upon successful completion.

### napi_reject_deferred
<!-- YAML
added: REPLACEME
-->
```C
NAPI_EXTERN napi_status napi_reject_deferred(napi_env env,
napi_deferred deferred,
napi_value rejection);
```

- `[in] env`: The environment that the API is invoked under.
- `[in] deferred`: The deferred object whose associated promise to resolve.
- `[in] rejection`: The value with which to reject the promise.

This API rejects a JavaScript promise by way of the deferred object
with which it is associated. Thus, it can only be used to reject JavaScript
promises for which the corresponding deferred object is available. This
effectively means that the promise must have been created using
`napi_create_promise()` and the deferred object returned from that call must
have been retained in order to be passed to this API.

The deferred object is freed upon successful completion.

### napi_is_promise
<!-- YAML
added: REPLACEME
-->
```C
NAPI_EXTERN napi_status napi_is_promise(napi_env env,
napi_value promise,
bool* is_promise);
```

- `[in] env`: The environment that the API is invoked under.
- `[in] promise`: The promise to examine
- `[out] is_promise`: Flag indicating whether `promise` is a native promise
Copy link
Member

Choose a reason for hiding this comment

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

As someone who has helped maintain large promise libraries - I'm totally fine with making people Promise.resolve promises they pass to napi_* methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean by "I'm totally fine making people Promise.resolve promises"? Do you mean that you don't mind if the native side resolves a promise created in JS and then passed to the native side?

Copy link
Member

Choose a reason for hiding this comment

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

@gabrielschulhof I mean I'm fine with is_promise only caring about and working with native promises - and ignoring non-native promises in n-api completely

object - that is, a promise object created by the underlying engine.

[Promises]: #n_api_promises
[Asynchronous Operations]: #n_api_asynchronous_operations
[Basic N-API Data Types]: #n_api_basic_n_api_data_types
[ECMAScript Language Specification]: https://tc39.github.io/ecma262/
Expand All @@ -3406,6 +3543,7 @@ support it:
[Section 9.1.6]: https://tc39.github.io/ecma262/#sec-ordinary-object-internal-methods-and-internal-slots-defineownproperty-p-desc
[Section 12.5.5]: https://tc39.github.io/ecma262/#sec-typeof-operator
[Section 24.3]: https://tc39.github.io/ecma262/#sec-dataview-objects
[Section 25.4]: https://tc39.github.io/ecma262/#sec-promise-objects
[Working with JavaScript Functions]: #n_api_working_with_javascript_functions
[Working with JavaScript Properties]: #n_api_working_with_javascript_properties
[Working with JavaScript Values]: #n_api_working_with_javascript_values
Expand Down
78 changes: 78 additions & 0 deletions src/node_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,14 @@ V8EscapableHandleScopeFromJsEscapableHandleScope(
static_assert(sizeof(v8::Local<v8::Value>) == sizeof(napi_value),
"Cannot convert between v8::Local<v8::Value> and napi_value");

napi_deferred JsDeferredFromV8Persistent(v8::Persistent<v8::Value>* local) {
return reinterpret_cast<napi_deferred>(local);
}

v8::Persistent<v8::Value>* V8PersistentFromJsDeferred(napi_deferred local) {
return reinterpret_cast<v8::Persistent<v8::Value>*>(local);
}

napi_value JsValueFromV8LocalValue(v8::Local<v8::Value> local) {
return reinterpret_cast<napi_value>(*local);
}
Expand Down Expand Up @@ -774,6 +782,33 @@ napi_status Unwrap(napi_env env,
return napi_ok;
}

napi_status ConcludeDeferred(napi_env env,
napi_deferred deferred,
napi_value result,
bool is_resolved) {
NAPI_PREAMBLE(env);
CHECK_ARG(env, result);

v8::Local<v8::Context> context = env->isolate->GetCurrentContext();
v8::Persistent<v8::Value>* deferred_ref =
V8PersistentFromJsDeferred(deferred);
v8::Local<v8::Value> v8_deferred =
v8::Local<v8::Value>::New(env->isolate, *deferred_ref);

auto v8_resolver = v8::Local<v8::Promise::Resolver>::Cast(v8_deferred);

v8::Maybe<bool> success = is_resolved ?
v8_resolver->Resolve(context, v8impl::V8LocalValueFromJsValue(result)) :
v8_resolver->Reject(context, v8impl::V8LocalValueFromJsValue(result));

deferred_ref->Reset();
delete deferred_ref;

RETURN_STATUS_IF_FALSE(env, success.FromMaybe(false), napi_generic_failure);

return GET_RETURN_STATUS(env);
}

} // end of namespace v8impl

// Intercepts the Node-V8 module registration callback. Converts parameters
Expand Down Expand Up @@ -3332,3 +3367,46 @@ napi_status napi_cancel_async_work(napi_env env, napi_async_work work) {

return napi_clear_last_error(env);
}

NAPI_EXTERN napi_status napi_create_promise(napi_env env,
napi_deferred* deferred,
napi_value* promise) {
NAPI_PREAMBLE(env);
CHECK_ARG(env, deferred);
CHECK_ARG(env, promise);

auto maybe = v8::Promise::Resolver::New(env->isolate->GetCurrentContext());
CHECK_MAYBE_EMPTY(env, maybe, napi_generic_failure);

auto v8_resolver = maybe.ToLocalChecked();
auto v8_deferred = new v8::Persistent<v8::Value>();
v8_deferred->Reset(env->isolate, v8_resolver);

*deferred = v8impl::JsDeferredFromV8Persistent(v8_deferred);
*promise = v8impl::JsValueFromV8LocalValue(v8_resolver->GetPromise());
return GET_RETURN_STATUS(env);
}

NAPI_EXTERN napi_status napi_resolve_deferred(napi_env env,
napi_deferred deferred,
napi_value resolution) {
return v8impl::ConcludeDeferred(env, deferred, resolution, true);
}

NAPI_EXTERN napi_status napi_reject_deferred(napi_env env,
napi_deferred deferred,
napi_value resolution) {
return v8impl::ConcludeDeferred(env, deferred, resolution, false);
}

NAPI_EXTERN napi_status napi_is_promise(napi_env env,
napi_value promise,
bool* is_promise) {
CHECK_ENV(env);
CHECK_ARG(env, promise);
CHECK_ARG(env, is_promise);

*is_promise = v8impl::V8LocalValueFromJsValue(promise)->IsPromise();

return napi_clear_last_error(env);
}
14 changes: 14 additions & 0 deletions src/node_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,20 @@ NAPI_EXTERN
napi_status napi_get_node_version(napi_env env,
const napi_node_version** version);

// Promises
NAPI_EXTERN napi_status napi_create_promise(napi_env env,
napi_deferred* deferred,
napi_value* promise);
NAPI_EXTERN napi_status napi_resolve_deferred(napi_env env,
napi_deferred deferred,
napi_value resolution);
NAPI_EXTERN napi_status napi_reject_deferred(napi_env env,
napi_deferred deferred,
napi_value rejection);
NAPI_EXTERN napi_status napi_is_promise(napi_env env,
napi_value promise,
bool* is_promise);

EXTERN_C_END

#endif // SRC_NODE_API_H_
1 change: 1 addition & 0 deletions src/node_api_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ typedef struct napi_handle_scope__ *napi_handle_scope;
typedef struct napi_escapable_handle_scope__ *napi_escapable_handle_scope;
typedef struct napi_callback_info__ *napi_callback_info;
typedef struct napi_async_work__ *napi_async_work;
typedef struct napi_deferred__ *napi_deferred;

typedef enum {
napi_default = 0,
Expand Down
8 changes: 8 additions & 0 deletions test/addons-napi/test_promise/binding.gyp
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"targets": [
{
"target_name": "test_promise",
"sources": [ "test_promise.c" ]
}
]
}
60 changes: 60 additions & 0 deletions test/addons-napi/test_promise/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
'use strict';

const common = require('../../common');
const test_promise = require(`./build/${common.buildType}/test_promise`);
const assert = require('assert');

let expected_result, promise;

// A resolution
expected_result = 42;
promise = test_promise.createPromise();
promise.then(
common.mustCall(function(result) {
assert.strictEqual(result, expected_result,
'promise resolved as expected');
}),
common.mustNotCall());
test_promise.concludeCurrentPromise(expected_result, true);

// A rejection
expected_result = 'It\'s not you, it\'s me.';
promise = test_promise.createPromise();
promise.then(
common.mustNotCall(),
common.mustCall(function(result) {
assert.strictEqual(result, expected_result,
'promise rejected as expected');
}));
test_promise.concludeCurrentPromise(expected_result, false);
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to test resolution and rejection behavior with promises as @addaleax noted in #14365 (comment).


// Chaining
promise = test_promise.createPromise();
promise.then(
common.mustCall(function(result) {
assert.strictEqual(result, 'chained answer',
'resolving with a promise chains properly');
}),
common.mustNotCall());
test_promise.concludeCurrentPromise(Promise.resolve('chained answer'), true);

assert.strictEqual(test_promise.isPromise(promise), true,
'natively created promise is recognized as a promise');

assert.strictEqual(test_promise.isPromise(Promise.reject(-1)), true,
'Promise created with JS is recognized as a promise');

assert.strictEqual(test_promise.isPromise(2.4), false,
'Number is recognized as not a promise');

assert.strictEqual(test_promise.isPromise('I promise!'), false,
Copy link
Member

Choose a reason for hiding this comment

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

Hah!

'String is recognized as not a promise');

assert.strictEqual(test_promise.isPromise(undefined), false,
'undefined is recognized as not a promise');

assert.strictEqual(test_promise.isPromise(null), false,
'null is recognized as not a promise');

assert.strictEqual(test_promise.isPromise({}), false,
'an object is recognized as not a promise');
Loading