Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation mismatch for N-API structures #13469

Closed
RReverser opened this issue Jun 5, 2017 · 5 comments
Closed

Documentation mismatch for N-API structures #13469

RReverser opened this issue Jun 5, 2017 · 5 comments
Labels
doc Issues and PRs related to the documentations. node-api Issues and PRs related to the Node-API.

Comments

@RReverser
Copy link
Member

Documentation on the N-API page describes napi_property_attributes as following:

typedef enum {
  napi_default = 0,
  napi_read_only = 1 << 0,
  napi_dont_enum = 1 << 1,
  napi_dont_delete = 1 << 2,
  napi_static_property = 1 << 10,
} napi_property_attributes;

whereas src/node_api_types.h describes it as:

typedef enum {
  napi_default = 0,
  napi_writable = 1 << 0,
  napi_enumerable = 1 << 1,
  napi_configurable = 1 << 2,

  // Used with napi_define_class to distinguish static properties
  // from instance properties. Ignored by napi_define_properties.
  napi_static = 1 << 10,
} napi_property_attributes;

Basically, values of all bit flags were inverted.

Given the last commit, it seems it's the documentation that is outdated here but would be nice to confirm and sync two places to agree with each other.

Another one is napi_property_descriptor - website docs don't mention second name field of type napi_value.

@mscdex mscdex added doc Issues and PRs related to the documentations. node-api Issues and PRs related to the Node-API. labels Jun 5, 2017
@mscdex
Copy link
Contributor

mscdex commented Jun 5, 2017

/cc @nodejs/n-api

@RReverser
Copy link
Member Author

@mscdex Right, thanks, forgot I can add labels and mention corresponding group myself...

@jasongin
Copy link
Member

jasongin commented Jun 5, 2017

Yes, the documentation is out of date. These changes were made in the code after that part of the documentation was originally written, and we forgot to update the doc.

@mhdawson
Copy link
Member

mhdawson commented Jun 5, 2017

Will take a look at updating this tomorrow.

mhdawson added a commit to mhdawson/io.js that referenced this issue Jun 6, 2017
@RReverser
Copy link
Member Author

RReverser commented Jun 8, 2017

Another one: napi_create_symbol expects a char * for description according to docs, but napi_value in actual header file. Not sure which one is intended here (I would expect the char *)?

Moved to a separate issue: #13555

RReverser added a commit to RReverser/emnapi that referenced this issue Jun 8, 2017
addaleax pushed a commit that referenced this issue Jun 10, 2017
PR-URL: #13508
Fixes: #13469
Fixes: #13458
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this issue Apr 10, 2018
PR-URL: nodejs#13508
Fixes: nodejs#13469
Fixes: nodejs#13458
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
MylesBorins pushed a commit that referenced this issue Apr 16, 2018
Backport-PR-URL: #19447
PR-URL: #13508
Fixes: #13469
Fixes: #13458
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jason Ginchereau <jasongin@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

No branches or pull requests

4 participants