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: from zzo38 via IRC - Some ideas for N-API #14256

Closed
refack opened this issue Jul 15, 2017 · 13 comments
Closed

n-api: from zzo38 via IRC - Some ideas for N-API #14256

refack opened this issue Jul 15, 2017 · 13 comments
Labels
doc Issues and PRs related to the documentations. feature request Issues that request new features to be added to Node.js. node-api Issues and PRs related to the Node-API.

Comments

@refack
Copy link
Contributor

refack commented Jul 15, 2017

edited to reflect completed tasks (@gabrielschulhof)

  • Version: 8.1.3
  • Subsystem: n-api

http://zzo38computer.org/textfile/miscellaneous/napi_ext

documentation

String lifetimes.

The document should specify if N-API copies the string, to make it clear.
(With SQLite, you can specify whether or not the string should be copied
and if not, what to call to free it.)

Examples of asynchronous working.

There aren't any example, and it does not explain many things such as
mutexes, calling JavaScript codes during the working, example of using
napi_make_callback(), thread safety, etc.

SQLite has support for multithreads too (if enabled), although there is
some confusion in the N-API documentation as to what exactly is allowed
when interfacing SQLite (and other stuff) with N-API when using the
asynchronous working.

External values.

The documentation should probably mention that an external value appears
as an object (sealed, frozen, with no properties and no prototype) to
JavaScript codes, even though it is a separate type to native codes.

Get property names.

The documentation for napi_get_property_names() does not say whether it
gets only enumerable properties or all properties, and does not say if it
includes symbols or not.

C++ wrapping.

What is the relation to C++ (I thought this is a C API?) and what is the
reason for the restrictions with napi_wrap()? Also, is it necessary to
call napi_wrap() only during the constructor callback, or is it OK to do
afterward too?

Throwing exceptions.

The document says that napi_throw() throws an Error provided. Is it
necessary for it to be an Error, or can you throw any value? You should be
allowed to throw any value, I should think.

Implementation

Retrieving new.target.

When a function is called as a constructor, new.target should be
retrievable. This may replace is_construct_call since you can do the same
thing with this anyways.

napi_get_new_target()

Executing JavaScript codes.

Some things aren't and perhaps shouldn't be defined in N-API, but it can
be useful to execute external JavaScript codes sometimes (during
initialization, especially) in order to do such things.

napi_run_script()

Further module arguments.

It currently (seems to) provide no way to get the "require", "__dirname",
and "__filename" arguments of a Node.js module. If the native code is a
Node.js module there should probably be some way to retrieve such values
(which may be used with the above "executing JavaScript codes" during the
initialization).

Retrieving the finalizer for a value.

For objects created using N-API, it would help to have a function to get
the finalizer callback for the value, for example:

napi_get_value_finalizer(napi_env env,napi_value val,napi_finalize*result)

This can be used to determine who created an object, by comparing the
finalizer function pointer (which will be a null pointer if there is no
finalizer callback defined) with the address of the native code's own
functions (there is no point calling the returned finalizer callback).

Reading strings of 8-bit characters.

The function napi_get_value_string_latin1() is missing. Not sure what to
do for strings having character codes that don't fit in 8-bits; it may be
defined to just use the low 8-bits, or it may be specified as undefined.

Support for WeakMap.

N-API can already create and manipulate a Array, but there is no support
to create and manipulate a WeakMap, which may sometimes be useful for
internally attaching extra data to arbitrary objects.

(You might be able to define your own finalizer callbacks also using a
WeakMap anyways, by giving a key for the object to check, and the value
being a external value with a finalizer callback defined, so that it will
be called when the key object is finalized; I don't know whether or not it
will work, but it seems like it is allowed to work, at least.)

Creating functions and objects.

When using napi_create_object() and napi_create_function(), you cannot
specify a finalizer callback, which probably should be allowed since it
can be useful to have.

Also, napi_create_object() should allow you to optionally specify the
prototype to use (which is a napi_value which is either null or a object;
if it is not specified then it can use the default Object.prototype).

napi_new_instance()

Exotic objects.

A suggestion is a new N-API function to create custom exotic objects (like
Proxy in JavaScript, but without the extra checking).

Generator functions.

A way to create generator functions with N-API functions. (You can already
fake it, but real generator functions may help a bit better, will work if
you use the actual generator methods rather than others with the same
name, and may be more efficient in some cases maybe.)

Attaching a finalizer callback to the generator object can also help.

Immediate garbage collection.

Sometimes you may wish to trigger garbage collection sooner in order to
make finalizer callbacks to be called. This will not necessarily be
guaranteed, although attempting it can nevertheless be useful sometimes.
(This should also be provided in the "v8" module so that it can be used
from JavaScript codes directly, too.)

Moving values into scopes.

Sometimes you might want to return a value that is not scoped and then to
delete the reference to it at the same time. In order to do this, it may
be useful to add the value to the scope so that it can be returned.

N-API might automatically do this; the documentation isn't quite clear. If
N-API does already do this, it should be explained, and the stuff in this
section need not be implemented.

Another alternative would be to provide another function to return values
from a native code rather than returning it directly.

Another possibility may be to create an array and add the value into that
array to prevent it from getting lost, and the array is automatically lost
when the function returns, but that seems a bit klugy to me.

