DelSkayn / rquickjs

High level bindings to the quickjs javascript engine
MIT License
431 stars 58 forks source link

Missing documentation around class register / define #289

Open Sytten opened 2 months ago

Sytten commented 2 months ago

I am trying to understand the difference between

Class::<MyStruct>::define(&ctx.globals()).unwrap();

And

Class::<MyStruct>::register(&ctx).unwrap()

The register is not needed to be able to do const a = new MyStruct(), but the define is? What is the purpose of register then?

Also found a small nit, probably meant to say Otherwise the class will not be available or something like it. But then again this doesn't true anymore?

https://github.com/DelSkayn/rquickjs/blob/9e8cc41f7304e1222efc3fb77a4e533bd8b2ce93/core/src/class.rs#L184

EDIT: I see that Class::<MyStruct>::instance registers the class if it not registered, so it done automatically for you when you first use the class with the macro since the IntoJs uses that method. This should likely be added to the documentation.

I would also add documentation surrounding the define in there, something akin to Note this is difference from registering the constructor globally, see XXX. And then point to a more expanded documentation in the class macro page.

Sytten commented 2 months ago

I created this small helper trait that maybe we could copy into rquickjs?

use rquickjs::{class::JsClass, Class, Ctx, Result};

pub trait ExposeJs<'js> {
    fn expose(ctx: &Ctx<'js>) -> Result<()>;
}

impl<'js, T: JsClass<'js>> ExposeJs<'js> for T {
    fn expose(ctx: &Ctx<'js>) -> Result<()> {
        Class::<T>::register(ctx)?;
        Class::<T>::define(&ctx.globals())?;
        Ok(())
    }
}
richarddd commented 2 months ago

@Sytten there is a difference between define and register. register registers the class in the runtime (so it can be accessed and interop between js/rust). define however adds the constructors (if available) and also registers the class. Your util has no effect as it can be replaced by calling define. If you need to register the constructors of the class (so you can do new MyClass) you need to use define

Sytten commented 2 months ago

Yes I agree, I just feel that the define global is what most people think of when they think register. And also I had to find about it in an issue because it is not documented.