Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: ResultExt trait #39

Closed
wants to merge 6 commits into from
Closed

Conversation

vtenfys
Copy link

@vtenfys vtenfys commented Mar 10, 2021

This follows discussion in neon-bindings/examples#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
Copy link
Member

@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
Copy link
Author

vtenfys commented Mar 11, 2021

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 rust-lang/rust#44265 lands?

@kjvalencik
Copy link
Member

kjvalencik commented Mar 11, 2021

@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
Copy link
Member

Thanks for the contribution! This idea has been incorporated into #44.

@kjvalencik kjvalencik closed this Sep 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants