ObjectWrap construction flow about node-addon-api HOT 6 CLOSED

Sure, I can try to implement it.

from node-addon-api.

So the idea here is that the whatever function the original code use to initialize a new instance (in this case is the Database::New) should be the last thing get called in the ConstructoCallbackWrapper. By the time we call it, all preparation should have been done.

from node-addon-api.

To clarify this problem:
So in Database::New() it creates a new Baton, which calls ObjectWrap->Ref(). However, if we change Database::New to be Database::Database (similar to what is done in Canvas), then in the ConstructorCallbackWrapper, the napi_ref haven't be created by the time Ref() is called.

So I think we should create napi_ref before calling the object-wrapped class constructor.
@mhdawson @jasongin Please tell me if this issue is still not clear.

from node-addon-api.

This is tricky. The napi_ref is created by the napi_wrap() call, and that can't be called until the class instance is constructed (or at least allocated).

I think the fix should be to move the call to napi_wrap() and reference assignment from ObjectWrap::ConstructorCallbackWrapper() to the ObjectWrap::ObjectWrap() constructor, where it would execute after the instance is allocated but before the subclass constructor. If napi_wrap() returns an error status there, it should be thrown as an exception, which will be caught by the details::WrapCallback() wrapper in ConstructorCallbackWrapper().

@sampsongao Do you want to implement this change or should I?

from node-addon-api.

Unrelated to fixing the current issue, Sampson mentioned that because it does not support the 2 step flow used with Nan, the conversion script cannot make the substitutions automatically and it takes some work to do it manually. Would it make sense to support that flow in addition or instead to make conversion easier ?

from node-addon-api.

Commited d45ba72

from node-addon-api.