fix(neon): Relax lifetime constraints on With and provide helper for forcing HRTB with JS values

crates/neon/src/macros.rs

@@ -201,15 +201,15 @@ pub use neon_macros::main;
 /// ```
 /// # #[cfg(all(feature = "napi-6", feature = "futures"))]
 /// # {
-/// # use neon::types::extract::{TryIntoJs, With};
+/// # use neon::types::extract::{self, TryIntoJs};
 /// #[neon::export]
 /// async fn add(a: f64, b: f64) -> impl for<'cx> TryIntoJs<'cx> {
 ///     let sum = a + b;
 ///
-///     With(move |_cx| {
+///     extract::with(move |cx| {
 ///         println!("Hello from the JavaScript main thread!");
 ///
-///         sum
+///         sum.try_into_js(cx)
 ///     })
 /// }
 /// # }

crates/neon/src/types_impl/extract/mod.rs

@@ -109,7 +109,7 @@ use crate::{
 pub use self::{
     boxed::Boxed,
     error::{Error, TypeExpected},
-    with::With,
+    with::{with, With},
 };
 
 #[cfg(feature = "serde")]

crates/neon/src/types_impl/extract/with.rs

@@ -1,15 +1,28 @@
-use crate::{context::Cx, result::JsResult, types::extract::TryIntoJs};
+use crate::{
+    context::Cx,
+    result::JsResult,
+    types::{extract::TryIntoJs, Value},
+};
 
 /// Wraps a closure that will be lazily evaluated when [`TryIntoJs::try_into_js`] is
 /// called.
 ///
 /// Useful for executing arbitrary code on the main thread before returning from a
 /// function exported with [`neon::export`](crate::export).
 ///
+/// If you see the following error, due to [incorrect inference][issue], use [`with`]
+/// instead.
+///
+/// [issue]: https://github.com/rust-lang/rust/issues/70263
+///
+/// ```text
+/// error: implementation of `neon::types::extract::TryIntoJs` is not general enough
+/// ```
+///
 /// ## Example
 ///
 /// ```
-/// # use neon::{prelude::*, types::extract::{TryIntoJs, With}};
+/// # use neon::{prelude::*, types::extract::{self, TryIntoJs}};
 /// use std::time::Instant;
 ///
 /// #[neon::export(task)]
@@ -18,28 +31,44 @@ use crate::{context::Cx, result::JsResult, types::extract::TryIntoJs};
 ///     let sum = nums.into_iter().sum::<f64>();
 ///     let log = format!("sum took {} ms", start.elapsed().as_millis());
 ///
-///     With(move |cx| -> NeonResult<_> {
+///     extract::with(move |cx| -> NeonResult<_> {
 ///         cx.global::<JsObject>("console")?
 ///             .method(cx, "log")?
 ///             .arg(&log)?
 ///             .exec()?;
 ///
-///         Ok(sum)
+///         sum.try_into_js(cx)
 ///     })
 /// }
 /// ```
-pub struct With<F, O>(pub F)
+pub struct With<F>(pub F);
+
+/// Helper to ensure correct inference of [lifetime bounds][hrtb] on closures
+/// provided to [`With<F>`](With) without needing [explicit annotation][binder].
+///
+/// **Note:** The return type is [`JsResult`]. If you need to return a non-JavaScript type,
+/// call [`TryIntoJs::try_into_js`].
+///
+/// _See [`With`](With#Example) for example usage._
+///
+/// [hrtb]: https://doc.rust-lang.org/nomicon/hrtb.html
+/// [binder]: https://rust-lang.github.io/rfcs/3216-closure-lifetime-binder.html
+pub fn with<V, F>(f: F) -> With<F>
 where
-    // N.B.: We include additional required bounds to allow the compiler to infer the
-    // correct closure argument when using `impl for<'cx> TryIntoJs<'cx>`. Without
-    // these bounds, it would be necessary to write a more verbose signature:
-    // `With<impl for<'cx> FnOnce(&mut Cx<'cx>) -> SomeConcreteReturnType>`.
-    for<'cx> F: FnOnce(&mut Cx<'cx>) -> O;
+    V: Value,
+    for<'cx> F: FnOnce(&mut Cx<'cx>) -> JsResult<'cx, V>,
+
+    // N.B.: This bound ensures that the return type implements `TryIntoJs<'_>`
+    // without making it an opaque `impl Trait`.
+    for<'cx> With<F>: TryIntoJs<'cx, Value = V>,
+{
+    With(f)
+}
 
-impl<'cx, F, O> TryIntoJs<'cx> for With<F, O>
+impl<'cx, O, F> TryIntoJs<'cx> for With<F>
 where
-    F: FnOnce(&mut Cx) -> O,
     O: TryIntoJs<'cx>,
+    F: FnOnce(&mut Cx<'cx>) -> O,
 {
     type Value = O::Value;
 
@@ -48,4 +77,4 @@ where
     }
 }
 
-impl<F, O> super::private::Sealed for With<F, O> where for<'cx> F: FnOnce(&mut Cx<'cx>) -> O {}
+impl<F> super::private::Sealed for With<F> {}

