-
Notifications
You must be signed in to change notification settings - Fork 41
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
v8/node deprecated APIs and how to handle them #7
Comments
It looks like nan has upgraded for this, but only for strings: nodejs/nan@24a22c3 Corresponding PR: nodejs/nan#808 Nevermind, looks like it's still in the works: nodejs/nan#811 |
Finally fixed with 3a3bb28 |
This issue seems to be getting a lot of attention from people in the community. Please note that For example: Nan::To<int32_t>(info[0]).FromJust() Instead of: info[0]->Int32Value(Nan::GetCurrentContext()).FromJust(); |
Hi all, In Node 8, I had: v8::String::Utf8Value textV8(args[4]->ToString());
std::string text(*textV8); But in Node 12, with: v8::Local<v8::String> textV8(args[4]->ToString(context).FromMaybe(v8::Local<v8::String>())); I don't figure out how concert in std::string Edit: v8::Local<v8::String> textV8(args[4]->ToString(context).FromMaybe(v8::Local<v8::String>()));
Nan::Utf8String textNan(textV8);
std::string text(*textNan); |
In node 12, a load of deprecated v8 APIs were removed: bcoin-org/bcrypto#7
In node 12, a load of deprecated v8 APIs were removed: bcoin-org/bcrypto#7
Stash attempts to fix node-osmium so that it compiles on node 12 with v8 deprecatons before I got bored and decided to use 'n' to downgrade node for this one bit of code I wanted to run instead. Ever feel you start with somethihng simple and end up six problems deep? No idea whether any of this actually works. I was stabbing at fixing compiler errors before making it logically work before I gave up. I was having particular trouble with fixing GetFunction (see https://stackoverflow.com/questions/61946241) Useful info: - https://github.com/joyeecheung/node/blob/v8-maybe-doc/CPP_STYLE_GUIDE.md#use-maybe-version-of-v8-apis - https://nodesource.com/blog/cpp-addons-for-nodejs-v4 - bcoin-org/bcrypto#7 (Vaguely) relevant issues: - osmcode#108
marked |
Several API calls from V8 were deprecated in version 7.0 (that ships w/ node 12), this commit replaces then Refs: nodejs/node#23122 nodejs/node#23159 bcoin-org/bcrypto#7
Several API calls from V8 were deprecated in version 7.0 (that ships w/ node 12), this commit replaces then Refs: nodejs/node#23122 nodejs/node#23159 bcoin-org/bcrypto#7
Several API calls from V8 were deprecated in version 7.0 (that ships w/ node 12), this commit replaces then Refs: nodejs/node#23122 nodejs/node#23159 bcoin-org/bcrypto#7
Several API calls from V8 were deprecated in version 7.0 (that ships w/ node 12), this commit replaces then Refs: nodejs/node#23122 nodejs/node#23159 bcoin-org/bcrypto#7
Putting this here to document for my own sanity.
V8 removed a bunch of calls and the node.js devs have decided to maintain them for the time being, but have marked them as deprecated. We need to switch away from them.
Deprecated APIs:
Uint32Value()
Int32Value()
IntegerValue()
NumberValue()
BooleanValue()
ToString()
Example migration:
Originally:
Now must be:
Or:
Or:
Int32Value
returnsMaybe<uint32_t>
.ToInt32
returnsMaybeLocal<Int32>
.Note that
ToChecked()
is an alias forFromJust()
.ToString()
was also deprecated, but nan.h should be handling this. Not us, but for completeness, heres the migration.From:
To:
Note that anything that returns
MaybeLocal
instead ofMaybe
should have anIsEmpty()
check.Example (from the style guide below):
References:
https://v8docs.nodesource.com/node-10.6/dc/d0a/classv8_1_1_value.html
https://v8docs.nodesource.com/node-10.6/de/da6/classv8_1_1_int32.html
https://v8docs.nodesource.com/node-10.6/d9/d4b/classv8_1_1_maybe.html
https://v8docs.nodesource.com/node-10.6/d8/d7d/classv8_1_1_maybe_local.html
https://github.com/addaleax/node/blob/78b888c6b1ca11e578be007a79476a792f726532/deps/v8/include/v8.h
https://github.com/nodejs/node/blob/master/deps/v8/include/v8.h
Timeline:
APIs removed in V8 7.0 and native addons nodejs/node#23122
[v10.x] deps: increase V8 deprecation levels nodejs/node#23159
Style Guide (mentioning the changes):
I'm not sure how this will impact node versions prior to node v10. This maybe needs to be added as a part of nan? Not sure. Going to do more research.
Update:
Just discovered the magic of
Nan::To
(1, 2, 3).We can do
Nan::To<int32_t>(info[0]).FromJust()
.Nan::To
seems undocumented, but it seems to be the inverse ofNan::New
.The text was updated successfully, but these errors were encountered: