Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign updoc: add blurb about implications of ABI stability #22508
Conversation
This comment has been minimized.
This comment has been minimized.
nodejs-github-bot
added
the
doc
label
Aug 24, 2018
gabrielschulhof
referenced this pull request
Aug 24, 2018
Closed
Document limitations of the ABI stability offered by N-API #332
addaleax
reviewed
Aug 24, 2018
|
|
||
| Although N-API provides an ABI stability guarantee, other parts of Node.js do | ||
| not, and any external libraries used from the addon may not. In particular, | ||
| neither of the following Node.js APIs provides an ABI stability guarantee: |
This comment has been minimized.
This comment has been minimized.
addaleax
Aug 24, 2018
Member
I think this wording is a bit misleading – we do have ABI stability guarantees, but we follow semver rather than not allowing any breakage.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
mscdex
Aug 24, 2018
•
Contributor
How about instead (s/neither/none/, s/Node.js//, s/provides/provide/):
In particular, none of the following APIs provide an ABI stability guarantee:
This comment has been minimized.
This comment has been minimized.
|
Perhaps this could be combined with #22237 or vice versa? |
addaleax
reviewed
Aug 24, 2018
| #include <node_object_wrap.h> | ||
| #include <node_perf_common.h> | ||
| #include <node_platform.h> | ||
| #include <node_version.h> |
This comment has been minimized.
This comment has been minimized.
addaleax
Aug 24, 2018
Member
Only node_buffer.h, node_version.h, node_object_wrap.h and node.h (+ N-API) are real public API. I’ll open a PR to put the others behind NODE_WANT_INTERNALS.
Trott
reviewed
Aug 25, 2018
| @@ -367,6 +367,10 @@ set of APIs that are used by the native code. Instead of using the V8 | |||
| or [Native Abstractions for Node.js][] APIs, the functions available | |||
| in the N-API are used. | |||
|
|
|||
| Creating and maintaining an add-on that benefits from the ABI stability | |||
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Trott
Aug 25, 2018
Member
Actually, I'll just go ahead and make that change myself since I'm right here...
lpinca
approved these changes
Aug 25, 2018
|
LGTM with comments addressed. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@addaleax I have updated the text to reflect your comments. |
mhdawson
approved these changes
Aug 29, 2018
|
LGTM - once linter is green... |
This comment has been minimized.
This comment has been minimized.
BridgeAR
added
the
author ready
label
Aug 31, 2018
This comment has been minimized.
This comment has been minimized.
addaleax
reviewed
Sep 2, 2018
| ```C | ||
| #include <node_api.h> | ||
| ``` | ||
| and by checking, for all external library that it uses, that the external |
This comment has been minimized.
This comment has been minimized.
gabrielschulhof
force-pushed the
gabrielschulhof:abi-stability-implications
branch
from
d331e95
to
7ba8ae4
Sep 3, 2018
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Landed in 7033fc7. |
gabrielschulhof commentedAug 24, 2018
Mention that ABI stability can be achieved only by linking to ABI-
stable parts of Node.js and to other libraries which are ABI-stable.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes