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

Node 12 support #32

Closed
FibHeap opened this issue Oct 28, 2019 · 8 comments
Closed

Node 12 support #32

FibHeap opened this issue Oct 28, 2019 · 8 comments

Comments

@FibHeap
Copy link

FibHeap commented Oct 28, 2019

It seems npm install fails on node 12 (12.12.0). Handle is not defined.

> node-gyp rebuild

make: Entering directory '/home/manuel/playground/node_modules/gstreamer-superficial/build'
  CXX(target) Release/obj.target/gstreamer-superficial/gstreamer.o
In file included from ../gstreamer.cpp:4:0:
../GLibHelpers.h:9:1: error: ‘Handle’ does not name a type; did you mean ‘rand_r’?
 Handle<Object> createBuffer(char *data, int length);
 ^~~~~~
 rand_r
../GLibHelpers.h:11:1: error: ‘Handle’ does not name a type; did you mean ‘rand_r’?
 Handle<Value> gstsample_to_v8( GstSample *sample );
 ^~~~~~
 rand_r
../GLibHelpers.h:12:1: error: ‘Handle’ does not name a type; did you mean ‘rand_r’?
 Handle<Value> gstvaluearray_to_v8( const GValue *gv );
 ^~~~~~
 rand_r
../GLibHelpers.h:13:1: error: ‘Handle’ does not name a type; did you mean ‘rand_r’?
 Handle<Value> gvalue_to_v8( const GValue *gv );

Is there already a workaround?

@molecular
Copy link

molecular commented Dec 3, 2019

I second this issue (using node v13)

I doubt there's a "workaround" (less downgrading node). Code just needs to be migrated.

I tried replacing all Handle<...> with Local<...> and that got rid of the "Handle not defined" error (not sure if it is correct, though), but I'm getting more deprecation problems. There's a guide how to migrate for most of the error that pop up: bcoin-org/bcrypto#7. Tried to use that, but got stuck at some point. That stuff is just over my head at this point.

@dturing can you help?

I'm offering a bounty of 1 BCH (Bitcoin Cash) to solve this and get it merged here.

@dturing
Copy link
Owner

dturing commented Dec 3, 2019

should be fine with release 1.3.0 (commit 22834ce )

@dturing dturing closed this as completed Dec 3, 2019
@molecular
Copy link

cool!

still having issues (3 similar ones like the following):

../GObjectWrap.cpp:36:82: error: ‘class v8::Local<v8::String>’ has no member named ‘ToLocalChecked’
   36 |  constructorLocal->SetName(String::NewFromUtf8(isolate, G_OBJECT_TYPE_NAME(obj)).ToLocalChecked());

this is with node v12.11.0

@dturing dturing reopened this Dec 3, 2019
@dturing
Copy link
Owner

dturing commented Dec 3, 2019

after a9f3d7c (rel 1.3.1), works (for me) with node 12.11 and 13.2

@dturing dturing closed this as completed Dec 3, 2019
@molecular
Copy link

bounty unlocked.

@dturing give me a BCH address, please.

@dturing
Copy link
Owner

dturing commented Dec 3, 2019

via PM, and thank you!

@molecular
Copy link

molecular commented Dec 3, 2019

thank you

sent the bounty

@dturing
Copy link
Owner

dturing commented Dec 3, 2019

@molecular received, thanks again!

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

No branches or pull requests

3 participants