Function and FunctionReference documentation by NickNaso · Pull Request #299 · nodejs/node-addon-api

NickNaso · 2018-07-12T10:01:38Z

First pass on function documentation. Please review only function.md. I'm actually writing the function reference documentation and I will push it as soon as possible.

mhdawson · 2018-07-12T21:31:39Z

-[C++ wrapper classes for the ABI-stable C APIs for Node.js](https://nodejs.github.io/node-addon-api/)
+The **Function** class is a representation of the JavaScript function. A function
+is a JavaScript procedure, a set of statements that performs a task or calculates
+a value. This class provides some methods to create and exectue a function.


How about
+The Function class is a representation of the JavaScript function. A function
+is a set of statements that performs a task or calculates
+a value. This class provides some methods to create and execute a function.

mhdawson · 2018-07-12T21:33:01Z

+  - std::string (encoded using UTF-8)
+  - std::u16string
+  - napi::Value
+  - napi_value


I don't think this is right? It likely needs to be a napi_value which is handle for a JavaScript function.

mhdawson · 2018-07-12T21:33:49Z

+```
+
+- `[in] env`: The `napi_env` environment in which to construct the Function object.
+- `[in] cb`: Object that implement the operator **()** accepting a `const CallbackInfo&`


Should this just say it needs to be an instance of Callable ?

is it good for you: Object that implements Callable.?

mhdawson · 2018-07-12T21:34:21Z

+
+- `[in] env`: The `napi_env` environment in which to construct the Function object.
+- `[in] cb`: Object that implement the operator **()** accepting a `const CallbackInfo&`
+and returns either void or Value.


Same comment as above.

Ditto regarding "void or Value".

mhdawson · 2018-07-12T21:35:40Z

+```
+
+- `[in] recv`: The `this` object passed to the called function.
+- `[in] args`: List of JavaScript values as `napi_value` representing the


Maybe

Vector of JavaScript values as napi_value representing the

mhdawson · 2018-07-12T21:38:28Z

+- `[in] args`: Initializer list of JavaScript values as `napi_value` representing
+the arguments of the function.
+
+Returns a Value representing the JavaScript object returned by the function.


I think we need to have somewhere, either in the intro or in the description of the MakeCallback function where you should use Call versus MakeCallback. You should use Call if there is already a JavaScript function on the stack (for example when running a native method called from JavaScript), and only use MakeCallback in special cases like running OnError, OnOk in AsyncWorker which don't have an existing JavaScript function on the stack.

mhdawson · 2018-07-12T21:40:08Z

+
+Returns a Value representing the JavaScript object returned by the function.
+
+### New


I think all the new methods should be together.

mhdawson · 2018-07-12T21:40:23Z

+
+### Constructor
+
+Creates a new


partial line.

mhdawson · 2018-07-12T21:40:49Z

+FunctionReference();
+```
+
+- `[in] Env`: The environment in which to construct the


partial line. I see that in a bunch of places throughout.

mhdawson · 2018-07-12T21:42:10Z

+
+- `[in] Env`: The environment in which to construct the
+
+Returns a new


Function reference overall looks like its missing the substitution. For example "Returns a new" should probably be "Returns a new FunctionReference"

mhdawson · 2018-07-12T21:42:41Z

+
+- `[in] Env`: The environment in which to construct the
+
+Returns a new


I think we also need some explanation of why you use a FunctionReference instead of a Function and when.

mhdawson · 2018-07-18T16:43:45Z

@NickNaso just wondering if you are going to be able to update this week?

gabrielschulhof · 2018-07-18T16:46:53Z

+
+### Constructor
+
+Creates a new instance of `Function` object.


of the Function object.

gabrielschulhof · 2018-07-18T16:47:52Z

+
+### New
+
+Creates instance of a `Function` object.


Creates an instance

gabrielschulhof · 2018-07-18T16:48:32Z

+```
+
+- `[in] env`: The `napi_env` environment in which to construct the Function object.
+- `[in] cb`: Object that implement the operator **()** accepting a `const CallbackInfo&`


gabrielschulhof · 2018-07-18T16:49:49Z

+
+- `[in] env`: The `napi_env` environment in which to construct the Function object.
+- `[in] cb`: Object that implement the operator **()** accepting a `const CallbackInfo&`
+and returns either void or Value.


Perhaps change this to "void or a Napi::Value".

gabrielschulhof · 2018-07-18T16:50:20Z

+- `[in] data`: User-provided data context. This will be passed back into the
+function when invoked later.
+
+Returns instance of a `Function` object.


an instance

gabrielschulhof · 2018-07-18T16:51:09Z

+
+- `[in] env`: The `napi_env` environment in which to construct the Function object.
+- `[in] cb`: Object that implement the operator **()** accepting a `const CallbackInfo&`
+and returns either void or Value.


Ditto regarding "void or Value".

gabrielschulhof · 2018-07-18T16:51:23Z

+- `[in] data`: User-provided data context. This will be passed back into the
+function when invoked later.
+
+Returns instance of a `Function` object.


an instance

gabrielschulhof · 2018-07-18T16:52:45Z

+- `[in] args`: Initializer list of JavaScript values as `napi_value` representing
+the arguments of the function.
+
+Returns a Value representing the JavaScript object returned by the function.


"Returns a Napi::Value representing the JavaScript value..." we shouldn't say "JavaScript object" because that has a special meaning.

You're right. Sorry.
Done.

NP 🙂 Weird though ... github hasn't updated to show that 😕

Oh, I see - you fixed the ones below, but you missed this one. NP, but please say "`Napi::Value`" rather than merely "`Value`" – that is, please use the name space with the type name.

gabrielschulhof · 2018-07-18T16:53:21Z

+- `[in] args`: List of JavaScript values as `napi_value` representing the
+arguments of the function.
+
+Returns a Value representing the JavaScript object returned by the function.


gabrielschulhof · 2018-07-18T16:53:56Z

+- `[in] args`: Array of JavaScript values as `napi_value` representing the
+arguments of the function.
+
+Returns a Value representing the JavaScript object returned by the function.


Ditto also for all instances to follow.

gabrielschulhof · 2018-07-18T16:57:11Z

+arguments of the function.
+
+Returns a Value representing the JavaScript object returned by the function.
+


I'm surprised we don't have overloads which accept an array of Napi::Value items 😕

So do I, what do you think if I open an issue about that? Just to remember this and maybe after the documentation I can try to work on that.

From what I recall, these were my reasons for not adding such an overload:

Function arguments are almost always specified inline with the function call, not prepared ahead of time as an array.

Such an overload would be less efficient. The underlying napi_call_function() takes an array of napi_value. So converting from an array of Napi::Value to that would require allocating a new array and copying each of the (non-contiguous) napi_value elements into the array.

The implicit conversion operator napi_value means it's usually just as convenient to create a vector or std::initializer_list of napi_value instead of Napi::Value. And it's more efficient because it uses less memory by not needlessly duplicating the env value.

But I can understand why the lack of such an overload could be unexpected, given the pattern of most other wrapper functions that take Napi::Value types directly.

Good points. Thanks for sharing this.

NickNaso · 2018-07-18T19:48:09Z

@mhdawson I'm starting working now.

jasongin · 2018-07-19T21:28:43Z

+Calls a Javascript function from a native add-on.
+
+```cpp
+Value Call(const std::initializer_list<napi_value>& args) const;


This needs an example, because some people may less familiar with syntax for initializer_list. Especially since this is the most convenient and efficient overload to use for most typical cases.

NickNaso · 2018-08-07T18:05:43Z

Just tried to address all the things issued after first review.

mhdawson

LGTM @NickNaso pushed a couple of suggestions, please take a look an let me know if they look good to you.

NickNaso · 2018-08-07T21:29:42Z

@mhdawson They are both good to me. Thanks for your help.

mhdawson · 2018-08-07T21:30:46Z

@NickNaso thanks, will leave until tomorrow to let @gabrielschulhof time to take a look and then will go ahead and land.

gabrielschulhof · 2018-08-08T20:19:13Z

-You are reading a draft of the next documentation and it's in continuous update so
-if you don't find what you need please refer to:
-[C++ wrapper classes for the ABI-stable C APIs for Node.js](https://nodejs.github.io/node-addon-api/)
+The **Function** class provides a set of methods to create a function object in


"... a set of methods ~~to create~~for creating ..."

gabrielschulhof · 2018-08-08T20:19:52Z

-[C++ wrapper classes for the ABI-stable C APIs for Node.js](https://nodejs.github.io/node-addon-api/)
+The **Function** class provides a set of methods to create a function object in
+native code that can later be called from JavaScript. The created function is not
+automatically visible from JavaScript, instead it needs to be part of the add-on's


This is a run-on sentance. Let's have "... from JavaScript. Instead it ..."

gabrielschulhof · 2018-08-08T20:20:22Z

+The **Function** class provides a set of methods to create a function object in
+native code that can later be called from JavaScript. The created function is not
+automatically visible from JavaScript, instead it needs to be part of the add-on's
+module exports or be returned by one of the modules exported functions.


"module's" instead of "modules"

gabrielschulhof · 2018-08-08T20:21:09Z

+module exports or be returned by one of the modules exported functions.
+
+In addition the `Function` class also provides methods that can be used to call
+functions that were created in JavaScript and passed to the native.


"... to the native add-on", perhaps?

gabrielschulhof · 2018-08-08T20:22:15Z

+addon.fn();
+```
+
+The `Function` class allows to call a JavaScript function object from a native


Maybe we should rephrase this as "With the Function class it is possible to call..."

gabrielschulhof · 2018-08-08T20:34:50Z

-if you don't find what you need please refer to:
-[C++ wrapper classes for the ABI-stable C APIs for Node.js](https://nodejs.github.io/node-addon-api/)
+FunctionReference is a subclass of [Reference](reference.md), and is equivalent to
+an instance of `Reference<Function>`. This means that a FunctionReference holds a 


"FunctionReference" should be in backticks.

gabrielschulhof · 2018-08-08T20:35:23Z

-[C++ wrapper classes for the ABI-stable C APIs for Node.js](https://nodejs.github.io/node-addon-api/)
+FunctionReference is a subclass of [Reference](reference.md), and is equivalent to
+an instance of `Reference<Function>`. This means that a FunctionReference holds a 
+[Function](function.md), and a count of the number of references to that Function. 


Backticks around "Function" (both in the link and at the end of the line).

gabrielschulhof · 2018-08-08T20:37:00Z

+FunctionReference is a subclass of [Reference](reference.md), and is equivalent to
+an instance of `Reference<Function>`. This means that a FunctionReference holds a 
+[Function](function.md), and a count of the number of references to that Function. 
+When the count is greater than 0, a FunctionReference is not eligible for garbage 


Backticks around FunctionReference please!

In general, please make another pass and add backticks around type names etc.!

gabrielschulhof · 2018-08-08T20:42:55Z

+### New
+
+Creates a new JavaScript value from one that represents the constructor for the
+object.


Perhaps "Constructs a new instance by calling the constructor held by this reference."?

gabrielschulhof · 2018-08-08T20:44:56Z

+- `[in] args`: Vector of JavaScript values as napi_value representing the 
+arguments of the constructor function.
+
+Returns a new JavaScript object.


Ditto ... ?

NickNaso · 2018-08-21T15:18:25Z

@gabrielschulhof Could you take a look at my last update? Thanks :-)

gabrielschulhof · 2018-08-11T21:36:03Z

+module exports or be returned by one of the module's exported functions.
+
+In addition the `Function` class also provides methods that can be used to call
+functions that were created in JavaScript and passed to the native  add-on.


nit: two spaces between "native" and "add-on"

gabrielschulhof · 2018-08-11T21:36:59Z

+With the `Function` class it is possible to call a JavaScript function object
+from a native add-on with two different methods: `Call` and `MakeCallback`.
+The API of these two methods is very similar, but they are used in different
+context. The `MakeCallback` method is used to call from native code back into


nit: "contexts"

gabrielschulhof · 2018-08-11T21:37:56Z

+```
+
+- `[in] env`: The `napi_env` environment in which to construct the Value object.
+- `[in] value`: The `napi_value` which is handle for a JavaScript function.


"...which is a handle..."

gabrielschulhof · 2018-08-21T16:50:22Z

+static Function New(napi_env env, Callable cb, const char* utf8name = nullptr, void* data = nullptr);
+```
+
+- `[in] env`: The `napi_env` environment in which to construct the Function object.


Please put backticks around "Function"

gabrielschulhof · 2018-08-21T16:56:31Z

-if you don't find what you need please refer to:
-[C++ wrapper classes for the ABI-stable C APIs for Node.js](https://nodejs.github.io/node-addon-api/)
+`FunctionReference` is a subclass of [Reference](reference.md), and is equivalent to
+an instance of `Reference<Function>`. This means that a FunctionReference holds a


Please put backticks around "FunctionReference"!

gabrielschulhof · 2018-08-21T17:17:44Z

+When the count is greater than 0, a FunctionReference is not eligible for garbage
+collection. This ensures that the `Function` will remain accessible, even if the
+original reference to it is no longer available.
+`FunctionReference` allows the referenced JavaScript function object  to be called


Two spaces between "object" and "to".

gabrielschulhof · 2018-08-21T17:18:33Z

+### Weak
+
+Creates a "weak" reference to the value, in that the initial count of number of
+references is set to 0.


I think it's sufficient to say "in that the initial reference count is set to 0."

gabrielschulhof · 2018-08-21T17:19:13Z

+### Persistent
+
+Creates a "persistent" reference to the value, in that the initial count of
+number of references is set to 1.


Ditto re "reference count".

gabrielschulhof · 2018-08-21T17:19:51Z

+Napi::Object New(const std::initializer_list<napi_value>& args) const;
+```
+
+- `[in] args`: Initializer list of JavaScript values as napi_value representing


Backticks around "napi_value", please!

gabrielschulhof · 2018-08-21T17:20:14Z

+Napi::Object New(const std::vector<napi_value>& args) const;
+```
+
+- `[in] args`: Vector of JavaScript values as napi_value representing the


Backticks around "napi_value", please!

mhdawson · 2018-08-22T21:28:28Z

@gabrielschulhof I think this is waiting for you now.

gabrielschulhof · 2018-08-23T18:26:07Z

I actually have to take a few hours before I can review. BRB 🙂

gabrielschulhof · 2018-08-23T18:25:21Z

+Function(napi_env env, napi_value value);
+```
+
+- `[in] env`: The `napi_env` environment in which to construct the Value object.


"... the Function object." (with backticks around "Function").

gabrielschulhof · 2018-08-23T20:40:29Z

+static Function New(napi_env env, Callable cb, const std::string& utf8name, void* data = nullptr);
+```
+
+- `[in] env`: The `napi_env` environment in which to construct the Function object.


Backticks around "Function"

gabrielschulhof · 2018-08-23T20:43:39Z

+- `[in] env`: The `napi_env` environment in which to construct the Value object.
+- `[in] value`: The `napi_value` which is a handle for a JavaScript function.
+
+Returns a non-empty `Function` instance.


We should really be consistent about whether we say Napi::Value, Napi::Function, Napi::FunctionReference, etc. or Value, Function, or FunctionReference. IMO we should always add the name space in front of the class name. @mhdawson what do you think?

Ok for me. Is it ok for you @mhdawson ?

gabrielschulhof · 2018-08-23T20:46:59Z

-You are reading a draft of the next documentation and it's in continuous update so
-if you don't find what you need please refer to:
-[C++ wrapper classes for the ABI-stable C APIs for Node.js](https://nodejs.github.io/node-addon-api/)
+`FunctionReference` is a subclass of [Reference](reference.md), and is equivalent to


Here too, and in all cases where we use class names in the documentation, we should add the name space explicitly – that is, Napi::FunctionReference and Napi::Reference in this case. Additionally, "Reference" should be in backquotes.

@NickNaso the word "Reference" inside the square bracket also needs backticks.

gabrielschulhof · 2018-08-23T20:48:09Z

+instead of `MakeCallback` and vice-versa.
+
+The `FunctionReference` class inherits its behavior from the `Reference` class
+(for more info see: [Reference](reference.md)).


Backticks around "Reference"

gabrielschulhof · 2018-08-23T20:48:46Z

+```
+
+- `[in] env`: The environment in which to construct the FunctionReference object.
+- `[in] ref`: The N-API reference to be held by the FunctionReference.


Backticks around "FunctionReference".

gabrielschulhof · 2018-08-24T12:12:45Z

@mhdawson shall we go with full-name-space notation like Napi::Value throughout, or shall Value suffice? IMO we should go with full-name-space notation, but I do not cling strongly to that opinion 🙂 Either way, we should be consistent throughout the docs.

mhdawson · 2018-08-29T20:30:54Z

I agree that full-name-space would be best.

mhdawson · 2018-09-06T17:51:36Z

ok, lets land and then update to always use the name-space

PR-URL: #299 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

mhdawson · 2018-09-06T20:17:23Z

Landed as 757eb1f

PR-URL: nodejs/node-addon-api#299 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

NickNaso added 2 commits June 19, 2018 01:44

Initial commit for function and function reference documentation

17b299d

First pass on function documentation

d2293c5

mhdawson reviewed Jul 12, 2018

View reviewed changes

Comment thread doc/function_reference.md Outdated

### Constructor

Creates a new

Copy link
Copy Markdown

Member

mhdawson Jul 12, 2018

Choose a reason for hiding this comment

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

partial line.

mhdawson reviewed Jul 12, 2018

View reviewed changes

gabrielschulhof reviewed Jul 18, 2018

View reviewed changes

jasongin reviewed Jul 19, 2018

View reviewed changes

NickNaso added 2 commits August 2, 2018 18:25

Fixed documentation for function as requested after first review

4a81a7d

Added function referercne documentation

ab4d8a6

NickNaso changed the title ~~Function documentation~~ Function and FunctionReference documentation Aug 7, 2018

mhdawson added 2 commits August 7, 2018 17:17

squash: add some suggested tweaks.

a8ca2cf

squash: add suggested tweaks

9469a00

mhdawson approved these changes Aug 7, 2018

View reviewed changes

Typo natice to native

a09045b

gabrielschulhof reviewed Aug 8, 2018

View reviewed changes

Just addressed changes requested after rview

8b3dbd8

gabrielschulhof reviewed Aug 21, 2018

View reviewed changes

Some little fix after review

6cf54c5

Missing corrections after last review

be41532

gabrielschulhof reviewed Aug 23, 2018

View reviewed changes

Added missing backticks

6bdfc56

Added backticks

fcc61a3

Added missing namespace

390ecd1

mhdawson pushed a commit that referenced this pull request Sep 6, 2018

doc: add function and function reference doc

757eb1f

PR-URL: #299 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

mhdawson closed this Sep 6, 2018

kevindavies8 added a commit to kevindavies8/node-addon-api-Develop that referenced this pull request Aug 24, 2022

doc: add function and function reference doc

d826a32

PR-URL: nodejs/node-addon-api#299 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Marlyfleitas added a commit to Marlyfleitas/node-api-addon-Development that referenced this pull request Aug 26, 2022

doc: add function and function reference doc

7728f69

PR-URL: nodejs/node-addon-api#299 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

wroy7860 added a commit to wroy7860/addon-api-benchmark-node that referenced this pull request Sep 19, 2022

doc: add function and function reference doc

abf10aa

PR-URL: nodejs/node-addon-api#299 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this pull request Aug 11, 2023

doc: add function and function reference doc

9608b3a

PR-URL: nodejs/node-addon-api#299 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>


		Returns a Value representing the JavaScript object returned by the function.

		### New


		- `[in] Env`: The environment in which to construct the

		Returns a new

		arguments of the function.

		Returns a Value representing the JavaScript object returned by the function.

Uh oh!

Conversation

NickNaso commented Jul 12, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mhdawson commented Jul 18, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NickNaso Jul 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NickNaso Jul 18, 2018 •

edited

Loading