First pass at basic Node Addon API docs #268

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

doc/basic_types.md

+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

doc/basic_types.md

+- `[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

doc/basic_types.md

+- `[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

doc/basic_types.md

+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

doc/basic_types.md

+```
+
+Gets the underlying `napi_valuetype` of the value.


Maybe it's more appropriate use: Returns

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

doc/basic_types.md

+`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

doc/basic_types.md

+
+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

doc/basic_types.md

+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

doc/basic_types.md

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


empy -> empty

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

doc/basic_types.md

+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

doc/basic_types.md

+```
+
+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

doc/basic_types.md

+```
+
+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

doc/basic_types.md

+- `[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

doc/basic_types.md

+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

doc/basic_types.md

+
+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

doc/basic_types.md

+```
+
+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

doc/basic_types.md

+
+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

doc/basic_types.md

+occurs, a `Napi::Error` will get thrown.
+
+#### New


This should be #### Length

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

doc/string.md

@@ -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

doc/string.md

+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

doc/basic_types.md

-[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

doc/basic_types.md

+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

doc/basic_types.md

+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

doc/basic_types.md

+
+## 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

doc/basic_types.md

+
+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

doc/basic_types.md Outdated

occurs, a `Napi::Error` will get thrown.

#### New

Copy link

Member

mhdawson May 17, 2018

Choose a reason for hiding this comment

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

This should be #### Length

mhdawson reviewed May 17, 2018

View reviewed changes

doc/string.md

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

```

Copy link

Member

mhdawson May 17, 2018

Choose a reason for hiding this comment

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

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

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] value` - The underlying JavaScript value that the `Value` instance represents

		Used to create a Node addon API `Value` that represents the `napi_value` passed

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

		Tests if this value strictly equals another value.

		`napi_coerce_to_object`. This will throw a JavaScript exception if the
		coercion fails.


		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`.

		created from the JavaScript primitive. Note that the value is not coerced to a
		string.


		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.

		```

		Returns the underlying N-API value primitive. If the instance is _empty_,

		```

		Returns `true` if this value strictly equals another value, or `false` otherwise

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


		This conversion does NOT coerce the type. Calling any methods inappropriate for
		the actual value type will throw `Napi::Error`.

		```

		Gets the environment the value is associated with. See `napi_env` for more


		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


		## 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

First pass at basic Node Addon API docs #268

First pass at basic Node Addon API docs #268

Conversation

digitalinfinity commented May 10, 2018

NickNaso left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhdawson commented May 17, 2018

mhdawson commented May 30, 2018

digitalinfinity commented May 30, 2018

digitalinfinity commented Jun 7, 2018

gabrielschulhof left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhdawson left a comment

Choose a reason for hiding this comment

mhdawson commented Jun 14, 2018

mhdawson commented Jun 14, 2018