First pass at basic Node Addon API docs by digitalinfinity · Pull Request #268 · nodejs/node-addon-api

digitalinfinity · 2018-05-10T16:52:03Z

Added docs for Basic Types, Strings and Symbols. Feedback welcome, I think this will take a couple more iterations to get right.

NickNaso

I tried to give you some suggestions.

NickNaso · 2018-05-10T16:56:16Z

+Value is the base class of Node Addon API's fundamental object type hierarchy.
+It represents a JavaScript value of an unknown type. It is a thin wrapper around
+the N-API datatype `napi_value`. Methods on this class can be used to check
+the JavaScript type of the underlying `napi_value` and also convert to C++ 


NickNaso · 2018-05-10T17:00:07Z

+- `[in] env` - The `napi_env` environment in which to construct the Value object.
+- `[in] value` - The underlying JavaScript value that the `Value` instance represents
+
+Used to create a Node addon API `Value` that represents the `napi_value` passed


Maybe here we need to use: Returns a Node addon API Value that represents the napi_value passed in.

Agreed we to follow the standard format we should have a Returns after the parameters.

NickNaso · 2018-05-10T17:07:25Z

+
+- `[in] other` - The value to compare against
+
+Tests if this value strictly equals another value.


Maybe it's better move Tests if this value strictly equals another value. on top and here something like this: Returns true if the value isn strictly equals another value, or false otherwise.

NickNaso · 2018-05-10T17:14:50Z

+When C++ exceptions are disabled at compile time, a method with a `Value` 
+return type may return an empty value to indicate a pending exception. So when 
+not using C++ exceptions, callers should check whether the value is empty 
+before attempting to use it.


Maybe it's better to move this part on top and here: Returns true if the Value is an empty value.

About the error if the function that you are calling doesn't return value it' more appropriate use Env::IsExceptionPending() to retrieve if there is an active error.

NickNaso · 2018-05-10T17:16:00Z

+napi_valuetype Type() const;
+```
+
+Gets the underlying `napi_valuetype` of the value.


Maybe it's more appropriate use: Returns

NickNaso · 2018-05-10T17:26:12Z

+Converts a `Napi::Value` to a JavasScript Object. This is a wrapper around
+`napi_coerce_to_object`. This will throw a JavaScript exception if the 
+coercion fails.
+


Move this part on top and here something like this: Returns a JavaScript Object.

NickNaso · 2018-05-10T17:29:30Z

+## Name
+
+Names are JavaScript values that can be used as a property name. There are two
+specialized types of names supported in Node Addon API- `String` and `Symbol`.


Maybe you can link the doc of String and Symbol:
Names are JavaScript values that can be used as a property name. There are two
specialized types of names supported in Node Addon API- String and Symbol.

NickNaso · 2018-05-10T17:31:00Z

+created. If the constructor is invoked passing in an N-API value, then a Name is
+created from the JavaScript primitive. Note that the value is not coerced to a
+string.
+


Maybe it could be better to split this two cases

NickNaso · 2018-05-10T17:32:12Z

+```
+
+Returns an empy array. If an error occurs, a `Napi::Error` will get thrown.
+


empy -> empty

NickNaso · 2018-05-10T17:35:09Z

+
+Wraps a `napi_value` as a `Napi::Array`. If an error occurs, a `Napi::Error` 
+will get thrown.
+


Move this on top and here something like this: Returns the primitive top wrap.

mhdawson · 2018-05-17T13:36:07Z

+operator napi_value() const;
+```
+
+Returns the underlying N-API value primitive. If the instance is _empty_,


How about `Returns the underlying N-API napi_value'. If ...

mhdawson · 2018-05-17T13:37:18Z

+bool operator ==(const Value& other) const;
+```
+
+Returns `true` if this value strictly equals another value, or `false` otherwise


I think we need full sentences so needs a period at the end of the sentence. Say goes throughout the rest of the doc.

mhdawson · 2018-05-17T13:38:00Z

+
+- `[in] env` - The `napi_env` environment in which to construct the Value object.
+- `[in] value` - The C++ type to represent in JavaScript.
+


Needs a Returns... before the explanation of how it works.

mhdawson · 2018-05-17T13:39:29Z

+```cpp
+template <typename T> T As() const;
+```
+


Needs Returns.... same for other places, won't mention it specifically in those other places.

mhdawson · 2018-05-17T13:40:05Z

+Casts to another type of `Napi::Value`, when the actual type is known or assumed.
+
+This conversion does NOT coerce the type. Calling any methods inappropriate for 
+the actual value type will throw `Napi::Error`.


I think an example here would help.

mhdawson · 2018-05-17T13:40:46Z

+Napi::Env Env() const;
+```
+
+Gets the environment the value is associated with. See `napi_env` for more 


This should probably reference the Env class as opposed to napi_env.

mhdawson · 2018-05-17T13:41:43Z

+JavaScript `null` or `undefined`, which are valid values.
+
+When C++ exceptions are disabled at compile time, a method with a `Value` 
+return type may return an empty value to indicate a pending exception. So when 


So when --> Therefore when

mhdawson · 2018-05-17T13:45:57Z

+This method returns a native representation of a JavaScript array. If an error
+occurs, a `Napi::Error` will get thrown.
+
+#### New


