src: add Addon<T> class

[gabrielschulhof]

• separate out instance-related APIs from ObjectWrap<T> into a new
  class InstanceWrap<T> which then becomes a base class for
  ObjectWrap<T>.
• Expose new DefineProperties overload on Object which accepts a
  list of napi_property_descriptor and implement the other overloads
  as a call to it.
• Add Addon<T> class as a subclass of InstanceWrap<T>.
• Add macros NODE_API_ADDON() and NODE_API_NAMED_ADDON() to load an
  add-on from its Addon<T> subclass definition.

Bindings created like this perform slightly worse than static ones in
exchange for the benefit of having the context of a class instance as
their C++ this object. Static bindings can still be created and
associated with the exports object and they can use
Napi::Env::GetInstanceData() to retrieve the add-on instance.

Edit: I removed the DefineProperties overload, opting instead to have Addon<T> provide a DefineProperties method to its subclasses.

[gabrielschulhof]

Telling it to ignore whitespace makes reviewing easier:

[gabrielschulhof]

@mhdawson @NickNaso @addaleax @legendecas @KevinEady @jschlight as discussed in today's meeting, I re-worked the PR as follows:

• rebased onto master
• broke the changes into two commits
  1. the actual changes
  2. group the InstanceWrap<T> functions into the portion of napi-inl.h that implements InstanceWrap<T> and fix whitespace issues intentionally left in the first commit to make it easier to review

I also made the commit message more verbose:

• separate out instance-related APIs from ObjectWrap<T> into a new
  class InstanceWrap<T> which then becomes a base class for
  ObjectWrap<T>.
• Add Addon<T> class as a subclass of InstanceWrap<T>,
  reimplementing Unwrap() to retrieve the instance data using
  GetInstanceData<T>() of Napi::Env instead of napi_unwrap().
• Add macros NODE_API_ADDON() and NODE_API_NAMED_ADDON() to load an
  add-on from its Addon<T> subclass definition.

Bindings created like this perform slightly worse than static ones in
exchange for the benefit of having the context of a class instance as
their C++ this object. This way, they avoid having to call
info.GetInstanceData<ClassName>() in the bindings, which brings with
it the risk that the wrong ClassName will end up in the template
parameter thus resulting in a hard-to-track-down segfault. Static
bindings can still be created and associated with the exports object
and they can use Napi::Env::GetInstanceData() to retrieve the add-on
instance.

[mhdawson]

@gabrielschulhof thanks for refactoring. Trying hard to carve out a block of time to review. Failed today but will try again tomorrow.

[gabrielschulhof]

@mhdawson thank you!

[mhdawson]

The tests seems to compile/pass without this change. Wondering as to my its in this PR.

[gabrielschulhof]

I added it because I was working on an add-on definition macro right next to it, and we shouldn't be exposing the entry point symbol. You're right though, I'll make it a separate PR.

[mhdawson]

These changes mean we won't be able to run the benchmarks if the target is less than NAPI_VERSION 6.

I wonder if the changes to the benchmark code should be additive so that we get new numbers if the target is NAPI_VERSION 6 or greater as opposed to replacing the existing ones.

That would also let us keep an eye on the delta between using the additional wrapper or not.

[mhdawson]

Overall I really like this. I think it really helps with ease of use. Just had a few comments. Once you've updated let me know and I'll LGTM.

Totally separate but something I think this helps enable and that I think it would be really cool is that instead of having to create the code to call DefineAddon(), you could just annotate the methods and we'd generate the required call to DefineAddon(), maybe something like:

 // @function increment
 Napi::Value Increment(const Napi::CallbackInfo& info) {
    return Napi::Number::New(info.Env(), ++value);
  }

It also relates a bit to the question we had about generating documentation as well as we could then also add information
to define the parameters, and possibly even alias them to make code more readable. Maybe something like

 /**
  * @function increment
  * @param {number} increment_amount - amount to increment when the method is called
  * @returns {number} the incremented value
  */
 Napi::Value Increment(const Napi::CallbackInfo& info) {
    value = value + ${PARAM_increment_amount}.Uint32Value();
    return Napi::Number::New(info.Env(), value);
  }

Where ${PARAM_increment_amount} would be converted to info[0].AsNapi::Number() based on being the first parameter defined, and being defined as type number.

I think it would require a pre-processing step but that might be reasonable. Thinking about it, the same could be useful for object wrap as well

I think we could probably come up with annotations for the void* data and attributes although those are probably use infrequently enough that what I've outlined would be useful without them.

[gabrielschulhof]

@mhdawson I have made the changes you requested.

[mhdawson]

LGTM

[gabrielschulhof]

Rebased and squashed.

[gabrielschulhof]

CI:

Version	Job	Status
v10.x	https://ci.nodejs.org/job/node-test-node-addon-api-new/2494/	✓
v12.x	https://ci.nodejs.org/job/node-test-node-addon-api-new/2495/	✓
v14.x	https://ci.nodejs.org/job/node-test-node-addon-api-new/2496/	✓
v15.x	https://ci.nodejs.org/job/node-test-node-addon-api-new/2497/	✓

[gabrielschulhof]

Landed in 5af645f.