migrate keytar to use n-api #268
Conversation
|
This has been published to npm as |
| Napi::Value DeletePassword(const Napi::CallbackInfo& info) { | ||
| Napi::Env env = info.Env(); | ||
| if (!info[0].IsString()) { | ||
| Napi::TypeError::New(env, "Parameter 'service' must be a string"); |
There was a problem hiding this comment.
Is ThrowAsJavaScriptException missing here?
There was a problem hiding this comment.
I think it makes sense to extract a method like that:
void ThrowTypeError(napi_env env, const char* message) {
Napi::TypeError::New(env, message).ThrowAsJavaScriptException();
}
There was a problem hiding this comment.
@vadim-termius, you are right -- I missed this in the conversion. #270 will resolve this.
Just saw the 6th release and remembered my latest research on this topic. According to the documentation there is no guarantee in ABI stability if the module uses API from |
|
@vadim-termius nice spot - I couldn't spot anything obviously dependent on |
This PR migrates keytar from nan to N-API via the node-addon-api module.
Migrating to N-API provides ABI stability meaning that versions of keytar compiled for one major version can run on later major versions of Node.js without recompilation. This means that issues like #174 would be non-issues in the future.