crates/neon/src/types_impl/function/mod.rs

@@ -217,19 +217,19 @@ impl<'cx> private::TryIntoArgumentsInternal<'cx> for () {
     }
 }
 
-impl<'cx, F, O> private::TryIntoArgumentsInternal<'cx> for With<F, O>
+impl<'cx, F, O> private::TryIntoArgumentsInternal<'cx> for With<F>
 where
-    F: FnOnce(&mut Cx) -> O,
+    F: FnOnce(&mut Cx<'cx>) -> O,
     O: private::TryIntoArgumentsInternal<'cx>,
 {
     fn try_into_args_vec(self, cx: &mut Cx<'cx>) -> NeonResult<private::ArgsVec<'cx>> {
         (self.0)(cx).try_into_args_vec(cx)
     }
 }
 
-impl<'cx, F, O> TryIntoArguments<'cx> for With<F, O>
+impl<'cx, F, O> TryIntoArguments<'cx> for With<F>
 where
-    F: FnOnce(&mut Cx) -> O,
+    F: FnOnce(&mut Cx<'cx>) -> O,
     O: TryIntoArguments<'cx>,
 {
 }

test/napi/lib/extract.js

@@ -78,4 +78,11 @@ describe("Extractors", () => {
       }
     );
   });
+
+  it("With", async () => {
+    assert.strictEqual(await addon.sleepWithJs(1.5), 1.5);
+    assert.strictEqual(await addon.sleepWithJsSync(1.5), 1.5);
+    assert.strictEqual(await addon.sleepWith(1.5), 1.5);
+    assert.strictEqual(await addon.sleepWithSync(1.5), 1.5);
+  });
 });

test/napi/src/js/extract.rs

@@ -145,3 +145,45 @@ pub fn extract_either(either: Either<String, f64>) -> String {
         Either::Right(n) => format!("Number: {n}"),
     }
 }
+
+#[neon::export(task)]
+// Ensure that `With` produces a closure that can be moved across thread boundaries
+// and can return a JavaScript value.
+fn sleep_with_js(n: f64) -> impl for<'cx> TryIntoJs<'cx> {
+    use std::{thread, time::Duration};
+
+    thread::sleep(Duration::from_millis(n as u64));
+
+    with(move |cx| Ok(cx.number(n)))
+}
+
+#[neon::export]
+// Ensure that `With` can be used synchronously
+fn sleep_with_js_sync(n: f64) -> impl for<'cx> TryIntoJs<'cx> {
+    sleep_with_js(n)
+}
+
+#[neon::export(task)]
+// Ensure that `With` can be used Rust data
+fn sleep_with(n: f64) -> impl for<'cx> TryIntoJs<'cx> {
+    use std::{thread, time::Duration};
+
+    // HACK: Force HRTB on the closure. Can be replaced with `closure_lifetime_binder`
+    // https://rust-lang.github.io/rfcs/3216-closure-lifetime-binder.html
+    fn bind<O, F>(f: F) -> F
+    where
+        for<'cx> F: FnOnce(&mut Cx<'cx>) -> O,
+    {
+        f
+    }
+
+    thread::sleep(Duration::from_millis(n as u64));
+
+    With(bind(move |_| n))
+}
+
+#[neon::export]
+// Ensure that `With` can be used Rust data synchronously
+fn sleep_with_sync(n: f64) -> impl for<'cx> TryIntoJs<'cx> {
+    sleep_with(n)
+}

test/napi/src/js/functions.rs

@@ -47,10 +47,19 @@ pub fn call_js_function_with_bind_and_args_with(mut cx: FunctionContext) -> JsRe
 }
 
 pub fn call_js_function_with_bind_and_args_and_with(mut cx: FunctionContext) -> JsResult<JsNumber> {
+    // HACK: Force HRTB on the closure. Can be replaced with `closure_lifetime_binder`
+    // https://rust-lang.github.io/rfcs/3216-closure-lifetime-binder.html
+    fn bind<O, F>(f: F) -> F
+    where
+        for<'cx> F: FnOnce(&mut Cx<'cx>) -> O,
+    {
+        f
+    }
+
     let n: f64 = cx
         .argument::<JsFunction>(0)?
         .bind(&mut cx)
-        .args(With(|_| (1, 2, 3)))?
+        .args(With(bind(|_| (1, 2, 3))))?
         .call()?;
     Ok(cx.number(n))
 }

test/napi/src/js/futures.rs

@@ -4,7 +4,7 @@ use neon::{
     prelude::*,
     types::{
         buffer::TypedArray,
-        extract::{Error, Json, TryIntoJs, With},
+        extract::{self, Error, Json, TryIntoJs},
     },
 };
 
@@ -129,9 +129,9 @@ fn async_with_events(
     Ok(async move {
         let res = data.into_iter().map(|(a, b)| a * b).collect::<Vec<_>>();
 
-        With(move |cx| -> NeonResult<_> {
+        extract::with(move |cx| -> NeonResult<_> {
             emit(cx, "end")?;
-            Ok(res)
+            res.try_into_js(cx)
         })
     })
 }