Add closure support.

[abrodersen]

Rendered

Add support for Rust closures to the JsFunction type.

[dherman]

A very nicely thought-out RFC!

So I think there are some interesting cross-cutting questions with the "classes 2.0" macros (see #6). My vision is for the default and most common idiom for defining both functions and methods is through the front-end macros:

module! {
    ...
    function increment(x: u32) -> u32 {
        Ok(x + 1)
    }
    ...
}

This abstracts away the most annoying boilerplate in Neon, and we should be able to implement it using bare functions instead of closures, to avoid the cost of the extra runtime indirection.

But I also think I agree with the premise of this RFC, which is that in the cases where you're doing it programmatically, you'd like to have the flexibility closures provide available to you.

This makes me think it's actually OK to offer two different constructors for JsFunction, one that takes bare functions (call it JsFunction::from_fn) and one that takes closures (call it JsFunction::new, using your RFC's design). From an API design perspective, I don't usually like to push those kinds of decisions on programmers. But in practice, I think the distribution would be something like 90% of cases use the macro, 9% of cases use the closure API, and 1% of cases use from_fn. But it would also allow the macro implementation to use from_fn under the hood, to get maximum performance.

[abrodersen]

@dherman I think that makes sense. I'll update my pull request to add a from_fn constructor for the JsFunction struct.

[dherman]

I am going to try to see if I can implement this without the Box indirection and without rustc getting all riled up. 😛 If we can do that, it's fully backwards-compatible. But either way I'd like to see if I can get this implemented in the 0.2 release.

[amckibben]

Any update on this? I'd really love to see closure support.

[kjvalencik]

@amckibben I have some concerns about implementing this. There are no stable features in N-API to attach a Drop task when the JsFunction gets garbage collected. I've also seem some comments from the N-API team that seem to suggest that in v8 native backed functions can't be garbage collected. This would present a memory leak.

[dherman]

This would present a memory leak.

Unfortunately I think this is going to make this unworkable, as nice as it would be. Neon is just limited by the limitations of V8 and N-API. I filed neon-bindings/neon#518 for us to document the reason for this restriction in the API docs.

[ovr]

ref https://github.com/neon-bindings/neon/issues/518#issuecomment-920378958

Closing because this is something that has since been implemented in Node-API.

Can anyone provide information about this RFC? If it's possible to be implemented via the new NAPI, let's reopen this RFC or I can make a new RFC for this one, because it looks useful.

[kjvalencik]

@ovr It might be worth re-writing in the context of the current API which has gone through some changes. Additionally, I don't think this RFC can be implemented as written since it would require GATs (generic associated types).

It's a nice ergonomic API, but I think everything in this could be accomplished with a static dispatch function and a JsBox. Possibly with a bit of glue code in JS.