Nan examples fail on node 12

[josephg]

With node 12 the nan examples fail:

sephsmac:nan josephg$ node --version
v12.1.0
sephsmac:nan josephg$ pwd
/Users/josephg/3rdparty/node-addon-examples/1_hello_world/nan
sephsmac:nan josephg$ node-gyp rebuild
...
  CXX(target) Release/obj.target/hello/hello.o
../hello.cc:9:68: error: too few arguments to function call, single argument 'context' was not specified
               Nan::New<v8::FunctionTemplate>(Method)->GetFunction());
               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
/Users/josephg/.node-gyp/12.1.0/include/node/v8.h:5947:3: note: 'GetFunction' declared here
  V8_WARN_UNUSED_RESULT MaybeLocal<Function> GetFunction(
  ^
/Users/josephg/.node-gyp/12.1.0/include/node/v8config.h:347:31: note: expanded from macro 'V8_WARN_UNUSED_RESULT'
#define V8_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
                              ^
1 error generated.
make: *** [Release/obj.target/hello/hello.o] Error 1
gyp ERR! build error 
gyp ERR! stack Error: `make` failed with exit code: 2

Other examples fail in similar ways.

Updating to the latest version of nan does not fix the issue.

[mhdawson]

@nodejs/addon-api is this something you are aware of?

[josephg]

Yes, but nan also supports node 12. I got very confused trying to update some nan-related code using patterns found here, because my code wouldn’t compile. It took me awhile to figure out the official examples are wrong.

Until nan is deprecated in favor of napi / node-addon-api the examples should probably be maintained. Or not - in which case the readme should make that clear so others aren’t also confused.

[mhdawson]

@josephg How extensive are the changes? I don't think we have any active maintainers of the examples on the nan side. We'll need to find a volunteer who has time to update the docs.

[gabrielschulhof]

@mhdawson looks like V8 finally dropped the non-Maybe versions of its APIs, so we'll have to scan for instances where we're using them.

[josephg]

I ended up abandoning my nan code for node 12+ in favor of an early napi port because it was easier to port all the code to napi than figure out how to use nan correctly in all cases, across all versions of v8.

In the interim, maybe we should change the README to mention that the examples don't work on node 12+?

[mhdawson]

@josephg updating the REAMDE at least temporarily sounds good. Would you like to submit a PR for that?