This should be #### Length

mhdawson · 2018-05-17T13:48:39Z

@@ -39,13 +33,41 @@ operator std::u16string() const;
 ```


I think an example or more description might be useful here.

mhdawson · 2018-05-17T13:49:39Z

+String::New(napi_env env, const std::string& value);
+String::New(napi_env env, const std::u16string& value);
+String::New(napi_env env, const char* value);
+String::New(napi_env env, const char16_t* value);


We should talk about this. Its not consistent with anything I've seen elsewhere in the node documentation

But with overrides it does make sense to me

Yes, that was kind of what I was thinking. Anyone else have thoughts?

mhdawson · 2018-05-17T13:52:53Z

Left a few comments but a really good first cut.

mhdawson · 2018-05-30T00:45:02Z

@digitalinfinity are you going to get a chance to update for the comments?

digitalinfinity · 2018-05-30T01:15:02Z

@mhdawson yes- so sorry, I've been sidetracked with other things for the past couple weeks but I should be able to find some cycles this week to incorporate all the great feedback provided so far.

digitalinfinity · 2018-06-07T02:33:25Z

Ok, finally got a chance to incorporate the feedback. @NickNaso @mhdawson great feedback btw. @mhdawson I still need to add a couple examples but I think all of the other comments have been addressed.

gabrielschulhof

Just a few nits.

gabrielschulhof · 2018-06-13T19:22:03Z

-[C++ wrapper classes for the ABI-stable C APIs for Node.js](https://nodejs.github.io/node-addon-api/)
+Node Addon API consists of a few fundamental data types. These allow a user of 
+the API to create, convert and introspect fundamental JavaScript types, and
+interop with their C++ counterparts.


I think you should expand this to "interoperate".

gabrielschulhof · 2018-06-13T19:25:33Z

+Returns `true` if the value is uninitialized.
+
+An empty value is invalid, and most attempts to perform an operation on an 
+empty value will result in an exception. Note an empty value is distinct from


I don't think you need "Note" here.

gabrielschulhof · 2018-06-13T19:27:17Z

+bool IsBuffer() const;
+```
+
+Returns `true` if the underlying value is a Node `Buffer` or `false`


We should write "Node.js" instead of "Node".

gabrielschulhof · 2018-06-13T19:32:53Z

+
+## Array
+
+Arrays are native representations of JavaScript Arrays. `Napi::Array` is sugar


I think we should say "a wrapper" instead of "sugar".

gabrielschulhof · 2018-06-13T19:33:37Z

+
+Returns an empty array.
+
+If an error occurs, a `Napi::Error` will get thrown. If C++ exceptions are not


"will be thrown" might be better – here, and subsequently.

mhdawson

LGTM. After the few minor comments I think it is good to land and we can iterate after that if necessary.

mhdawson · 2018-06-14T21:41:26Z

I believe all comments are now addressed will land.

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

mhdawson · 2018-06-14T21:45:12Z

Landed as e029a07

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

NickNaso reviewed May 10, 2018

View reviewed changes

mhdawson reviewed May 17, 2018

View reviewed changes

digitalinfinity added 2 commits June 6, 2018 19:30

First pass at basic Node Addon API docs

93b99e0

Incorporate review feedback

f83f63f

digitalinfinity force-pushed the doc_updates branch from 4a0aee8 to f83f63f Compare June 7, 2018 02:32

gabrielschulhof reviewed Jun 13, 2018

View reviewed changes

mhdawson approved these changes Jun 14, 2018

View reviewed changes

squash: address comments

0c24234

mhdawson pushed a commit that referenced this pull request Jun 14, 2018

doc: First pass at basic Node Addon API docs

e029a07

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

mhdawson closed this Jun 14, 2018

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

doc: First pass at basic Node Addon API docs

4bc43ae

PR-URL: nodejs/node-addon-api#268 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: First pass at basic Node Addon API docs

c4afb51

PR-URL: nodejs/node-addon-api#268 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: First pass at basic Node Addon API docs

592e0b2

PR-URL: nodejs/node-addon-api#268 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: First pass at basic Node Addon API docs

f3a68b7

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


		- `[in] other` - The value to compare against

		Tests if this value strictly equals another value.

		```

		Returns an empy array. If an error occurs, a `Napi::Error` will get thrown.


		Wraps a `napi_value` as a `Napi::Array`. If an error occurs, a `Napi::Error`
		will get thrown.


		- `[in] env` - The `napi_env` environment in which to construct the Value object.
		- `[in] value` - The C++ type to represent in JavaScript.


		## Array

		Arrays are native representations of JavaScript Arrays. `Napi::Array` is sugar


		Returns an empty array.

		If an error occurs, a `Napi::Error` will get thrown. If C++ exceptions are not

Uh oh!

Conversation

digitalinfinity commented May 10, 2018

Uh oh!

NickNaso left a comment

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!

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 May 17, 2018

Uh oh!

mhdawson commented May 30, 2018

Uh oh!

digitalinfinity commented May 30, 2018

Uh oh!

digitalinfinity commented Jun 7, 2018

Uh oh!

gabrielschulhof left a comment

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 left a comment

Choose a reason for hiding this comment