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

Use NAPI for node-spellchecker #214

Open
rebornix opened this issue Sep 26, 2018 · 7 comments
Open

Use NAPI for node-spellchecker #214

rebornix opened this issue Sep 26, 2018 · 7 comments

Comments

@rebornix
Copy link

rebornix commented Sep 26, 2018

It has been a while since our last discussion of spell checker in VSCode microsoft/vscode#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
Copy link
Owner

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
Copy link
Author

rebornix commented Sep 26, 2018

@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
Copy link

bpasero commented Dec 13, 2018

@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
Copy link
Owner

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

@bartosz-antosik
Copy link
Owner

@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
Copy link
Author

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

@bartosz-antosik
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants