update calls to use contexts and convert Maybes into Checked values, … #3
Conversation
…for compatibility with node 12.6.0 and v8 >= 7.5.288.22
src/zlib_sync.cc
Outdated
| @@ -65,15 +65,15 @@ class ZlibSyncInflate : public ObjectWrap { | |||
|
|
|||
| static NAN_METHOD(Push) { | |||
| ZlibSyncInflate* self = ObjectWrap::Unwrap<ZlibSyncInflate>(info.This()); | |||
|
|
|||
| auto context = info.GetIsolate()->GetCurrentContext(); | |||
There was a problem hiding this comment.
Since we're using nan anyway, let's stick with Nan::GetCurrentContext() for this and the other context.
Also, Local<Context> is a bit more readable than auto.
| Local<Object> buffer = Local<Object>::Cast(info[0]); | ||
| int flush = Z_NO_FLUSH; | ||
| if(info[1]->IsBoolean()) { | ||
| if(info[1]->BooleanValue()) { | ||
| if(info[1]->BooleanValue(context).ToChecked()) { |
There was a problem hiding this comment.
Since we can use the magic of nan, Nan::To<bool>().FromJust() would be preferred. Some of these *Value() calls are deprecated anyway.
| @@ -201,7 +201,7 @@ class ZlibSyncInflate : public ObjectWrap { | |||
| Nan::SetAccessor(itpl, Nan::New<String>("result").ToLocalChecked(), GetResult); | |||
| Nan::SetAccessor(itpl, Nan::New<String>("windowBits").ToLocalChecked(), GetWindowBits); | |||
|
|
|||
| target->Set(Nan::New<String>("Inflate").ToLocalChecked(), tpl->GetFunction()); | |||
| target->Set(Nan::New<String>("Inflate").ToLocalChecked(), tpl->GetFunction(context).ToLocalChecked()); | |||
There was a problem hiding this comment.
This thing is deprecated too, could add context as first arg and a ToChecked() at the end.
|
You're welcome. Thanks for making this package! Sorry for the late reply. I have been busy working on a cancellable tasks async addon for ES2019. I will make the requested changes and push a new pull request. Is that still something you'd still like me to do? I'm only asking because I don't know if you have already made the changes yourself. Cheers |
|
Go ahead. Just commit to the branch, no need to make a new PR |
|
Though, if this is going to take another 8 days, I'll just make the changes here. |
…for compatibility with node 12.6.0 and v8 >= 7.5.288.22