diff --git a/crates/neon/src/macros.rs b/crates/neon/src/macros.rs index 4180b4510..6f079338a 100644 --- a/crates/neon/src/macros.rs +++ b/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) /// }) /// } /// # } diff --git a/crates/neon/src/types_impl/extract/mod.rs b/crates/neon/src/types_impl/extract/mod.rs index 48957cb54..192d753ff 100644 --- a/crates/neon/src/types_impl/extract/mod.rs +++ b/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")] diff --git a/crates/neon/src/types_impl/extract/with.rs b/crates/neon/src/types_impl/extract/with.rs index bfce5c03f..da2fd8321 100644 --- a/crates/neon/src/types_impl/extract/with.rs +++ b/crates/neon/src/types_impl/extract/with.rs @@ -1,4 +1,8 @@ -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. @@ -6,10 +10,19 @@ use crate::{context::Cx, result::JsResult, types::extract::TryIntoJs}; /// 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::(); /// let log = format!("sum took {} ms", start.elapsed().as_millis()); /// -/// With(move |cx| -> NeonResult<_> { +/// extract::with(move |cx| -> NeonResult<_> { /// cx.global::("console")? /// .method(cx, "log")? /// .arg(&log)? /// .exec()?; /// -/// Ok(sum) +/// sum.try_into_js(cx) /// }) /// } /// ``` -pub struct With(pub F) +pub struct With(pub F); + +/// Helper to ensure correct inference of [lifetime bounds][hrtb] on closures +/// provided to [`With`](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(f: F) -> With 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 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: TryIntoJs<'cx, Value = V>, +{ + With(f) +} -impl<'cx, F, O> TryIntoJs<'cx> for With +impl<'cx, O, F> TryIntoJs<'cx> for With 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 super::private::Sealed for With where for<'cx> F: FnOnce(&mut Cx<'cx>) -> O {} +impl super::private::Sealed for With {} diff --git a/crates/neon/src/types_impl/function/mod.rs b/crates/neon/src/types_impl/function/mod.rs index bd3b8e0a4..d0a9b6867 100644 --- a/crates/neon/src/types_impl/function/mod.rs +++ b/crates/neon/src/types_impl/function/mod.rs @@ -217,9 +217,9 @@ impl<'cx> private::TryIntoArgumentsInternal<'cx> for () { } } -impl<'cx, F, O> private::TryIntoArgumentsInternal<'cx> for With +impl<'cx, F, O> private::TryIntoArgumentsInternal<'cx> for With 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> { @@ -227,9 +227,9 @@ where } } -impl<'cx, F, O> TryIntoArguments<'cx> for With +impl<'cx, F, O> TryIntoArguments<'cx> for With where - F: FnOnce(&mut Cx) -> O, + F: FnOnce(&mut Cx<'cx>) -> O, O: TryIntoArguments<'cx>, { } diff --git a/test/napi/lib/extract.js b/test/napi/lib/extract.js index 62926e3da..ddb3e0a45 100644 --- a/test/napi/lib/extract.js +++ b/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); + }); }); diff --git a/test/napi/src/js/extract.rs b/test/napi/src/js/extract.rs index a3c1da5dd..7dd19eecc 100644 --- a/test/napi/src/js/extract.rs +++ b/test/napi/src/js/extract.rs @@ -145,3 +145,45 @@ pub fn extract_either(either: Either) -> 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(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) +} diff --git a/test/napi/src/js/functions.rs b/test/napi/src/js/functions.rs index 2abcab8b2..3cdc65c73 100644 --- a/test/napi/src/js/functions.rs +++ b/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 { + // 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(f: F) -> F + where + for<'cx> F: FnOnce(&mut Cx<'cx>) -> O, + { + f + } + let n: f64 = cx .argument::(0)? .bind(&mut cx) - .args(With(|_| (1, 2, 3)))? + .args(With(bind(|_| (1, 2, 3))))? .call()?; Ok(cx.number(n)) } diff --git a/test/napi/src/js/futures.rs b/test/napi/src/js/futures.rs index acbba29cb..c4446a3b3 100644 --- a/test/napi/src/js/futures.rs +++ b/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::>(); - With(move |cx| -> NeonResult<_> { + extract::with(move |cx| -> NeonResult<_> { emit(cx, "end")?; - Ok(res) + res.try_into_js(cx) }) }) }