neon-bindings / rfcs

RFCs for changes to Neon
Apache License 2.0
14 stars 9 forks source link

RFC: `ResultExt` trait #39

Closed vtenfys closed 3 years ago

vtenfys commented 3 years ago

This follows discussion in https://github.com/neon-bindings/examples/pull/67

I'm quite new to Rust so hopefully my proposal is clear and I haven't missed any obvious alternatives!

View Rendered RFC

kjvalencik commented 3 years ago

@davidbailey00 The main blocker for this RFC is that in order to support this in a very general fashion, requires Generic Associated Types or GATs, which do not exist in Rust.

We would need to choose between supporting generic Into responses on structs or boxed trait object representations (runtime cost).

This is an interesting idea, but I think it needs a proof of concept that can demonstrate we can provide this in a zero-cost, ergonomic format. I then think we would want to do a holistic update of Neon APIs to expect Into<Throw> or Into<Value> like traits.

vtenfys commented 3 years ago

The main blocker for this RFC is that in order to support this in a very general fashion, requires Generic Associated Types or GATs, which do not exist in Rust.

On that basis, should this be revisited when https://github.com/rust-lang/rust/issues/44265 lands?

kjvalencik commented 3 years ago

@davidbailey00 I would ❤️ to implement this sooner, if possible. I tried a few different hacks to work around the problem, but was unsuccessful. It would be awesome of some contributor could figure out how to solve it.

Maybe there's something we could do with proc macros to make it ergonomic without GATs? Here's my ideal API for JsFunction::new:

trait TryIntoJs {
    type Error: IntoThrow;

    fn try_into_js<'a, C: Context<'a>>(self, cx: &mut C) -> Result<Handle<'a, JsValue>, Self::Error>;
}

trait IntoThrow {
    fn try_into_js<'a, C: Context<'a>>(self, cx: &mut C) -> JsResult<Self>;
}

impl JsFunction {
    pub fn new<'a, C, F, T, E>(f: F) -> JsResult<'a, JsFunction>
    where
        C: Context<'a>,
        F: Fn(FunctionContext) -> Result<T, E>,
        T: TryIntoJs,
        E: IntoThrow,
    {
        todo!()
    }
}

As long as the function returns something that we can always turn into a JsResult, it should be accepted. We can provide noop implementations of TryIntoJs for all of the Js types.

kjvalencik commented 3 years ago

Thanks for the contribution! This idea has been incorporated into https://github.com/neon-bindings/rfcs/pull/44.