Repository navigation
Update V8 API usages for compatibility with node 12 - #72
Conversation
7e01c6d to
5d66d82
Compare
5d66d82 to
0fc4bdd
Compare
| // Nan doesn't wrap this functionality | ||
| #if NODE_MAJOR_VERSION >= 12 | ||
| Isolate::GetCurrent(), | ||
| #endif |
There was a problem hiding this comment.
In node 12, the v8::String::Write method changed to take a v8::Isolate as its first parameter. Usually we don't have to manually deal with different Node/V8 versions in our code, because Nan wraps these APIs for us. But Nan does not provide a wrapper for String::Write.
| // Nan doesn't wrap this functionality | ||
| #if NODE_MAJOR_VERSION >= 12 | ||
| Isolate::GetCurrent(), | ||
| #endif |
There was a problem hiding this comment.
Here's another case where we really want to use String::Write for performance, because it allows us to reuse the std::string that we've already allocated.
|
What is blocking this one? |
|
Anyone interested in this one? Node 12 is more and more widespread now. |
|
I think this is ready to go. Just needs a look from someone on @atom/core. |
as-cii
left a comment
as-cii
left a comment
There was a problem hiding this comment.
This looks reasonable to me, @maxbrunsfeld. 👍
I've also updated the Travis build to include Node 12, alongside versions 8 and 10.