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

Incompatible with node v10 (NODE_MODULE_VERSION=64) #66

Open
p120ph37 opened this issue Jul 25, 2018 · 10 comments
Open

Incompatible with node v10 (NODE_MODULE_VERSION=64) #66

p120ph37 opened this issue Jul 25, 2018 · 10 comments
Labels
Request A cool feature request for the library

Comments

@p120ph37
Copy link

The API for Node 10 has changed, and the existing robot-js bindings do not compile against it correctly. Testing with Node v10.6.0 on Linux, I get this set of errors:

In file included from ../src/NodeImage.h:15:0,
                 from ../src/NodeImage.cc:14:
../src/NodeImage.cc: In static member function ‘static void ImageWrap::GetPixel(const v8::FunctionCallbackInfo<v8::Value>&)’:
../src/NodeCommon.h:68:47: error: no matching function for call to ‘v8::Function::NewInstance(int, v8::Local<v8::Value> [4])’
    (isolate, JsColor)->NewInstance (4, _jsArgs) \
                                               ^
../src/NodeCommon.h:112:54: note: in definition of macro ‘RETURN’
 #define RETURN( value ) { args.GetReturnValue().Set (value); return; }
                                                      ^
../src/NodeCommon.h:125:10: note: in expansion of macro ‘NEW_COLOR’
  RETURN (NEW_COLOR (r, g, b, a));
          ^
../src/NodeImage.cc:112:2: note: in expansion of macro ‘RETURN_COLOR’
  RETURN_COLOR (color.R, color.G,
  ^
../src/NodeCommon.h:68:47: note: candidates are:
    (isolate, JsColor)->NewInstance (4, _jsArgs) \
                                               ^
../src/NodeCommon.h:112:54: note: in definition of macro ‘RETURN’
 #define RETURN( value ) { args.GetReturnValue().Set (value); return; }
                                                      ^
../src/NodeCommon.h:125:10: note: in expansion of macro ‘NEW_COLOR’
  RETURN (NEW_COLOR (r, g, b, a));
          ^
../src/NodeImage.cc:112:2: note: in expansion of macro ‘RETURN_COLOR’
  RETURN_COLOR (color.R, color.G,
  ^
In file included from /home/travis/.node-gyp/10.6.0/include/node/node.h:63:0,
                 from /home/travis/.node-gyp/10.6.0/include/node/node_buffer.h:25,
                 from ../src/NodeCommon.h:16,
                 from ../src/NodeImage.h:15,
                 from ../src/NodeImage.cc:14:
/home/travis/.node-gyp/10.6.0/include/node/v8.h:3911:44: note: v8::MaybeLocal<v8::Object> v8::Function::NewInstance(v8::Local<v8::Context>, int, v8::Local<v8::Value>*) const
   V8_WARN_UNUSED_RESULT MaybeLocal<Object> NewInstance(
                                            ^
/home/travis/.node-gyp/10.6.0/include/node/v8.h:3911:44: note:   candidate expects 3 arguments, 2 provided
/home/travis/.node-gyp/10.6.0/include/node/v8.h:3914:44: note: v8::MaybeLocal<v8::Object> v8::Function::NewInstance(v8::Local<v8::Context>) const
   V8_WARN_UNUSED_RESULT MaybeLocal<Object> NewInstance(
                                            ^
/home/travis/.node-gyp/10.6.0/include/node/v8.h:3914:44: note:   candidate expects 1 argument, 2 provided
../src/NodeImage.cc: In static member function ‘static void ImageWrap::New(const v8::FunctionCallbackInfo<v8::Value>&)’:
../src/NodeImage.cc:206:37: error: no matching function for call to ‘v8::Function::NewInstance(int, v8::Local<v8::Value> [4])’
       _jsArgs[1] = args[1], _jsArgs));
                                     ^
../src/NodeImage.cc:206:37: note: candidates are:
In file included from /home/travis/.node-gyp/10.6.0/include/node/node.h:63:0,
                 from /home/travis/.node-gyp/10.6.0/include/node/node_buffer.h:25,
                 from ../src/NodeCommon.h:16,
                 from ../src/NodeImage.h:15,
                 from ../src/NodeImage.cc:14:
/home/travis/.node-gyp/10.6.0/include/node/v8.h:3911:44: note: v8::MaybeLocal<v8::Object> v8::Function::NewInstance(v8::Local<v8::Context>, int, v8::Local<v8::Value>*) const
   V8_WARN_UNUSED_RESULT MaybeLocal<Object> NewInstance(
                                            ^
/home/travis/.node-gyp/10.6.0/include/node/v8.h:3911:44: note:   candidate expects 3 arguments, 2 provided
/home/travis/.node-gyp/10.6.0/include/node/v8.h:3914:44: note: v8::MaybeLocal<v8::Object> v8::Function::NewInstance(v8::Local<v8::Context>) const
   V8_WARN_UNUSED_RESULT MaybeLocal<Object> NewInstance(
                                            ^
/home/travis/.node-gyp/10.6.0/include/node/v8.h:3914:44: note:   candidate expects 1 argument, 2 provided
In file included from ../src/NodeImage.h:15:0,
                 from ../src/NodeImage.cc:14:
../src/NodeImage.cc:223:35: error: no matching function for call to ‘v8::Function::NewInstance(int, v8::Local<v8::Value> [4])’
     _jsArgs[1] = args[1], _jsArgs)));
                                   ^
../src/NodeCommon.h:112:54: note: in definition of macro ‘RETURN’
 #define RETURN( value ) { args.GetReturnValue().Set (value); return; }
                                                      ^
../src/NodeImage.cc:223:35: note: candidates are:
     _jsArgs[1] = args[1], _jsArgs)));
                                   ^
