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

renaming n-api to something more appropriate and descriptive #420

Closed
jasnell opened this issue Dec 31, 2020 · 26 comments
Closed

renaming n-api to something more appropriate and descriptive #420

jasnell opened this issue Dec 31, 2020 · 26 comments
Milestone

Comments

@jasnell
Copy link
Member

jasnell commented Dec 31, 2020

N-API is a poorly selected name for the ABI-stable API for native addons. While the intent is for the name to be pronounced "En-Ay-Pee-Eye", it is most often colloquially reduced to "nappy", which -- if you're unfamiliar -- is term often used in English as a derogatory way describing the hair of black people. Aside from that, it's just not actually descriptive of what the API is for.

I'd like to propose "Stable ABI" (or S-ABI for short) as an alternative. (@addaleax has suggested C-API as a viable alternative also... based on the fact that, well, "that's exactly what it is" 🤣... that would work also)

I can open a PR to update all of the references in project but wanted to determine the support. I'd rather not spend a ton of time bike-shedding alternative names.

There's been some adoption of the API in other projects so the old name certainly won't disappear entirely overnight, but renaming it here would be a great step in the right direction.

/cc @addaleax @nodejs/tsc

@jasnell
Copy link
Member Author

jasnell commented Dec 31, 2020

Let's make it a vote with emoji. 🎉 = S-ABI 🚀 = C-API 👎🏻 = status quo

@Trott
Copy link
Member

Trott commented Dec 31, 2020

@nodejs/n-api

@mhdawson
Copy link
Member

mhdawson commented Jan 4, 2021

Will put on agenda of the N-API team for this Friday.

@mhdawson
Copy link
Member

mhdawson commented Jan 4, 2021

@jasnell, the files in src are node_api.h, node_api_types.h and despite being a bit longer it would likely be less confusing/more consistent to rename doc references to node_api or node-api than using C-API or S-API.

@gabrielschulhof
Copy link
Collaborator

@mhdawson we have js_native_api.h for non-Node.js-specific APIs, so we could diverge this further into node_api_* and js_api_*.

@mhdawson
Copy link
Member

mhdawson commented Jan 5, 2021

@gabrielschulhof I'm not quite sure I follow. The discussion is around replacing N-API in all of the docs with something else. I'm suggesting we use NODE-API to replace N-API versus the other suggestions based on the names of the include files.

@gengjiawen
Copy link
Member

n-api has been Node.js brand for a long time. Search engine, books, library author used it everywhere. It can have two names, but rename it will cost too much for community.
image

@mhdawson
Copy link
Member

mhdawson commented Jan 8, 2021

From the discussion in the team meeting,

  1. @jasnell were you suggesting that it would change all of the API call names (for example napi_create_buffer) as due to the ABI stability promises we cannot remove the existing symbols, they would have to be duplicated. Adding duplicate symbols could double the code space (if we inline common implementation) or slow things down requiring and additional call for every method. The other complication is that methods that used the new symbols would not run on older Node.js versions. This would mean that the new aliases would all have to be added as part of a new node api version.

  2. in terms of naming from the discussion if there is a rename we'd prefer node_api overall, and then for the new method names we would use node_api_xxxx for node specific methods and js_api_xxx for the non-node specific methods. That would also avoid duplicates of the existing include files (src/node_api.h, src/node_api_types.h)

  3. The team does share the concern about the issue related breaking existing content, blog posts etc. We will continue the discussion in the next meeting based on your answer to question 1).

@jasnell
Copy link
Member Author

jasnell commented Jan 8, 2021

No, not suggesting we rename or duplicate symbols, just do better as we go for ward from here. node_api works good as a name.

@mhdawson
Copy link
Member

mhdawson commented Jan 8, 2021

@jasnell it sounds like what we might be able to do is:

  • Make sure that when we talk about the API going forward we use NODE-API instead of N-API, do a careful pass through docs and use that where it won't cause broken links for existing references etc.
  • Leave all methods, symbols etc as they are, while the goal of the steps from more consistently referring to the API as NODE-API leading to people thinking node api when they see napi or n-api.

@jasnell would that align with how you were thinking we'd address the concern?

@addaleax
Copy link
Member

addaleax commented Jan 8, 2021

were you suggesting that it would change all of the API call names (for example napi_create_buffer) as due to the ABI stability promises we cannot remove the existing symbols, they would have to be duplicated. Adding duplicate symbols could double the code space (if we inline common implementation) or slow things down requiring and additional call for every method. The other complication is that methods that used the new symbols would not run on older Node.js versions. This would mean that the new aliases would all have to be added as part of a new node api version.

