Skip to content
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

Fix node 10 compilation error #53

Merged
merged 1 commit into from
Sep 22, 2018
Merged

Fix node 10 compilation error #53

merged 1 commit into from
Sep 22, 2018

Conversation

alexdima
Copy link
Contributor

This fixes #52 and allows for the project to be compiled with node 10.

I've used the pattern presented in https://nodejs.org/api/addons.html#addons_wrapping_c_objects i.e.

void MyObject::New(const FunctionCallbackInfo<Value>& args) {
  Isolate* isolate = args.GetIsolate();

  if (args.IsConstructCall()) {
    // Invoked as constructor: `new MyObject(...)`
    double value = args[0]->IsUndefined() ? 0 : args[0]->NumberValue();
    MyObject* obj = new MyObject(value);
    obj->Wrap(args.This());
    args.GetReturnValue().Set(args.This());
  } else {
    // Invoked as plain function `MyObject(...)`, turn into construct call.
    const int argc = 1;
    Local<Value> argv[argc] = { args[0] };
    Local<Context> context = isolate->GetCurrentContext();
    Local<Function> cons = Local<Function>::New(isolate, constructor);
    Local<Object> result =
        cons->NewInstance(context, argc, argv).ToLocalChecked();
    args.GetReturnValue().Set(result);
  }
}

@alexdima
Copy link
Contributor Author

The build has failed, but I think that is unrelated to my changes. Perhaps a flaky test?

image

@implausible implausible merged commit 622756b into Axosoft:master Sep 22, 2018
@implausible
Copy link
Contributor

Thank you so much for the help. I'm sorry I couldn't get to you sooner.

@jagu-sayan
Copy link

jagu-sayan commented Sep 25, 2018

@implausible Can you publish a new github release and npm release with all this fixes ?

I do npm install @atom/nsfw for now with node 10

@implausible
Copy link
Contributor

implausible commented Sep 25, 2018

@jagu-sayan Thank you for the reminder. I've made a 1.1.0 release.

https://github.com/Axosoft/nsfw/releases/tag/v1.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fails to build on node v10
3 participants