doc: add blurb about implications of ABI stability - #22508
doc: add blurb about implications of ABI stability#22508gabrielschulhof wants to merge 3 commits into
Conversation
Sorry, something went wrong.
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.
|
@gabrielschulhof build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/664/pipeline |
Sorry, something went wrong.
|
|
||
| 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: |
There was a problem hiding this comment.
I think this wording is a bit misleading – we do have ABI stability guarantees, but we follow semver rather than not allowing any breakage.
Sorry, something went wrong.
There was a problem hiding this comment.
Good point - sorry!
Sorry, something went wrong.
There was a problem hiding this comment.
How about instead (s/neither/none/, s/Node.js//, s/provides/provide/):
In particular, none of the following APIs provide an ABI stability guarantee:
Sorry, something went wrong.
|
Perhaps this could be combined with #22237 or vice versa? |
Sorry, something went wrong.
| #include <node_object_wrap.h> | ||
| #include <node_perf_common.h> | ||
| #include <node_platform.h> | ||
| #include <node_version.h> |
There was a problem hiding this comment.
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.
Sorry, something went wrong.
| 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 |
There was a problem hiding this comment.
add-on -> addon for consistency with the rest of the doc.
Sorry, something went wrong.
There was a problem hiding this comment.
Actually, I'll just go ahead and make that change myself since I'm right here...
Sorry, something went wrong.
lpinca
left a comment
There was a problem hiding this comment.
LGTM with comments addressed.
Sorry, something went wrong.
|
@mscdex I believe there's a difference between this ABI compatibility note and the one in #22237, in that herein we deal with ABI compatibility across major versions of Node.js, whereas in the other we deal with ABI compatibility within a major version of Node.js. |
Sorry, something went wrong.
|
@addaleax I have updated the text to reflect your comments. |
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM - once linter is green...
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
There was a problem hiding this comment.
typo: libraries
Sorry, something went wrong.
Sorry, something went wrong.
|
Landed in 7033fc7. |
Sorry, something went wrong.
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. PR-URL: nodejs#22508 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
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. PR-URL: #22508 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
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. PR-URL: #22508 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
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