Basic data types documentation for nodejs/abi-stable-node#280. #165

jschlight · 2017-10-26T01:07:52Z

This is my initial hack at documentation for Basic Types. I'm open to all suggestions and criticisms to improve this document. Please let me know how it can be improved with additional content or format changes.

Initial hack at documetation for Basic Types.

mhdawson · 2017-10-27T13:57:33Z

doc/N-API Basic Types.md

+Value is a the base class upon which other JavaScipt values like Number, Boolean, String, and Object are based. 
+
+```
+Value();


For each of the methods I would add a heading. In this example:

Default Constructor

and we might want to add the Added in: which is used in the standard Node doc as well.

We may want to include the Returns line, if if it is just Returns void (or whatever we think is the best way to say returns nothing)

See https://nodejs.org/dist/latest-v8.x/docs/api/n-api.html#n_api_napi_create_reference in the N-API doc as the example I looked at to come up with these suggestions.

mhdawson · 2017-10-27T14:00:00Z

doc/N-API Basic Types.md

@@ -0,0 +1,373 @@
+# N-API Documentation


A general thought is that some intro at the top that introduces the basic types and how they are used might be useful before the section with the details. We can add that later though so what you have here would be good to land without that.

mhdawson · 2017-10-27T14:02:10Z

Overall a good start just a few suggestions. I also think we can incrementally land changes so if you like my suggestions we could integrate those (and any others made in the mean time), land what you have so far and then continue to add content in follow on PRs.

mhdawson · 2017-10-27T14:02:49Z

doc/N-API Basic Types.md

@@ -0,0 +1,373 @@
+# N-API Documentation


We might want to put a big "WORK IN PROGRESS, NOT YET COMPLETE" at the top just to set peoples expectations.

…o documentation-push

Includes Callback, Env, External, and Value.

jschlight · 2017-11-16T05:15:38Z

I've split the earlier single large md file into four separate files that fit into the new documentation scaffolding structure. I've implemented Michael's suggestions with the exception of the Added in: marker which will require further research.

mhdawson · 2017-11-16T19:19:46Z

doc/callbackinfo.md

+size_t Length() const;
+```
+
+Returns the number of arguemnts passed in the Callabckinfo object.


Callbackinfo -> CallbackInfo (capital I)

Just noticed arguemnts -> arguments

arguemnts -> arguments

mhdawson · 2017-11-16T19:22:35Z

doc/callbackinfo.md

+Value NewTarget() const;
+```
+
+Returns the `new.target` value of the constructor call. If the CallbackInfo is not a constructor call, a call to `IsEmpty()` on the returned value returns true.


This reads a bit strangely (to my ears). Maybe:

If the function that was invoked (and for which the CallbackInfo was passed) is not a constructor call....

There is also similar wording in a few other places.

mhdawson · 2017-11-16T19:23:50Z

doc/callbackinfo.md

+
+Returns the JavaScript `this` value for the call
+
+### Data


Would be good to provide a bit more details on what/how Data is used but that can be added in a follow on PR.

I'm not clear how Data is used. I'm assuming that it's a place the C++ programmer can store general information with the C++ object, but I'm not sure. If so, I can add that to the documentation.

I think we can add the info as a follow on PR we just need to record somewhere that we should come back and do that. We should generate a test that uses it which would provide an example to base the doc on.

mhdawson · 2017-11-16T19:26:40Z

doc/env.md

+
+The opaque data structure containing the environment in which the request is being run.
+
+The Env object is usually created and passed by the calling system.


Would it be better to say

"is usually created and passed by the Node.js runtime or node-addon-api infrastructure"

The same comment also applies other places were we say "calling system"

mhdawson · 2017-11-16T19:28:14Z

doc/external.md

+```cpp
+template <typename T>
+static External New(napi_env env,
+			T* data,


The parameters don't line up here.

mhdawson · 2017-11-16T19:29:22Z

doc/external.md

+```cpp
+template <typename T>
+static External New(napi_env env,
+			T* data,


parameters don't line up here

mhdawson · 2017-11-16T19:29:56Z

doc/external.md

+Returns a pointer to the arbitrary C++ data held by the External object.
+
+### Not documented here
+


Is this just because the doc will be added in future PR ? Or do you have some questions that need to be answered before you can add the doc for these ?

I wanted the reviewers to know that they're not documented. I'm pretty sure there's no need to document them. If so, I'll remove the note.

I'm thinking best to remove. It confused me so it might confuse others as well.

mhdawson

Looks pretty close, just a few comments/questions.

jschlight · 2017-11-17T21:35:55Z

Michael, thank you for your comments. I've implemented your recommendations, have pushed updated files, and added a couple of comments above.

mhdawson

Just a few more comments/typos

jschlight · 2017-11-20T22:18:55Z

Thanks, Michael. I've made another push to my branch.

mhdawson

LGTM

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

mhdawson · 2017-11-21T19:06:28Z

Landed as 7ecab46

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

See nodejs/abi-stable-node#280.

58709f1

Initial hack at documetation for Basic Types.

jschlight mentioned this pull request Oct 26, 2017

Documentation push on node-add-api nodejs/abi-stable-node#280

Closed

mhdawson reviewed Oct 27, 2017

View reviewed changes

jschlight added 3 commits November 15, 2017 15:31

Merge branch 'master' of https://github.com/nodejs/node-addon-api int…

9c58077

…o documentation-push

Documentation updates.

9aa0bd4

Includes Callback, Env, External, and Value.

Removes onsolete file.

ea4e5bf

mhdawson reviewed Nov 16, 2017

View reviewed changes

doc/external.md Outdated

```cpp

template <typename T>

static External New(napi_env env,

T* data,

Copy link

Member

mhdawson Nov 16, 2017

Choose a reason for hiding this comment

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

parameters don't line up here

mhdawson reviewed Nov 16, 2017

View reviewed changes

jschlight changed the title ~~See nodejs/abi-stable-node#280.~~ Basic data types documentation for nodejs/abi-stable-node#280. Nov 17, 2017

Documentation updates.

d3542d3

mhdawson reviewed Nov 20, 2017

View reviewed changes

Updates to basic data types documentation.

793b55b

mhdawson approved these changes Nov 21, 2017

View reviewed changes

mhdawson pushed a commit that referenced this pull request Nov 21, 2017

doc: Initial documetation for Basic Types.

7ecab46

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

mhdawson closed this Nov 21, 2017

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

doc: Initial documetation for Basic Types.

4bec321

PR-URL: nodejs/node-addon-api#165 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: Initial documetation for Basic Types.

8968634

PR-URL: nodejs/node-addon-api#165 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: Initial documetation for Basic Types.

16520da

PR-URL: nodejs/node-addon-api#165 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: Initial documetation for Basic Types.

7b27278

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic data types documentation for nodejs/abi-stable-node#280. #165

Basic data types documentation for nodejs/abi-stable-node#280. #165

jschlight commented Oct 26, 2017

mhdawson Oct 27, 2017

mhdawson Oct 27, 2017

mhdawson commented Oct 27, 2017

mhdawson Oct 27, 2017

jschlight commented Nov 16, 2017

mhdawson Nov 16, 2017

mhdawson Nov 20, 2017

mhdawson Nov 20, 2017

mhdawson Nov 16, 2017

mhdawson Nov 16, 2017

jschlight Nov 17, 2017

mhdawson Nov 20, 2017

mhdawson Nov 16, 2017

mhdawson Nov 16, 2017

mhdawson Nov 16, 2017

mhdawson Nov 16, 2017

jschlight Nov 17, 2017

mhdawson Nov 20, 2017

mhdawson left a comment

jschlight commented Nov 17, 2017

mhdawson left a comment

jschlight commented Nov 20, 2017

mhdawson left a comment

mhdawson commented Nov 21, 2017


		The opaque data structure containing the environment in which the request is being run.

		The Env object is usually created and passed by the calling system.

		Returns a pointer to the arbitrary C++ data held by the External object.

		### Not documented here

Basic data types documentation for nodejs/abi-stable-node#280. #165

Basic data types documentation for nodejs/abi-stable-node#280. #165

Conversation

jschlight commented Oct 26, 2017

Choose a reason for hiding this comment

Default Constructor

Choose a reason for hiding this comment

mhdawson commented Oct 27, 2017

Choose a reason for hiding this comment

jschlight commented Nov 16, 2017

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

Choose a reason for hiding this comment

jschlight commented Nov 17, 2017

mhdawson left a comment

Choose a reason for hiding this comment

jschlight commented Nov 20, 2017

mhdawson left a comment

Choose a reason for hiding this comment

mhdawson commented Nov 21, 2017