mapbox / node-cpp-skel

Skeleton for bindings to C++ libraries for Node.js using node-addon-api
Creative Commons Zero v1.0 Universal
72 stars 10 forks source link

Support node v10x #125

Closed springmeyer closed 6 years ago

springmeyer commented 6 years ago

Node v10 was released on Tuesday: https://nodejs.org/en/blog/release/v10.0.0/.

This is a big deal because node does not release even numbered major releases very often.

Supporting node v10 will require upgrading to the latest nan release.

This means we'll need to deal with new v8/nan deprecations. Otherwise the builds will fail due to node-cpp-skel's use of -Werror and the new deprecations of Nan::MakeCallback (https://github.com/nodejs/nan/blob/master/doc/node_misc.md#nanmakecallback)

The deprecation errors look like:

../src/object_async/hello_async.cpp:186:19: error: 'Call' is deprecated [-Werror,-Wdeprecated-declarations]
        callback->Call(argc, static_cast<v8::Local<v8::Value>*>(argv));

The task ahead is to:

Open question: should node-cpp-skel be testing, in its job matrix, both node v6, v8, and v10? These are all currently within the LTS window (https://github.com/nodejs/Release#release-schedule). Or should node-cpp-skel just test the latest LTS version (in this case, right now v10) to keep the matrix under control and assume that downstream users of node-cpp-skel will add support for the versions they need?

NOTE: alternatively node-cpp-skel can (and likely should soon) move to using N-API instead of nan, which would mean that we'd be able to stop building binaries for each node version as those binaries would be compatible across MAJOR versions (perhaps with a few gochas). This is tracked at https://github.com/mapbox/node-cpp-skel/issues/45.

springmeyer commented 6 years ago

new deprecations of Nan::MakeCallback

These can be fixed by following the guidelines at https://github.com/nodejs/nan/blob/master/doc/asyncworker.md#api_nan_async_queue_worker

This class internally handles the details of creating an AsyncResource, and running the callback in the correct async context. To be able to identify the async resources created by this class in async-hooks, provide a resource_name to the constructor. It is recommended that the module name be used as a prefix to the resource_name to avoid collisions in the names. For more details see AsyncResource documentation. The resource_name needs to stay valid for the lifetime of the worker instance.

springmeyer commented 6 years ago

done with #144