neon-bindings / neon

Rust bindings for writing safe and fast native Node.js modules.
https://www.neon-bindings.com/
Apache License 2.0
8k stars 283 forks source link

Move `root` method to `Value` trait #996

Open kjvalencik opened 1 year ago

kjvalencik commented 1 year ago

Currently the .root(cx) method is provided by the object trait Object. This is because Node-API requires the value be an Object.

However, this is an overly restrictive requirement in Node-API and may be relaxed in the future to allow non-Object types (e.g., string).

If we want to support root for non-object types in the future, we will want to move the method. However, moving is a breaking change. Duplicating it is also a breaking change because users may end up with an ambiguous call if both traits are in scope.

I propose moving the function to the Value trait with a T: Object bound. We can relax this bound in the future without a breaking change.

trait Value {
    fn root<'cx, C>(&self, cx: &mut C) -> Root<Self>
    where
        C: Context<'cx>,
        Self: Object + Sized,
    {
        Root::new(cx, self)
    }
}
dherman commented 1 year ago

I like it!