search

nodejs / abi-stable-node

Repository used by the Node-API team to manage work related to Node-API and node-addon-api
239 stars 47 forks source link

N-API feature suggestion to consider #377

Closed mhdawson closed 4 years ago

mhdawson commented 5 years ago

https://github.com/nodejs/node/issues/28196

legendecas commented 5 years ago

Just noticed that there is one message in Contributing a new API to N-API:

Must be a necessary API and not a nice to have. Convenience APIs belong in node-addon-api.

On this consideration, how do we define an API is necessary or just one convenient API?

As for https://github.com/nodejs/node/issues/28196, setting length property to the created function object with existing APIs is possible. V8 has an API which could create a function object with length with 1 API call, yet engines like JerryScript don't have such API. Is this suggestion necessary to be implemented in N-API?

mhdawson commented 5 years ago

We should probably change that wording as we have decided that node-addon-api should mostly be a wrapper for N-API.

In any case I don't think it's easy to define a set of rules that will answer if an API is necessary or just one convenient API.

I'm on the edge with respect to #28196. I don't think we absolutely need to add it, but I also think that we might have started with a version that included the length if we had thought of it at the time. Avoiding a second call does improve performance. It is also very low risk, low overhead in terms of additional code. It is also my understanding that implementing it in all other engines will be easy, you just might not get the perf benefit.

Feel free to chime into the PR if you think there are good reasons that we should not add it.

legendecas commented 5 years ago

I have no strong opinions to #28196. I was just curious about if API evlution of minor additions to existing API like #28196 complying with current design pattern, and their impact on future N-API shapes.

mhdawson commented 5 years ago

@legendecas thanks, hopefully I've answered your question.

mhdawson commented 4 years ago

Closing since I believe this is addressed