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

Support for Node 12 #849

Closed
xzyfer opened this issue Apr 24, 2019 · 21 comments
Closed

Support for Node 12 #849

xzyfer opened this issue Apr 24, 2019 · 21 comments

Comments

@xzyfer
Copy link

xzyfer commented Apr 24, 2019

Simply opening this as a tracking issue we can subscribe to and link to in upstream issues.

@Flarna
Copy link
Member

Flarna commented Apr 25, 2019

I tried with Node 12 and it seems Nan 2.13.2 works fine as is.

The missing part in this repo seems to be only adding #define NODE_12_0_MODULE_VERSION 72 and update readme,... but this shouldn't be blocking.

@matoro
Copy link

matoro commented Apr 25, 2019

I am seeing these when trying to build sqlite3. Is this nan's fault?

In file included from ../../nan/nan_new.h:189,                                                                                                  
                 from ../../nan/nan.h:203,                                                                                                                                                                                                                                                                               
                 from ../src/database.h:10,                                                                                                                                                                                                                                                                              
                 from ../src/database.cc:4:                                                                                                                                                    
../../nan/nan_implementation_12_inl.h: In static member function ‘static Nan::imp::FactoryBase<v8::Function>::return_t Nan::imp::Factory<v8::Function>::New(Nan::FunctionCallback, v8::Local<v8::Value>)’:
../../nan/nan_implementation_12_inl.h:105:32: error: no matching function for call to ‘v8::Function::New(v8::Isolate*&, void (&)(const v8::FunctionCallbackInfo<v8::Value>&), v8::Local<v8::Object>&)’
                           , obj));                                                                                                                                                                                                                                                                                      
                                ^
In file included from ../../nan/nan_new.h:189,                                                                                                                                                                                                                                                                           
                 from ../../nan/nan.h:203,                                                                                                                                                                           
                 from ../src/database.h:10,                                                                                                                                                    
                 from ../src/database.cc:4:                                                                                                                                                    
../../nan/nan_implementation_12_inl.h:337:58: error: expected primary-expression before ‘>’ token                                                                                              
   return v8::StringObject::New(value).As<v8::StringObject>();                   
                                                          ^                                                                                        
../../nan/nan_implementation_12_inl.h:337:60: error: expected primary-expression before ‘)’ token                                                                                                                                                                                                                        
   return v8::StringObject::New(value).As<v8::StringObject>();                                                                                                                                                                       
                                                            ^                                                                                                                                                       
In file included from ../src/database.h:10,                                                                                                                                                    
                 from ../src/database.cc:4:                                                                                                                                      
../../nan/nan.h: In constructor ‘Nan::Utf8String::Utf8String(v8::Local<v8::Value>)’:                                                                                                           
../../nan/nan.h:1034:53: error: no matching function for call to ‘v8::Value::ToString()’                                                                                                       
       v8::Local<v8::String> string = from->ToString();                                                                                                                                        
                                                     ^

@matoro
Copy link

matoro commented Apr 25, 2019

One more:

In file included from ../src/database.h:10,                                                                                                                                                                                                                                                                              
                 from ../src/database.cc:4:                                                                                                                                                    
../../nan/nan.h:1044:74: error: no matching function for call to ‘v8::String::WriteUtf8(char*&, int, int, const int&)’                                                                                                                                                                                                  
         length_ = string->WriteUtf8(str_, static_cast<int>(len), 0, flags);                                                                                                                                                                                                                                             
                                                                          ^

@Flarna
Copy link
Member

Flarna commented Apr 25, 2019

At least for the second one it seems like sqlite is directly using v8 API string->WriteUtf8() which has been removed.

@matoro
Copy link

matoro commented Apr 25, 2019

@Flarna I don't think so. Am I reading this right?

#if NODE_MAJOR_VERSION >= 11
length_ = string->WriteUtf8(v8::Isolate::GetCurrent(), str_, static_cast<int>(len), 0, flags);

from

nan/nan.h

Line 1080 in 3bd8b1b

#if NODE_MAJOR_VERSION >= 11

This is being executed for versions 11 and up?

@Flarna
Copy link
Member

Flarna commented Apr 25, 2019

@matoro Which version of sqlite3 are you using? Is the latest version of NAN in use?
For Node.JS >= 11 the WriteUtf8(v8::Isolate::GetCurrent(),..) should be used but in our error message above I can see that the other one without v8::Isolate as first arg is used. Could be related to use of an outdated NAN version.

@Flarna
Copy link
Member

Flarna commented Apr 25, 2019

I just cloned https://github.com/mapbox/node-sqlite3 and built locally (windows) with success.

