search

microsoft / node-native-keymap

Provide OS keyboard layout functionality as a nodejs module
MIT License
136 stars 37 forks source link

refactor: use nan bindings #11

Closed deepak1556 closed 5 years ago

deepak1556 commented 5 years ago

Required for https://github.com/microsoft/vscode/tree/electron-6.0.x

egamma commented 5 years ago

@deepak1556 @alexandrudima is on paternity leave, back in July. How critical is this change?

//fyi @kieferrm

deepak1556 commented 5 years ago

@egamma this change is a blocker for electron 6 upgrade, but we can work with targeting the module against the branch instead of an official npm release. 

{
  "dependencies": {
     "native-keymap": "microsoft/node-native-keymap#node_v12"
  }
}

So I wouldn't say its critical to review and merge at the moment.

egamma commented 5 years ago

@deepak1556 good to know, thanks

bpasero commented 5 years ago

Not sure, but maybe @rebornix could also review.

alexdima commented 5 years ago

@deepak1556 Thank you for your work, but I've decided to adopt N-API (https://github.com/microsoft/node-native-keymap/pull/12) which in comparison to nan, guarantees ABI stability not only source stability.