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

Upgrade to nan@2.10.x - refs #125 #127

Closed springmeyer closed 6 years ago

springmeyer commented 6 years ago

This starts using Nan::AsyncResource (aka Node::AsyncResource under the hood) to enable us to upgrade to latest nan without hitting deprecation warnings (that we turn into errors in skel due to our use of -Werror).

What is Node::AsyncResource?

It looks like it is a simple C++ class that needs to be created and passed to all callbacks. It looks like it is needed to be able to support this JS-land API: https://nodejs.org/api/async_hooks.html

My understanding is that we need to create one with a unique name. The Nan wrappers make this easy: https://github.com/nodejs/nan/blob/master/doc/asyncworker.md#nanasyncworker.

For the one place what we call a callback directly (not using Nan) this PR creates a Nan::AsyncResource directly.

refs https://github.com/mapbox/mapbox-gl-native/issues/11288 refs https://github.com/mapbox/node-cpp-skel/pull/125

springmeyer commented 6 years ago

Summary is that one should use Nan::Call for sync calls, and Nan::Callback::Call for async.

from https://github.com/sass/node-sass/pull/2298 is relevant since we do both sync and async calls.