Skip to content

v8::FunctionTemplate inherit to Napi #246

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

Closed
iclosure opened this issue Apr 23, 2018 · 17 comments
Closed

v8::FunctionTemplate inherit to Napi #246

iclosure opened this issue Apr 23, 2018 · 17 comments

Comments

@iclosure
Copy link

iclosure commented Apr 23, 2018

Can we implement v8::FunctionTemplate::Inherit in Napi ?

@gabrielschulhof
Copy link
Contributor

FunctionTemplate is a V8-specific concept, whereas we're trying to keep N-API VM-agnostic. The behaviour of Inherit can be achieved with existing APIs.

@NickNaso
Copy link
Member

NickNaso commented Apr 23, 2018

Hi everyone,
@gabrielschulhof you're right, but I think that this is a duplicate issue, there is a discussion about Inherit here #229

@iclosure
Copy link
Author

iclosure commented Apr 23, 2018

But the method is invalid in 'super class' when I call with javascript. How can I fix it? (When I call the super method, then print error message: "TypeError: Illegal invocation")

@gabrielschulhof
Copy link
Contributor

@iclosure can you show me the code?

@iclosure
Copy link
Author

iclosure commented Apr 23, 2018

  1. Super Init:
void BaseObject::Initialize(Napi::Env env, Napi::Object exports)
{
    Napi::HandleScope scope(env);
    std::vector<PropertyDescriptor> properties = {
        InstanceAccessor("objectType", &BaseObject::GetObjectType, nullptr),
        InstanceAccessor("id", &BaseObject::GetId, &BaseObject::SetId),
        InstanceAccessor("domain", &BaseObject::GetDomain, &BaseObject::SetDomain),
        InstanceAccessor("name", &BaseObject::GetName, &BaseObject::SetName),
        InstanceAccessor("mark", &BaseObject::GetMark, &BaseObject::SetMark),
        InstanceAccessor("desc", &BaseObject::GetDesc, &BaseObject::SetDesc),
        InstanceMethod("isPrivateMark", &BaseObject::IsPrivateMark),
        InstanceMethod("childCount", &BaseObject::ChildCount),
        InstanceMethod("objectTypeString", &BaseObject::ObjectTypeString),
        InstanceMethod("resetData", &BaseObject::ResetData),
        InstanceMethod("clearData", &BaseObject::ClearData),
        InstanceMethod("domainOfType", &BaseObject::DomainOfType)
    };
    ctor = napi_init<BaseObject>(env, exports, "Object", properties);
}
  1. Sub Class inherit:
template<typename Sub, typename Super>
inline Napi::FunctionReference napi_init(Napi::Env env, Napi::Object exports, const char *name,
                                         const std::vector<Napi::ClassPropertyDescriptor<Sub> > &properties)
{
    Napi::HandleScope scope(env);
    Napi::Function object = Sub::DefineClass(env, name, properties);
    napi_inherits(env, object, Super::ctor.Value());
    exports.Set(name, object);
    Napi::FunctionReference ctor = Napi::Persistent(object);
    ctor.SuppressDestruct();
    return ctor;
}
  1. Javascript code:
var vehicle = new proto.Vehicle();
console.log(vehicle.clearData);
vehicle.clearData();
  1. napi_inherits():
inline void napi_inherits(napi_env env, napi_value ctor, napi_value super_ctor)
{
    napi_value global, global_object, set_proto, ctor_proto_prop, super_ctor_proto_prop;
    napi_value argv[2];

    napi_get_global(env, &global);
    napi_get_named_property(env, global, "Object", &global_object);
    napi_get_named_property(env, global_object, "setPrototypeOf", &set_proto);
    napi_get_named_property(env, ctor, "prototype", &ctor_proto_prop);
    napi_get_named_property(env, super_ctor, "prototype", &super_ctor_proto_prop);

    argv[0] = ctor_proto_prop;
    argv[1] = super_ctor_proto_prop;
    napi_call_function(env, global, set_proto, 2, argv, nullptr);

    argv[0] = ctor;
    argv[1] = super_ctor;
    napi_call_function(env, global, set_proto, 2, argv, nullptr);
}