There are quite some deprecation warnings because sqlite uses Nan::ForceSet() and Nan::MakeCallback() without AsyncResource but no build errors like above.

@matoro
Copy link

matoro commented Apr 25, 2019

I have patched the package I am building to bump to the latest version of sqlite3 (4.0.6) which depends on:

"dependencies": {
    "nan": "^2.12.1",
    "node-pre-gyp": "^0.11.0",
    "request": "^2.87.0"
},

Edit: I also had to bump natives to 1.1.6 which is an indirect dependency. Let me see if bumping nan to the latest fixes it.

Edit 2: scratch that, that's from master. sqlite3 release 4.0.6 depends on nan ~2.10.0.

@Flarna
Copy link
Member

Flarna commented Apr 25, 2019

ok, if I use 4.0.6 instead maser of sqlite3 I can see the same errors. Most likely TryGhost/node-sqlite3#1093 fixed the issues but no release was made yet.
Maybe you have to wait on TryGhost/node-sqlite3#1150

@junderw
Copy link

junderw commented Apr 26, 2019

tiny-secp256k1 maintainer here, here are the errors I found in NaN when compiling on v12.

In file included from ../../nan/nan_new.h:189:0,
                 from ../../nan/nan.h:223,
                 from ../native/addon.cpp:4:
../../nan/nan_implementation_12_inl.h: In static member function ‘static Nan::imp::FactoryBase<v8::StringObject>::return_t Nan::imp::Factory<v8::StringObject>::New(v8::Local<v8::String>)’:
../../nan/nan_implementation_12_inl.h:356:37: error: no matching function for call to ‘v8::StringObject::New(v8::Local<v8::String>&)’
   return v8::StringObject::New(value).As<v8::StringObject>();

../../nan/nan_implementation_12_inl.h:356:58: error: expected primary-expression before ‘>’ token
   return v8::StringObject::New(value).As<v8::StringObject>();
                                                          ^
../../nan/nan_implementation_12_inl.h:356:60: error: expected primary-expression before ‘)’ token
   return v8::StringObject::New(value).As<v8::StringObject>();

../../nan/nan_object_wrap.h: In destructor ‘virtual Nan::ObjectWrap::~ObjectWrap()’:
../../nan/nan_object_wrap.h:24:25: error: ‘class Nan::Persistent<v8::Object>’ has no member named ‘IsNearDeath’
     assert(persistent().IsNearDeath());
                         ^
../../nan/nan_object_wrap.h: In static member function ‘static void Nan::ObjectWrap::WeakCallback(const v8::WeakCallbackInfo<Nan::ObjectWrap>&)’:
../../nan/nan_object_wrap.h:127:26: error: ‘class Nan::Persistent<v8::Object>’ has no member named ‘IsNearDeath’
     assert(wrap->handle_.IsNearDeath());

@xzyfer
Copy link
Author

xzyfer commented Apr 26, 2019 via email

@lrecknagel
Copy link

I have a similar issue as @matoro with node-rdkafka:

Log:

...
../src/common.cc: In function ‘T NodeKafka::GetParameter(v8::Local<v8::Object>, std::__cxx11::string, T) [with T = std::__cxx11::basic_string<char>; std::__cxx11::string = std::__cxx11::basic_string<char>]’:
../src/common.cc:94:55: error: no matching function for call to ‘v8::Value::ToString()’
       v8::Local<v8::String> val = parameter->ToString();
...

Code:

...
v8::Local<v8::Value> parameter = Nan::Get(object, field).ToLocalChecked();
...
v8::Local<v8::String> val = parameter->ToString();

Any ideas on that case ?

@Flarna
Copy link
Member

Flarna commented Apr 26, 2019

@lrecknagel Don't use v8 directly, use NAN or napi.
v8::Local<v8::String> val = parameter->ToString(); => Nan::To<v8::String>(parameter) should work.

@darkgl0w
Copy link

darkgl0w commented May 10, 2019

Hi,

EDIT: narrowed down this issue to outdated dependencies and possible flaky tests.
I made all nan relevant changes thanks to Flarna 👍 .

@ccope
Copy link

ccope commented May 10, 2019

@darkgl0w Warnings are not errors, a memory dump might be required to diagnose the segfault

@Flarna
Copy link
Member

Flarna commented May 10, 2019

Nan::MakeCallback() is deprecated since NodeJs 10. Your Node 10 build shows the same warnings.
see https://github.com/nodejs/nan/blob/master/doc/node_misc.md#api_nan_make_callback for more details.

gauravtiwari pushed a commit to rails/webpacker that referenced this issue May 22, 2019
* Temporarily fix Travis CI builds