@refack refack added discuss Issues opened for discussions and feedbacks. doc Issues and PRs related to the documentations. feature request Issues that request new features to be added to Node.js. node-api Issues and PRs related to the Node-API. labels Jul 15, 2017
@refack
Copy link
Contributor Author

refack commented Jul 15, 2017

@nodejs/n-api @mhdawson some more feedback from zzo38

@addaleax addaleax removed the discuss Issues opened for discussions and feedbacks. label Jul 16, 2017
@mhdawson
Copy link
Member

@zzo38 thanks for the feedback will review and think about how to address.

@mhdawson
Copy link
Member

@refack how did you get the feedback ? I'm thinking we may want to invite @zzo38r to our weekly N-API meeting to go through the feedback and have a discussion.

One question I have is if they are porting are particular module(s) and these are suggestions based on the port. It would help in prioritization.

@refack refack changed the title n-api: from zzo38 via IRC — Some ideas for N-API n-api: from zzo38 via IRC - Some ideas for N-API Jul 18, 2017
@refack
Copy link
Contributor Author

refack commented Jul 18, 2017

@mhdawson zzo38 has been available only on IRC - http://logs.libuv.org/node-dev/2017-07-08#18:38:01.168

@jasongin
Copy link
Member

jasongin commented Jul 18, 2017

What is the relation to C++

I've noticed several people are confused about this. While we have some mention in the documentation, it is obviously not clear enough or prominent enough.

Examples of asynchronous working

Yes, we discussed this at the N-API WG meeting just last week. Better documentation and examples are definitely needed for async.

document says that napi_throw() throws an Error provided. Is it necessary for it to be an Error, or can you throw any value

Yes it allows any kind of value, though AFAIK it's bad practice to throw non-Error values.

The function napi_get_value_string_latin1() is missing.

It is implemented, but is missing from the documentation!

When using napi_create_object() and napi_create_function(), you cannot specify a finalizer callback, which probably should be allowed since it can be useful to have

You can specify a finalizer callback when creating any kind of value that has native data attached to it, including external values, wrapped native instances, typed arrays, and buffers. While I can't think of a scenario where you would need a finalizer for an object that has no native data, if necessary there is an easy workaround: Create a napi_external with a finalizer and attach it as a property on the object or function. The property will have the same lifetime so the finalizer will be invoked at the same time.

Retrieving new.target

This is tracked as nodejs/abi-stable-node#200, but has been considered lower priority.

Executing JavaScript code

It's tracked as nodejs/abi-stable-node#51. @mhdawson was there a reason we didn't open an issue in the node repo for this one? I think this is one of the higher-priority gaps we had identified.

Moving values into scopes

I don't understand the feedback here. Is this about something different from napi_escapable_handle_scope? Or suggesting a better way to work with escapable scopes?

tniessen pushed a commit that referenced this issue Aug 16, 2017
* Reordered string functions alphabetically
* Fixed a typo in napi_get_value_string_utf8

PR-URL: #14678
Fixes: #14397
Refs: #14256
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
MSLaguana pushed a commit to nodejs/node-chakracore that referenced this issue Aug 21, 2017
* Reordered string functions alphabetically
* Fixed a typo in napi_get_value_string_utf8

PR-URL: nodejs/node#14678
Fixes: nodejs/node#14397
Refs: nodejs/node#14256
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
MylesBorins pushed a commit that referenced this issue Sep 10, 2017
* Reordered string functions alphabetically
* Fixed a typo in napi_get_value_string_utf8

PR-URL: #14678
Fixes: #14397
Refs: #14256
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
MylesBorins pushed a commit that referenced this issue Sep 12, 2017
* Reordered string functions alphabetically
* Fixed a typo in napi_get_value_string_utf8

PR-URL: #14678
Fixes: #14397
Refs: #14256
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
@bnoordhuis
Copy link
Member

This issue has been dormant since mid-July. Can it be closed?

gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this issue Apr 10, 2018
* Reordered string functions alphabetically
* Fixed a typo in napi_get_value_string_utf8

PR-URL: nodejs#14678
Fixes: nodejs#14397
Refs: nodejs#14256
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
MylesBorins pushed a commit that referenced this issue Apr 16, 2018
* Reordered string functions alphabetically
* Fixed a typo in napi_get_value_string_utf8

Backport-PR-URL: #19447
PR-URL: #14678
Fixes: #14397
Refs: #14256
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
@gabrielschulhof
Copy link
Contributor

We've since added some of these things.

@devsnek
Copy link
Member

devsnek commented Apr 25, 2018

Executing JavaScript codes.

i've got some code locally that runs scripts and modules, just waiting on #20170 to figure out what to call the types that stays consistent

@gabrielschulhof
Copy link
Contributor

gabrielschulhof commented Apr 25, 2018 via email

@mhdawson
Copy link
Member

mhdawson commented Apr 27, 2018

I think it should be left open at least until we capture any remaining issues somewhere. @gabrielschulhof would you be able to review and open an issue in abi-stable-node to capture those that have not been addressed?

@gabrielschulhof
Copy link
Contributor

I've edited the original comment, striking out parts that are already implemented. I've created issues for the rest.

@mhdawson
Copy link
Member

@gabrielschulhof I think we can close this. @refack do you agree?

@refack
Copy link
Contributor Author

refack commented Apr 30, 2018

I was only the steward for this report... I trust you know how to handle this better than me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. feature request Issues that request new features to be added to Node.js. node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

No branches or pull requests

7 participants