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

Proper handling of callbacks with Nan #5

Closed
mapsam opened this issue Jun 17, 2016 · 2 comments
Closed

Proper handling of callbacks with Nan #5

mapsam opened this issue Jun 17, 2016 · 2 comments

Comments

@mapsam
Copy link
Contributor

mapsam commented Jun 17, 2016

Diving into #4 led me to checking our usage of Nan::MakeCallback, which led me to a few more tickets out in the wild about how it might not be the best thing in the world. Can we improve on our current usage?

refs:

cc @springmeyer @camilleanne @GretaCB

@GretaCB
Copy link
Contributor

GretaCB commented Oct 10, 2016

How can we consistify?

  • We might be using different versions of Nan across modules
  • We might be handling callback differently across modules
  • Handle callbacks instead of throwing (node-mapnik)
  • Return the error in the callback when doing validation whenever possible (node-mapnik)

@springmeyer
Copy link
Contributor

I re-read the original post linked above (nodejs/nan#284) and my conclusion is that we don't need to make any changes. The issue raised at nodejs/nan#284 is solvable by new APIs in nan to call callbacks directly, but does not apply to our usecase. The post says:

This form (MakeCallback) is intended only for use by C++ code that was dispatched from the libuv event loop, and not for C++ code that was invoked from JS code.

In the case of node-cpp-skel we are only doing the former: calling a callback after returning from the libuv event loop.

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