`nvm install node` will install the latest node version, which is 12 as
of writing. Because of a problem in the resolved node-sass version this
makes the builds fail.

To get green builds until a proper fix for node 12 is introduced we'll
install node 10, which is a LTS release, while 11 will be EOL in June
2019.

@rokumatsumoto is maybe working on a fix, but I think green builds,
especially for third party PRs are an important intermediate step.

#2077
sass/node-sass#2633
nodejs/nan#849
https://github.com/nodejs/Release

* Lock RuboCop to a version that supports Ruby 2.2

RuboCop 0.69.0 dropped support for Ruby 2.2, making RuboCop fail when it
was installed since it pointed to the git repo instead of a specific
version.

https://github.com/rubocop-hq/rubocop/blob/v0.69.0/CHANGELOG.md#changes

* Allow failures with ruby-head

These might – currently – not be related to Ruby itself, but to the
fact that bundler 2.1.0.pre.1 was installed, which is as of writing
not supported by webpacker (`~> 1.12` is used).

But this is to get the CI builds green again.
@bnoordhuis
Copy link
Member

I'm going to close this out because I have no reason to believe at this point that the most recent release doesn't build with Node.js v12.x. The linked issues are either about older nan releases or caused by using Node or V8 APIs directly.

Case in point: @imrehg, that's not nan@2.13.2, going by the line numbers.

@ralyodio
Copy link

ralyodio commented Jun 6, 2019

../../nan/nan_object_wrap.h:127:26: error: ‘class Nan::Persistentv8::Object’ has no member named ‘IsNearDeath’

@kkoopa
Copy link
Collaborator

kkoopa commented Jun 6, 2019

fbaf422

pdufour pushed a commit to lob/pdffonts that referenced this issue Dec 6, 2019
pdufour added a commit to lob/pdffonts that referenced this issue Dec 9, 2019
* Add support for node 12

See mapbox/node-cpp-skel#54
nodejs/nan#849

* updates

* Only need this

* Use compatible versions

* use dockerfile

* updates

* caching

* Run commands from docker

* Fix cov report

* Fixes

* lowercase

* re-add cov check

* change back to yarn run
KingTiger001 added a commit to KingTiger001/Rails-web-pack-project that referenced this issue Jan 15, 2023
* Temporarily fix Travis CI builds

`nvm install node` will install the latest node version, which is 12 as
of writing. Because of a problem in the resolved node-sass version this
makes the builds fail.

To get green builds until a proper fix for node 12 is introduced we'll
install node 10, which is a LTS release, while 11 will be EOL in June
2019.

@rokumatsumoto is maybe working on a fix, but I think green builds,
especially for third party PRs are an important intermediate step.

rails/webpacker#2077
sass/node-sass#2633
nodejs/nan#849
https://github.com/nodejs/Release

* Lock RuboCop to a version that supports Ruby 2.2

RuboCop 0.69.0 dropped support for Ruby 2.2, making RuboCop fail when it
was installed since it pointed to the git repo instead of a specific
version.

https://github.com/rubocop-hq/rubocop/blob/v0.69.0/CHANGELOG.md#changes

* Allow failures with ruby-head

These might – currently – not be related to Ruby itself, but to the
fact that bundler 2.1.0.pre.1 was installed, which is as of writing
not supported by webpacker (`~> 1.12` is used).

But this is to get the CI builds green again.
smartech7 pushed a commit to smartech7/ruby-webpacker that referenced this issue Aug 4, 2023
* Temporarily fix Travis CI builds

`nvm install node` will install the latest node version, which is 12 as
of writing. Because of a problem in the resolved node-sass version this
makes the builds fail.

To get green builds until a proper fix for node 12 is introduced we'll
install node 10, which is a LTS release, while 11 will be EOL in June
2019.

@rokumatsumoto is maybe working on a fix, but I think green builds,
especially for third party PRs are an important intermediate step.

rails/webpacker#2077
sass/node-sass#2633
nodejs/nan#849
https://github.com/nodejs/Release

* Lock RuboCop to a version that supports Ruby 2.2

RuboCop 0.69.0 dropped support for Ruby 2.2, making RuboCop fail when it
was installed since it pointed to the git repo instead of a specific
version.

https://github.com/rubocop-hq/rubocop/blob/v0.69.0/CHANGELOG.md#changes

* Allow failures with ruby-head

These might – currently – not be related to Ruby itself, but to the
fact that bundler 2.1.0.pre.1 was installed, which is as of writing
not supported by webpacker (`~> 1.12` is used).

But this is to get the CI builds green 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