Substitute for Inherit #229
Comments
I don’t think it’s explicitly part of the API right now, not even the C one. :/ What exactly do you need from such an API? Exact 1:1 correspondence to V8’s How far do you think you could get with classical JS inheritance, that is, extending the prototype chain and calling the superclass constructor from the subclasses?
Which project are you talking about? I’d be curious to look at it if it’s open source. :) |
|
Hy Anna, Thanks for your super fast response.
No, I do not need a 1:1 correspondence to the V8 Inherit function. As the maintainer, I would prefer if I can use
I was also thinking about creating a prototype chain but am unsure how to achieve this with the new API (that's the reason I ask). Could you help me out with a small example or some ideas?
It is open source. You can find it on GitHub. |
Prototype manipulation should be able to do that. :)
I think that’s not quite working yet. Making the necessary adjustments to the C++ wrapper should be pretty doable, but I think there’s at least one thing we can’t emulate: How V8 does signature checking for the invoked methods. When invoking a subclass method, I don’t think how we could easily tell that the I’m not sure what to do. :/ |
How would you do that? By setting the I see the pros and cons we are facing with adjusting the ObjectWrap class and I think paying the overhead in all other cases where inheritance isn't needed is not really an option. The question is, might we expose an ObjectWrapper like class especially for the inheritance use case? I'm sorry that I'm not a great help here. What are the specifics I could study to get a better overview of the topic so that I can be of more help? |
|
From nodejs/node#4179 it seems that full inheritance can be accomplished with Object.setPrototypeOf(SubClass.prototype, SuperClass.prototype);
Object.setPrototypeOf(SubClass, SuperClass);This can be done with existing N-API calls (untested, and ignoring return status): void napi_inherits(napi_env, napi_value ctor, napi_value super_ctor) {
napi_value global, global_object, set_proto, ctor_proto_prop, super_ctor_proto_prop;
napi_value args[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, NULL);
argv[0] = ctor;
argv[1] = super_ctor;
napi_call_function(env, global, set_proto, 2, argv, NULL);
} |
|
Thank you very much! |
|
Thanks. This helps to set up the prototype chain correctly. The issue that remains from my point of view is that I do not know how to call the super constructor from the child class. This is needed in my case to initialize some private fields that are then accessed in the methods. |
|
@MichaReiser In my example I simply call one binding from the other. |
|
@gabrielschulhof thanks for your example. However, I'm using the node-addon api (C++ object wrappers) and I don't believe I can call the super constructor in any way (since I cannot inherit). |
|
You could have the superclass constructor call a static function to do the
actual initialization. The subclass constructor could then call the same
static function to chain up.
|
|
I believe this is not possible when the constructor initializes private fields (these are not static). Except if you store these as JavaScript values (but then you lose the benefits of using |
|
IIRC if the static function is part of the same class then it has access to the private fields: #include <stdio.h>
class Something {
public:
Something(int x, double y) {
InitializeSomething(this, x, y);
}
void print() {
fprintf(stderr, "%d, %lf\n", x, y);
}
static void InitializeSomething(Something* self, int x, double y) {
self->x = x;
self->y = y;
}
private:
int x;
double y;
};
class SomethingSubclass: public Something {
public:
SomethingSubclass(int x, double y): Something(x, y) {}
};
int main() {
SomethingSubclass z(-1, 42.0);
z.print();
return 0;
} |
|
Any update on the discussion here? |
|
I tried to follow the instructions from @gabrielschulhof but I still believe that the proposed solution does not work with ObjectWrap and, therefore, no real substitution for the functionality of v8:Inherit and nan::ObjectWrap exist. I still try to migrate some llvm bindings for node from nan to napi and node-addons-api. I struggle to migrate the PointerType class of llvm that inherits from type. I implemented the Type (header) and PointerType (header) classes using node-addons-api without inheritance. If I use the proposed When I used nan, the So my question is: Is there a way to implement class inheritance using node-addon-api and ObjectWrap or can I not make use of ObjectWrap at all in cases where inheritance is needed? Thanks, |
How do you mean "fail"? |
|
The call throws a type error. E.g. when invoking toString on a PointerType Which makes sense since the |
|
@MichaReiser actually, the "Illegal invocation" is thrown internally by V8. The cause is indeed the fact that Unfortunately, V8 decides whether the receiver is an instance of a parent class solely based upon whether the child class was created using If we were to implement In N-API's The other alternative would be to drop the signature check in Node.js – both in @hashseed @verwaest is it possible to have V8 check the prototype chain as well as the |
|
@hashseed @verwaest I made a patch that adds a V8 cctest to illustrate the difference in the behaviour of |
|
Returning |
|
Relaying message from @verwaest since he doesn't have access to his github account right now:
TL;DR: the signature check has nothing to do with prototypal inheritance. It checks whether an object has been stamped out from a certain template, expecting a certain object layout. Changing the prototype chain does not affect this. Alternatively, you could post this question to [email protected] too. |
|
@hashseed @verwaest in that case I would say that this is a bug, because this check prevents one from implementing prototype-based inheritance. That is, given a native-backed superclass created with signature-checked prototype methods it is not possible to create a subclass of that superclass using only prototype assignment on the JavaScript side such that superclass prototype methods can be called on instances of the subclass. |
|
Its purpose is not checking prototypal inheritance though. It's purpose is to make sure the object has the expected object layout, e.g. in-object fields. |
|
@hashseed I understand that, but as a result it's preventing the correct functioning of prototypal inheritance. |
|
Talked to @verwaest offline.
|
|
@hashseed OK, I think I understand. var Sub = function RegExpSubclass(string, flags) {
RegExp.call(this, string, flags);
};
Object.setPrototypeOf(Sub.prototype, RegExp.prototype);
Object.setPrototypeOf(Sub, RegExp);
var superInst = new RegExp('\S+', 'g');
var subInst = new Sub('\S+', 'g');
console.log(JSON.stringify(superInst.exec('abc def'), null, 4));
console.log(JSON.stringify(subInst.exec('abc def'), null, 4));produces an error in the second case on both V8 and SpiderMonkey. @nodejs/n-api I guess if we want addon authors to allow subclassing of the classes they expose (by allowing them to specify that N-API forego the signature check) we need to give them a way to create prototype methods without signatures – i.e. a new variation of |
|
Note that the only proper way to subclass builtins is to actually create a
subclass:
class RegExpSubclass extends RegExp { ... }
That will actually work since the resulting instance will be a RegExp with
RegExpSubclass.prototype as __proto__.
…On Tue, Jul 31, 2018 at 7:35 PM gabrielschulhof ***@***.***> wrote:
@hashseed <https://github.com/hashseed> OK, I think I understand.
var Sub = function RegExpSubclass(string, flags) {
RegExp.call(this, string, flags);
};
Object.setPrototypeOf(Sub.prototype, RegExp.prototype);Object.setPrototypeOf(Sub, RegExp);
var superInst = new RegExp('\S+', 'g');var subInst = new Sub('\S+', 'g');
console.log(JSON.stringify(superInst.exec('abc def'), null, 4));console.log(JSON.stringify(subInst.exec('abc def'), null, 4));
produces an error in the second case on both V8 and SpiderMonkey.
@nodejs/n-api <https://github.com/orgs/nodejs/teams/n-api> I guess if we
want addon authors to allow subclassing of the classes they expose (by
allowing them to specify that N-API forego the signature check) we need to
give them a way to create prototype methods without signatures – i.e., a
new variation of napi_define_class() which does not add signature checks
to prototype methods.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#229 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AASgHn2gAlxXeg1PFDH3nuUUKucQM4H1ks5uMJVVgaJpZM4SK22O>
.
|
|
Any news on this issue? |
|
@verwaest currently V8 allows for the following two scenarios:
Neither of these scenarios works from N-API code, because the If the parent class and the child class are part of the same code base, such as in a native addon, it's easy to keep the parent and child const bindings = require('./build/Release/native_bindings');
class Superclass {
}
bindings.subclassMe(Superclass);The fundamental problem is that V8 is internally able to retrieve the parent function template when defining the subclass, but embedders have no way of retrieving the In N-API's So, is it possible to have a V8 API that requires only two |
|
@verwaest scenario 0 does work for template-backed functions. What doesn't work is my example below that (copied and corrected here): const bindings = require('./build/Release/native_bindings');
class Superclass {
}
const Subclass = bindings.subclassMe(Superclass);because in the native implementation of |
|
So you want to call FunctionTemplate::Inherit to create a new JSFunction
that's not a ES6 JavaScript subclass but results in exactly the same as
FunctionTemplate::Inherit? What's the purpose exactly? What's missing by
simply creating the function by doing extends on the function-template
backed JSFunction?
Can you draw some boxes and pointers that make it clear what JSFunctions
and what FunctionTemplates point to where, because I'm clearly not
understanding it :)
On 19 Sep 2018 18:48, "gabrielschulhof" <[email protected]> wrote:
@verwaest <https://github.com/verwaest> scenario 0 does work for
template-backed functions.
What doesn't work is my example below that (copied here):
const bindings = require('./build/Release/native_bindings');class Superclass {
}bindings.subclassMe(Superclass);
because in the native implementation of subclassMe there is no way to call
FunctionTemplate::Inherit(), because we do not have the FunctionTemplate of
the superclass.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#229 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AASgHsQnTH20-OZrr-9pvXZDiJCsUGFdks5ucnV1gaJpZM4SK22O>
.
|
|
@verwaest I would like to write the following function in C++: function subclassMe(superclass) {
class SubClass extends superclass {}
return SubClass;
}in such a way that, if I use it, I will not subsequently get However, this would mean that the C++ implementation of The JavaScript implementation of Here's a gist of what I'm trying to do: I realize that I could implement |
|
What about simply adding a C++ way to do the JS extends? You could do it by
creating such a script already, but I suppose a clear API on Function may
make sense.
…On Wed, 19 Sep 2018, 20:37 gabrielschulhof, ***@***.***> wrote:
@verwaest <https://github.com/verwaest> I would like to write the
following function in C++:
function subclassMe(superclass) {
class SubClass extends superclass {}
return SubClass;
}
in such a way that, if I use it, I will not subsequently get Illegal
Invocation exceptions when I call a superclass method on an instance of
the subclass.
However, this would mean that the C++ implementation of subclassMe would
have to have access to the FunctionTemplate of both the superclass that
was passed into it and the subclass it creates, so that it can connect the
two via FunctionTemplate::Inherit.
The JavaScript implementation of subclassMe is able to receive a
JavaScript function and correctly perform the extends. The C++
implementation can only do prototype assignment to try to mimic the
extends, but that results in illegal invocation exceptions if the
superclass has a prototype method, and that method gets called on an
instance of a subclass created by prototype assignment - which is the only
way to create subclasses in C++.
Here's a gist of what I'm trying to do:
https://gist.github.com/gabrielschulhof/377d1cf557794efc324d3c9a13c834e9
I realize that I could implement subclassMe in C++ by removing the
v8::Signature on the method, but I would like to be able to keep the
signature – after all, I'm able to keep the signature if I'm using the JS
implementation of subclassMe.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#229 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AASgHqnFr1lsks5L7q8AVlW7EJ9XXMcgks5uco75gaJpZM4SK22O>
.
|
|
@verwaest you mean, like, with const char* subclassMeJs =
"function subclassMe(superclass) {"
" class subclass extends superclass {"
" /* subclass methods would be defined in JS here */"
" };"
" return subclass;"
"}";... and ultimately ... Local<Value> subclass =
subclassMeFn->Call(context, recv, 1, &superclass).ToLocalChecked(); |
|
Perhaps the solution here lies in using another pattern for your classes. What if you create all your classes in JS land and then supplement them with native methods? |
|
Yes, something like that is what I meant. If it's performance sensitive and
is a common pattern I see no issue with having a V8 API that does exactly
that on Function either.
…On Thu, 20 Sep 2018, 03:25 Gus Caplan, ***@***.***> wrote:
Perhaps the solution here lies in using another pattern for your classes.
What if you create all your classes in JS land and then supplement them
with native methods?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#229 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AASgHshu6kdBQZ73eSizqdN3BWLboVb7ks5ucu59gaJpZM4SK22O>
.
|
|
@verwaest I can certainly implement that, but then I have to write the methods of the subclass in JavaScript. So, a V8 API on v8::Local<v8::FunctionTemplate> v8::Function::Extend(...); |
|
@verwaest I guess the parameters for the new API would be the same or almost the same as for |
|
Any updates on this? |
|
@gabrielschulhof one concern I have is that if we are asking for additional V8 functions, what is the likelyhood that other JS engines have the same functionality. We don't want to add something that precludes running on other engines... |
Add the ability to define a subclass of a JS class by generating a JS
snippet that uses the `extends` syntax and calls the native bindings
which are passed in as parameters to the generated function responsible
for defining the subclass.
An example of a generated JS snippet might be:
```js
(function(parent, ctor, getSuperParams, prop0, prop1) {
'use strict';
class NativeSubclass extends parent {
constructor() {
super(...getSuperParams.apply(null, arguments));
ctor.apply(this, arguments);
}
subMethod() {
if (!(this instanceof NativeSubclass))
throw new Error('Illegal invocation');
return prop0.apply(this, arguments);
}
chainableMethod() {
if (!(this instanceof NativeSubclass))
throw new Error('Illegal invocation');
return prop1.apply(this, arguments);
}
}
return NativeSubclass;
})
```
where `ctor`, `getSuperParams`, `prop0`, and `prop1` are native
functions supplied to `napi_define_subclass`.
Signed-off-by: Gabriel Schulhof <[email protected]>
Refs: nodejs/node-addon-api#229
|
@mhdawson given that this can be implemented without support from the engine, as nodejs/node#36148 shows, I think it would be absolutely beneficial to have engine support for extending arbitrary JS classes. @verwaest-zz @hashseed I'm not sure you are any longer involved with this subject, but hopefully you can ping the right folks to show them nodejs/node#36148 as an example of the kind of functionality we would need from V8. Basically, nodejs/node#36148 implements #229 (comment), but "a clear API on Function" would still help immensely to reduce the complexity of the implementation, not least because the extra indirection in nodejs/node#36148 reduces the performance of calling the native function even further. |
|
CC: @verwaest |
|
This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made. |
|
I've managed to make inheritance work in two steps. First, make one class Clazz {
public:
virtual ~Clazz() {}
};
class ScriptWrappable : public Napi::ObjectWrap<ScriptWrappable> {
public:
ScriptWrappable(const Napi::CallbackInfo& info)
: Napi::ObjectWrap<ScriptWrappable>(info) {
// set _instance from meta data inside info.Data
}
template <typename T, Napi::Value (T::*cb)(const Napi::CallbackInfo&)>
Napi::Value InstanceMethodCallback(const Napi::CallbackInfo& info) {
T* t = static_cast<T*>(_instance.get());
return (t->*cb)(info);
}
std::unique_ptr<Clazz> _instance;
};
class Base : public Clazz {
public:
Napi::Value Hello(const Napi::CallbackInfo& info);
};
class Sub : public Base {
public:
Napi::Value Hi(const Napi::CallbackInfo& info) {
return Napi::String::New(info.Env(), "hi");
}
static Napi::Function DefineClass(Napi::Env env) {
return ScriptWrappable::DefineClass(
env,
"Sub",
{ScriptWrappable::InstanceMethod<
&ScriptWrappable::InstanceMethodCallback<Base, &Base::Hello>>(
"hello"),
ScriptWrappable::InstanceMethod<
&ScriptWrappable::InstanceMethodCallback<Sub, &Sub::Hi>>("hi")}
// attach class meta data here
);
}
};And then, monkey patch the prototype to make Napi::Object global = env.Global();
Napi::Object Object = global.Get("Object").As<Napi::Object>();
Napi::Function setPrototypeOf =
Object.Get("setPrototypeOf").As<Napi::Function>();
Napi::Value clazz_proto = clazz.Get("prototype");
Napi::Value parent_proto = parent_clazz.Get("prototype");
setPrototypeOf.Call({clazz_proto, parent_proto});
setPrototypeOf.Call({clazz, parent_clazz});To make constructor and other things work correctly, class meta data should be attached as the data pointer of |
|
@ajihyf The example C++ code in your comment cannot work well (some compilation error). |
Sorry it's a code snippet I copied from my lib to show the basic idea and I didn't check its compilation outside the lib. It should work now. |
|
@devongovett, I have settled on a method which I think is the least bad one: https://mmomtchev.medium.com/c-class-inheritance-with-node-api-and-node-addon-api-c180334d9902 |
Hy
First of all, great work! I'm migrating a Nan based native addon to node-addon-api, and so far, the code is much cleaner than before. I do no longer need to copy past fragments as I now can remember the signatures and requires far less boilerplate code.
Right now I'm migrating my LLVM Wrapper for Node.js to napi and face the issue that I don't see a way to implement a class hierarchy using
Napi::ObjectWrap. I need class hierarchies to correctly model the LLVM Value API.Inheritance with the
v8API can be achieved by using theInheritFunction of theFunctionTemplate.Is this use case yet supported or are there ideas how to implement it in node-addon-api?
Cheers,
Micha
The text was updated successfully, but these errors were encountered: