bartosz-antosik / vscode-spellright

Multilingual, Offline and Lightweight Spellchecker for Visual Studio Code
Other
359 stars 37 forks source link

Use NAPI for node-spellchecker #214

Open rebornix opened 5 years ago

rebornix commented 5 years ago

It has been a while since our last discussion of spell checker in VSCode https://github.com/Microsoft/vscode/issues/20266 but this extension solves most of my problem, great work and thanks!

Nodejs 10 ships with an ABI stable native API called NAPI, which can be used to build a native module that runs across different nodejs versions. I ported node-spellchecker to NAPI and found it work (maybe not perfectly I didn't have a good coverage) on 8 to 10. You may want to take a look https://github.com/rebornix/napi-spellchecker and probably depend on this to avoid version mismatch issue next time we update Electron.

Once we update VSCode's Electron dependency to 3.0, the NAPI is stable and mature enough for use. When using this module in current VSCoode Insiders, it still works but we can see warnings about the NAPI is still in experimental state, so if it doesn't work as expected, this might be possible.

bartosz-antosik commented 5 years ago

Thanks for kind words about (if I understand correctly)! Spell Right! Indeed it has been a while!

Would be great to be relieved from the need to recompile binaries each time Electron version is bumped. I have realized some ago, that something has changed about this because my extension brings binaries rebuilt for version 1.8 of Electron and they somehow get loaded and work correctly with Electron version 2+. So there is a small mystery there!

I will of course consider switching to this back-end.

Do I understand correctly that it should be done around the moment you switch to Electron 3 in insiders build?

rebornix commented 5 years ago

@bartosz-antosik based on what I test and the limited nodejs API we use, if we migrate to NAPI now, it can work immediately in current Insiders. So you may probably want to upgrade to it and test if it breaks anything right now, if you don't want to wait for our Electron 3.0 upgrade as it may take a while since we see a few rendering issues in VSCode side. But it makes sense as well if we wait for Electron 3.0 which guarantees the stability.

bpasero commented 5 years ago

@bartosz-antosik fyi we just updated our insiders release to use Electron 3.0.x. That means your extension will not work in insiders unless updated with new native modules.

bartosz-antosik commented 5 years ago

@bpasero Thanks for the update. I will have a look at this ASAP.

bartosz-antosik commented 5 years ago

@rebornix @bpasero Could anyone please tell me because it is not clear to me how to make napi-spellchecker module a dependency?

I thought through npm install but it seems the module does not exists in the repository.

rebornix commented 5 years ago

@bartosz-antosik I didn't publish that to npm ;( let me warp things up and publish 🚀

bartosz-antosik commented 5 years ago

Drop me a line here once you are done. In the meantime, as people complain that it stopped working in Insiders build, I have updated binaries for version 3.x of Electron.