Merge pull request #1086 from neon-bindings/kv/with-fix

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

@@ -113,7 +113,7 @@ pub use self::{
         Int32Array, Int8Array, Uint16Array, Uint32Array, Uint8Array,
     },
     error::{Error, TypeExpected},
-    with::With,
+    with::with,
 };
 
 #[cfg(feature = "serde")]

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

@@ -1,15 +1,26 @@
-use crate::{context::Cx, result::JsResult, types::extract::TryIntoJs};
+use crate::{
+    context::Cx,
+    result::JsResult,
+    types::{extract::TryIntoJs, Value},
+};
+
+struct With<F>(pub F);
 
 /// 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).
 ///
+/// **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._
+///
 /// ## Example
 ///
 /// ```
-/// # use neon::{prelude::*, types::extract::{TryIntoJs, With}};
+/// # use neon::{prelude::*, types::extract::{self, TryIntoJs}};
 /// use std::time::Instant;
 ///
 /// #[neon::export(task)]
@@ -18,28 +29,28 @@ 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 fn with<V, F>(f: F) -> impl for<'cx> TryIntoJs<'cx, Value = V>
 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>,
+{
+    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 +59,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

@@ -8,7 +8,7 @@ use crate::{
     object::Object,
     result::{JsResult, NeonResult},
     types::{
-        extract::{TryFromJs, TryIntoJs, With},
+        extract::{TryFromJs, TryIntoJs},
         private::ValueInternal,
         JsFunction, JsObject, JsValue, Value,
     },
@@ -217,23 +217,6 @@ impl<'cx> private::TryIntoArgumentsInternal<'cx> for () {
     }
 }
 
-impl<'cx, F, O> private::TryIntoArgumentsInternal<'cx> for With<F, O>
-where
-    F: FnOnce(&mut 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>
-where
-    F: FnOnce(&mut Cx) -> O,
-    O: TryIntoArguments<'cx>,
-{
-}
-
 impl<'cx, T, E> private::TryIntoArgumentsInternal<'cx> for Result<T, E>
 where
     T: private::TryIntoArgumentsInternal<'cx>,

test/napi/lib/extract.js

@@ -90,4 +90,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

@@ -158,3 +158,36 @@ pub fn buffer_concat(mut a: Vec<u8>, Uint8Array(b): Uint8Array<Vec<u8>>) -> Arra
 pub fn string_to_buf(s: String) -> Uint8Array<String> {
     Uint8Array(s)
 }
+
+#[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};
+
+    thread::sleep(Duration::from_millis(n as u64));
+
+    with(move |cx| n.try_into_js(cx))
+}
+
+#[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

@@ -1,4 +1,4 @@
-use neon::{prelude::*, types::extract::With};
+use neon::{prelude::*, types::extract};
 
 fn add1(mut cx: FunctionContext) -> JsResult<JsNumber> {
     let x = cx.argument::<JsNumber>(0)?.value(&mut cx);
@@ -50,7 +50,9 @@ pub fn call_js_function_with_bind_and_args_and_with(mut cx: FunctionContext) ->
     let n: f64 = cx
         .argument::<JsFunction>(0)?
         .bind(&mut cx)
-        .args(With(|_| (1, 2, 3)))?
+        .arg(extract::with(|cx| Ok(cx.number(1))))?
+        .arg(2)?
+        .arg(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)
         })
     })
 }