-
Notifications
You must be signed in to change notification settings - Fork 30.4k
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
doc: add blurb about implications of ABI stability #22508
doc: add blurb about implications of ABI stability #22508
Conversation
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.
doc/api/n-api.md
Outdated
|
||
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this wording is a bit misleading – we do have ABI stability guarantees, but we follow semver rather than not allowing any breakage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point - sorry!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about instead (s/neither/none/, s/Node.js//, s/provides/provide/):
In particular, none of the following APIs provide an ABI stability guarantee:
Perhaps this could be combined with #22237 or vice versa? |
doc/api/n-api.md
Outdated
#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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
.
doc/api/addons.md
Outdated
@@ -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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add-on
-> addon
for consistency with the rest of the doc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I'll just go ahead and make that change myself since I'm right here...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with comments addressed.
@addaleax I have updated the text to reflect your comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - once linter is green...
doc/api/n-api.md
Outdated
```C | ||
#include <node_api.h> | ||
``` | ||
and by checking, for all external library that it uses, that the external |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: libraries
d331e95
to
7ba8ae4
Compare
Landed in 7033fc7. |
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
make -j4 test
(UNIX), orvcbuild test
(Windows) passes