Migrate to n-api - Githubissues

pulsar-edit / superstring

Native core components for Pulsar

MIT License

1 stars 5 forks source link

Migrate to n-api #5

Open bongnv opened 1 year ago

bongnv commented 1 year ago

This PR migrates superstring to n-api. It passed all unit tests with a few caveats:

v8::Set, v8::RegExp and node::ErrnoException aren't available in n-api so I used JsValueFromV8LocalValue to convert v8::Value to napi_value after using v8 and node API directly.
this native addon is still not thread-safe because it still still uses static variables like static Napi::FunctionReference constructor
will not work with the current tree-sitter because tree-sitter uses nan-API of superstring (ref)
noop delete was removed because it's 'noop'

mauricioszabo commented 1 year ago

So, while this took a lot longer to respond, maybe there is still some usage for this migration.

Originally, I found that it was more useful to migrate everything to the WASM version of Superstring. Unfortunately, on very big files (8k to 15k) the editor is struggling and typing lag is clearly visible and it interferes with the workflow.

So I'll be testing these changes on the PR to see if they work on newer Electron, and how much impact they do.

mauricioszabo commented 1 year ago

Quick update - it works, but not 100%.

It crashes with the error:

Cannot set property score of #<SubsequenceMatch> which has only a getter", source: /home/mauricio/projects/pulsar_repos/pulsar/node_modules/autocomplete-plus/lib/subsequence-provider.js

Basically, the Subsequence matcher is defined here on Superstring, but I remember it was some of the things that crashed the editor

mauricioszabo commented 1 year ago

@bongnv, after A LOT of testing, I have to say - GREAT JOB!

The Subsequence matcher bug happens some times (but not always - not sure why, honestly, but I feel I can work on a fix) and this fork is being used on the newer Electron. As soon as I work on a fix, I feel we can merge this!

I also saw some bugs on VIM Mode that, supposedly, start from Superstring but I need to reproduce them and be sure they are not also happening on the "stable" version of Superstring too.