Migrate to N-API #2
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We can re-add later, but we're already having too much trouble with the node.js version only, I don't feel it's a good idea to think about supporting YET ANOTHER version of this library...
13 tasks
|
Replaced by #5, that is a more complete version |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
So, for everyone wanting to help on this migration:
We have to uncomment everything that's on here: master...pulsar-edit:superstring:napi#diff-0d6aa7470314ca774fd7e07f67b35ecd3045fe5cb3a0e5c99f48bae897905d32
It helps quite a lot to have a LSP server running. The LSP I know that works with it uses Clang, and it expects a
compile_commands.jsonon the root of the code. To generate this just runnpm installthen run:This will generate both
Debug/compile_commands.jsonandRelease/compile_commands.json. Just select one and move to the root, likemv Release/compile_commands.json ....Wrappedclasses can receive arbitrary data. This basically do not work with Napi, so we need to generate aNapi::External<arbitrary-data>and pass this to the contructor. There's an example onPointWrapperclass: master...pulsar-edit:superstring:napi#diff-ca393bd63cb0ed098a1eee4542120db434961ff60b65c22b2476c15eb954451aR52-R54Other trick is that Nan have lots of
Unwrap()andWrap(). These are not necessary on Napi (Napi is saner in this sense)nop. This is No-op, so I don't see any reason to keep them. I'm removing it completely, just so people know :)