Fwiw, these are all problems that can be easily solved by using #define.

@mhdawson
Copy link
Member

mhdawson commented Jan 8, 2021

@addaleax using define was something that we discussed, from our understanding it can solve the names in the include files etc. but still leaves the symbols with the original names as what is exported by node. It would be another possibility for "partial" replacement but we wanted to understand what/how far @jasnell was suggesting we go before considering the different partial/full steps.

@jasnell
Copy link
Member Author

jasnell commented Jan 11, 2021

@mhdawson ... Yes, that sounds good. Using the #define approach for aliases would be ideal, I think.

@jschlight
Copy link
Collaborator

@mhdawson Since the N-API team has decided to move forward with this initiative, does it make sense to create a new issue on https://github.com/nodejs/abi-stable-node and organize our work there instead of continuing this issue? We could just rename #417 and pickup from there.

@mhdawson
Copy link
Member

If @jasnell is ok with it I can just move this issue over to the abi-stable-node repo ?

@jasnell
Copy link
Member Author

jasnell commented Jan 15, 2021

Go for it

@mhdawson mhdawson transferred this issue from nodejs/node Jan 18, 2021
@mhdawson mhdawson added this to the Milestone 11 milestone Jan 18, 2021
@jschlight
Copy link
Collaborator

jschlight commented Jan 19, 2021

As we are moving forward with this initiative, I thought I'd collect some ideas for further discussion and planning by the N-API team and others. Here are some aspects we need to consider:

  • Have we decided on NODE-API as the new name? - YES
  • What is our pattern for naming new symbols? - use node_api_ instead of napi_ to prefix new symbols
  • What is our approach for existing symbols? - leave as is
  • What locations of the Node.js core documentation need to be updated? - update wherever possible
  • What locations of ancillary repositories like node-addon-api need to be updated? - the others repos this team manages (node-addon-api, node-addon-examples, new headers repo)
  • What related projects such as node-gyp, node-pre-gyp, etc. need to be updated? - Jim will add comment on this issue were docs may need to be updated.
  • How will these updates be accomplished? - We can PR them?
  • What sort of outreach to the wider Node community do we need to make and what is our message?
    • plan is to frame as "clarification of what was already the name", hopefully does not need much messaging/clarification.

gabrielschulhof pushed a commit to gabrielschulhof/core-validate-commit that referenced this issue Feb 3, 2021
`n-api` shall no longer be accepted as a subsystem. Instead, the
accepted name for it shall be `node-api`.

Refs: nodejs/abi-stable-node#420
@gabrielschulhof
Copy link
Collaborator

gabrielschulhof commented Feb 4, 2021

At today's TSC meeting we concluded that, for the time being, we will replace documentation references to N-API with Node-API, stopping short of providing C preprocessor aliases for our symbols and public macros. We may make deeper changes if, after these changes, we feel that further changes are warranted.

@jschlight
Copy link
Collaborator

jschlight commented Feb 5, 2021

These are projects that facilitate the building of Node-API modules. Some of them include documentation that refers to N-API.

Docs

Related Node-API Build Tools

Since I've contributed to most of the projects that need documentation updates, I will go ahead and create the necessary PRs.

richardlau pushed a commit to nodejs/core-validate-commit that referenced this issue Feb 8, 2021
`n-api` shall no longer be accepted as a subsystem. Instead, the
accepted name for it shall be `node-api`.

Refs: nodejs/abi-stable-node#420
gabrielschulhof pushed a commit to nodejs/node that referenced this issue Feb 8, 2021
Refs: nodejs/abi-stable-node#420
PR-URL: #37259
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
@NickNaso
Copy link
Member

Should we change the blog posts we wrote for the Node.js Collections?

@mhdawson
Copy link
Member

@NickNaso I think we are probably ok leaving those as is.

@jschlight
Copy link
Collaborator

One last thing I see that should be fixed is the GitHub About box for https://github.com/nodejs/node-addon-api

@mhdawson
Copy link
Member

@jschlight - DONE

@mhdawson
Copy link
Member

Looks like we are done based on update from @jschlight in last Node-API team meeting, closing.

targos pushed a commit to nodejs/node that referenced this issue May 1, 2021
Refs: nodejs/abi-stable-node#420
PR-URL: #37259
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
@StephenLynx

This comment has been minimized.

@FrankFang

This comment has been minimized.

@nodejs nodejs locked and limited conversation to collaborators May 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants