doc: add guide for updating N-API API surface #21877
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| # Contributing a new API to N-API | ||
|
|
||
| N-API is Node's next generation ABI-stable API for native modules. While improving the API surface is encouraged and welcomed, the following are a set of principles and guidelines to keep in mind while adding a new N-API API. | ||
|
|
||
|
|
||
| - A new API **must** adhere to N-API API shape and spirit | ||
|
There was a problem hiding this comment. nit: for consistency, please use * instead of - for bullet lists. |
||
| - **Must** be a C API | ||
| - **Must** not throw exceptions | ||
| - **Must** return napi_status | ||
|
|
||
| - **Should** consume napi_env | ||
|
|
||
| - **Must** operate only on primitive data types, pointers to primitive datatypes or opaque handles | ||
| - **Must** be a necessary API and not a nice to have. Convenience APIs belong in node-addon-api. | ||
| - **Must** not change the signature of an existing N-API API or break ABI compatibility with other versions of Node. | ||
|
There was a problem hiding this comment. Some points have a period in the end, some have not. Should we unify them? |
||
| - New API **should** be agnostic towards the underlying JavaScript VM | ||
| - New API request PRs **must** have a corresponding documentation update | ||
|
There was a problem hiding this comment. Is |
||
| - New API request PRs **must** be tagged as **n-api**. | ||
| - There **must** be at least one test case showing how to use the API | ||
| - There **should** be at least one test case per interesting use of the API. | ||
| - There **should** be a sample provided that operates in a realistic way (operating how a real addon would be written) | ||
| - A new API **should** be discussed at the N-API working group meeting | ||
| - A new API addition **must** be signed off by at least two members of the N-API WG | ||
|
There was a problem hiding this comment. If this is accepted, we may need to update our main collaborator guides (currently only one collaborator LGTM is required independently of the WG). |
||
| - A new API addition **should** be simultaneously implemented in at least one other VM implementation of Node. | ||
|
|
||
| - A new API **must** be considered experimental for at least one minor version release of Node before it can be considered for promotion out of experimental | ||
|
|
||
| - Experimental APIs **must** be documented as such | ||
| - Experimental APIs **must** require an explicit compile-time flag (#define) to be set to opt-in | ||
|
|
||
| - Experimental APIs **must** be considered for backport | ||
| - Experimental status exit criteria **must** involve at least the following: | ||
| - A new PR **must** be opened in nodejs/node to remove experimental status. This PR **must** be tagged as **n-api** and **semver-minor**. | ||
|
|
||
| - Exiting an API from experimental **must** be signed off by the working group. | ||
| - If a backport is merited, an API **must** have a down level implementation. | ||
| - The API **should** be used by a published real-world module. Use of the API by a real-world published module will contribute favorably to the decision to take an API out of experimental status | ||
| - The API **must** be implemented in a node implementation with an alternate VM | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Please line break at <=80 chars... makes it significantly easier to review :-)