@gabrielschulhof
Copy link
Contributor

@iclosure I created an example using plain C N-API.

@iclosure
Copy link
Author

iclosure commented Apr 24, 2018

Hi @gabrielschulhof
It is helpful for static method and object's member properties to inherit! But the static or non-static member method of super-class could not be inherited by sub-class by using N-Api framework. The example as bellow:
The super-class method:
InstanceMethod("clearData", &SuperObject::ClearData)
I want to call the method "clearData" of super-class on sub-class in javascript, like this:
var obj = new addon.SubObject(); obj.clearData();
Then I got an error message: "TypeError: Illegal invocation".
How can I resolve this issue, please show me an example, thank you!!! 😊
I print all properties of SubObject:
image

Exists the "clearData" function in sub-class, but cannot be called. Because of it's a native code? Why?

@gabrielschulhof
Copy link
Contributor

@iclosure I have reproduced the problem. I'll try and find the reason.

@gabrielschulhof
Copy link
Contributor

// ES6 notation

class SuperClass {
  constructor() {
    console.log("SuperClass constructor");
  }
  superclassMethod() {
    console.log("SuperClass method");
  }
}

class SubClass extends SuperClass {
  constructor() {
    super();
    console.log("SubClass constructor");
  }
}

const x = new SubClass();
x.superclassMethod();

// Old style

const OldSuperClass = function OldSuperClass() {
  console.log("OldSuperClass constructor");
};

OldSuperClass.prototype.oldsuperclassMethod = function oldsuperclassMethod() {
  console.log("OldSuperClass method");
};

const OldSubClass = function OldSubClass() {
  OldSuperClass.apply(this, arguments);
  console.log("OldSubClass constructor");
};

Object.setPrototypeOf(OldSubClass.prototype, OldSuperClass.prototype);
Object.setPrototypeOf(OldSubClass, OldSuperClass);

const y = new OldSubClass();
y.oldsuperclassMethod();

// Native

const addon = require('./build/Release/inheritance.node');

const z = new addon.Subclass();
z.superclassMethod(); // <-- Throws illegal invocation error

@iclosure iclosure reopened this Apr 24, 2018
@gabrielschulhof
Copy link
Contributor

gabrielschulhof commented Apr 24, 2018

I think there's a bug in napi_define_class, because if I give napi_define_class zero descriptors, and I subsequently

  1. retrieve the "prototype" property of the resulting constructor,
  2. create a function that will become the instance method, and
  3. attach the function to the prototype retrieved in the first step,

then the call from JS gets routed correctly to the native method.

@gabrielschulhof
Copy link
Contributor

OK, so it's not retrieving the "prototype" property and adding methods to that that fixes the problem, but rather adding methods without a v8::Signature that fixes the problem. So, calling napi_define_properties() on the object returned by retrieving the "prototype" property instead of using napi_define_class() will result in a class that has methods that do not have a v8::Signature. Yet, as mentioned in 08a5b442e42ff67bd5666604eebe69f1b4a1c919, the v8::Signature is there for a good reason.

@gabrielschulhof
Copy link
Contributor

A link to that commit: nodejs/node@08a5b442e4

@iclosure
Copy link
Author

@gabrielschulhof Has any solution for this? :)

@gabrielschulhof
Copy link
Contributor

@iclosure there is no easy solution, because, in order to do it properly, we'd need to be able to retrieve the v8::FunctionTemplate from V8 after the fact. Now, some constructors, such as those for built-in objects may not even have a v8::FunctionTemplate.

Either way, implementing this without the support of the V8 folks would result in pretty limited support.

@gabrielschulhof
Copy link
Contributor

@iclosure with V8 as it is now, we could only support inheritance if both the parent class and the child class were created using N-API.

@iclosure
Copy link
Author

Get it,Thanks!😊

@mhdawson
Copy link
Member

@iclosure ok if we close this issue now?

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

4 participants