../src/NodeCommon.h:112:54: note: in definition of macro ‘RETURN’
 #define RETURN( value ) { args.GetReturnValue().Set (value); return; }
                                                      ^
In file included from /home/travis/.node-gyp/10.6.0/include/node/node.h:63:0,
                 from /home/travis/.node-gyp/10.6.0/include/node/node_buffer.h:25,
                 from ../src/NodeCommon.h:16,
                 from ../src/NodeImage.h:15,
                 from ../src/NodeImage.cc:14:
/home/travis/.node-gyp/10.6.0/include/node/v8.h:3911:44: note: v8::MaybeLocal<v8::Object> v8::Function::NewInstance(v8::Local<v8::Context>, int, v8::Local<v8::Value>*) const
   V8_WARN_UNUSED_RESULT MaybeLocal<Object> NewInstance(
                                            ^
/home/travis/.node-gyp/10.6.0/include/node/v8.h:3911:44: note:   candidate expects 3 arguments, 2 provided
/home/travis/.node-gyp/10.6.0/include/node/v8.h:3914:44: note: v8::MaybeLocal<v8::Object> v8::Function::NewInstance(v8::Local<v8::Context>) const
   V8_WARN_UNUSED_RESULT MaybeLocal<Object> NewInstance(
                                            ^
/home/travis/.node-gyp/10.6.0/include/node/v8.h:3914:44: note:   candidate expects 1 argument, 2 provided

In the long-term, I'm sure we will want to rework this module to use NAN or N-API, but in the short-term, I wonder if there is a way to adjust the macros so as to support ABI 64...

@p120ph37
Copy link
Author

p120ph37 commented Jul 25, 2018

Other projects have seen the same issue (and provide clues for fixing it):
phusion/node-sha3#33
Wulf/nodehun#63

Explanation of issue on SO:
https://stackoverflow.com/questions/45388032/how-to-silence-newinstance-is-deprecated-warning-in-node-gyp-rebuild-what

@dkrutsko
Copy link
Member

Thanks for this, I'll take a look!

@dkrutsko dkrutsko added the Request A cool feature request for the library label Jul 25, 2018
@p120ph37
Copy link
Author

I may actually have a fix ready for you already - I'll open a PR as soon as I've run it through CI to be sure it still compiles on ancient Node versions...

@dkrutsko
Copy link
Member

Sounds good, thanks.

@p120ph37
Copy link
Author

Okay - looks clean all the way back to Node v0.12, so I'll open the PR now.

@dkrutsko
Copy link
Member

Looks great, I'll prep for a new release today.

@p120ph37
Copy link
Author

I'm also working on updating my node-gyp-pre fork to the latest. I happen to be looking at all of this right now because I needed support for Chrome >=66, by way of Electron 3.0.0, which uses Node 10 headers, so I figured I'd just update the whole upstream chain while I was fixing things.

@srolel
Copy link

srolel commented Aug 28, 2018

Is this going to be released? Thanks :)

@s1hofmann
Copy link

Is there any update on upcoming releases?

@catrielmuller
Copy link

@s1hofmann The fix it's merged in the Dev Branch. You can install the lib from GitHub using this command:

npm install --save Robot/robot-js#dev

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Request A cool feature request for the library
Development

No branches or pull requests

5 participants