pmed / v8pp

Bind C++ functions and classes into V8 JavaScript engine
http://pmed.github.io/v8pp/
Other
898 stars 120 forks source link

Example in 'convert.md' does not work #38

Closed jcmonnin closed 7 years ago

jcmonnin commented 7 years ago

The examples for user defined type do not work.

Problem is here:

static bool is_valid(v8::Isolate*, v8::Handle<v8::Value> value)
{
    return !value.IsEmpty() && value->IsArray() && value->Length() == 3;
}

v8::Value has no method Length() (at least in recent v8 versions).

Related to that there is also the separate pull request about typos.

pmed commented 7 years ago

Hi!

Yes, the whole project documentation, and examples in particular, are quite incomplete. I will look through this and other examples, to check they work.

jcmonnin commented 7 years ago

Thanks for fixing the examples in the documentation.

pmed commented 7 years ago

Thanks for the bug catching :-)

jcmonnin commented 7 years ago

I think there are some other typos in the README.md:

// Memory for C++ class will remain when JavaScript object is deleted.
// v8pp::no_factory avoids creating any constructor for your C++ class from
// JavaScript, useful for classes you only wish to inject.
typedef v8pp::class_<my_class> my_class_wrapper(isolate);
v8::Handle<v8::Value> val = my_class_wrapper::reference_external(&my_class::instance());
// Assuming my_class::instance() returns reference to class

Should probably be something like:

// Memory for C++ class will remain when JavaScript object is deleted.
// Useful for classes you only wish to inject.
typedef v8pp::class_<my_class> my_class_wrapper(isolate);
v8::Handle<v8::Value> val = v8pp::class_<my_class>::reference_external(&my_class::instance());
// Assuming my_class::instance() returns reference to class
pmed commented 7 years ago

Yes, it should be, thanks!