Tweaks to API suggested on EPS

[mhdawson]

• Version: All
• Platform: ALL
• Subsystem: API

Suggestions from Ben on nodejs/node-eps#20 (comment)

1. 00X-ABI-Stable-Module-API.md

+
+### Types
+```C
+typedef struct napi_env__ *napi_env;
@bnoordhuis
bnoordhuis 5 hours ago Node.js Foundation member

Using typedef'd pointers has the issue that they will silently decay to void* when used in a void* context. Can be avoided by wrapping the pointer in a struct, like so:

typedef struct napi_value { struct napi_value *v; } napi_value;

2. 00X-ABI-Stable-Module-API.md

+
+struct napi_method_descriptor {
+  napi_callback callback;
+  char* utf8name;
@bnoordhuis
bnoordhuis 5 hours ago Node.js Foundation member

const char*?

3. 00X-ABI-Stable-Module-API.md

+NODE_EXTERN int napi_get_string_length(napi_env e, napi_value v);
+NODE_EXTERN int napi_get_string_utf8_length(napi_env e, napi_value v);
+NODE_EXTERN int napi_get_string_utf8(napi_env e, napi_value v,
+                                     char* buf, int bufsize);
@bnoordhuis
bnoordhuis 5 hours ago Node.js Foundation member

Why int instead of size_t?

4. 00X-ABI-Stable-Module-API.md

+// Methods to support error handling
+NODE_EXTERN void napi_throw(napi_env e, napi_value error);
+NODE_EXTERN void napi_throw_error(napi_env e, char* msg);
+NODE_EXTERN void napi_throw_type_error(napi_env e, char* msg);
@bnoordhuis
bnoordhuis 5 hours ago Node.js Foundation member

const char*

[sampsongao]

For the third point, I think v8's string API takes int as parameters. So we should keep it consistent to v8 api.

[digitalinfinity]

A quick skim through node_api_types.h and node_api.h indicate that most of these have been addressed with the exception of the first item. @mhdawson @jasongin can one of you sanity check for me please? I assume we'll want to do the first item soon?

[jasongin]

The first suggestion does not break the ABI, though it could break source code, if someone is implicitly converting one of those typedefs to void. So if we're going to do it, we should do it soon. It's just a minor thing though, so not critical in my opinion.

[jasongin]

And yes, items 2, 3, and 4 were resolved long ago.

[mhdawson]

Looks like these have all been addressed long ago, closing. Please let us know if that was not the right thing to do.