deprecation warnings when used with node v6.0.0

[mdouglass]

The default use of Nan::SetPrototypeMethod is giving warnings under node 6.0. I had to change from:

Nan::SetPrototypeMethod(tpl, "flood", Flood);

to:

tpl->PrototypeTemplate()->Set(Nan::New<v8::String>("flood").ToLocalChecked(), Nan::New<v8::FunctionTemplate>(QuadTree::Flood));

in order to bypass the warning. It would be nice if SetPrototypeMethod was updated to do this internally.

[kkoopa]

The proper fix will require changing the rest of the template setters, removing functionality, which will entail backwards incompatibility, requiring a new major version. I wish they had not introduced this warning, as the functionality is still available until Node 7. I shall think of something soon enough, but will hold off until more reports have arrived about other possible issues, since one can't publish new major versions all the time.

cc @bnoordhuis

[MylesBorins]

cc @nodejs/ctc

[bnoordhuis]

The fixes in #559 and #560 are really all that's needed to address this. Instead of setting a v8::Function property (which is per-context), you now set a v8::FunctionTemplate and the v8::Function is instantiated on demand.

[kkoopa]

But Nan::SetTemplate, Nan::SetInstanceTemplate and Nan::SetPropertyTemplate all accept a v8::Data argument, which will no longer work if only v8::Primitives and v8::FunctionTemplates are allowed, since it would be possible to pass e.g. a v8::Object to any of said functions.

[kkoopa]

However, it does seem like the signature still takes a v8::Data argument as of V8 5.2.146. If that is not going to be changed, I guess the modifications need not break compatibility.

[bnoordhuis]

Well, 5.2 enforces through an assert that the value is not an object, so sooner or later people will have to update their code.

It's possible to work around that to some extent using v8::Template::SetNativeDataProperty() but it takes a v8::Value, not a v8::Data. We could perhaps add an overload.

[kkoopa]

Yes, an overload might work. That is probably the best option for keeping consistency between versions, as I don't think v8::Template::SetNativeDataProperty() is available in old versions, and thus ought not to be directly exposed through NAN. I can put something together by tomorrow and push out a new minor version of NAN with explicit Node 6 support if no other issues arise by then.

[kkoopa]

Using v8::Template::SetNativeDataProperty() is not as easy as it sounds, since it would require storing the value somewhere accessible to the getter and setter. Using internal fields for this only works for object instances. It is easier to say caveat emptor and leave it up to the user not to misuse the template setting functions if they want full compatibility. However, there will then be no way of setting arbitrary values on templates in future versions without using v8::Template::SetNativeDataProperty() directly, and that causes further problems, as it takes a v8::AccessorGetterCallback and not a Nan::AccessorGetterCallback.

[zakdances]

It's not mentioned here, so I just want to note that this issues appears to be fixed since 2.3.2.

Thanks to kkoopa for the relevant commits (here and here and elsewhere) and to everyone else reporting in.