From 888466790b0dc9de95c15ce8d29da9e2641eb14c Mon Sep 17 00:00:00 2001 From: David Herman Date: Wed, 17 Nov 2021 23:07:00 -0800 Subject: [PATCH 1/8] Add `JsFunction::exec()` --- src/types/mod.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/types/mod.rs b/src/types/mod.rs index 8789e1aaf..a356fb8f8 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -811,6 +811,20 @@ impl JsFunction { }) } + pub fn exec<'a, 'b, C: Context<'a>, T, AS>( + self, + cx: &mut C, + this: Handle<'b, T>, + args: AS, + ) -> NeonResult<()> + where + T: Value, + AS: AsRef<[Handle<'b, JsValue>]>, + { + self.call(cx, this, args)?; + Ok(()) + } + pub fn construct<'a, 'b, C: Context<'a>, AS>(self, cx: &mut C, args: AS) -> JsResult<'a, CL> where AS: AsRef<[Handle<'b, JsValue>]>, From c9df3b632cf94bf0b9f84fbfa51067dff362a63c Mon Sep 17 00:00:00 2001 From: David Herman Date: Thu, 18 Nov 2021 14:48:02 -0800 Subject: [PATCH 2/8] Rename `neon::types::internal` to `neon::types::private`. --- src/object/class/mod.rs | 2 +- src/types/binary.rs | 2 +- src/types/boxed.rs | 2 +- src/types/buffer/types.rs | 2 +- src/types/date.rs | 2 +- src/types/error.rs | 2 +- src/types/mod.rs | 26 ++++++++++++-------------- src/types/{internal.rs => private.rs} | 0 src/types/promise.rs | 2 +- 9 files changed, 19 insertions(+), 21 deletions(-) rename src/types/{internal.rs => private.rs} (100%) diff --git a/src/object/class/mod.rs b/src/object/class/mod.rs index 8b5c50791..44f66c71a 100644 --- a/src/object/class/mod.rs +++ b/src/object/class/mod.rs @@ -11,7 +11,7 @@ use crate::context::{Context, Lock}; use crate::handle::{Handle, Managed}; use crate::object::{Object, This}; use crate::result::{JsResult, NeonResult, Throw}; -use crate::types::internal::{Callback, ValueInternal}; +use crate::types::private::{Callback, ValueInternal}; use crate::types::{build, JsFunction, JsValue, Value}; use neon_runtime; use neon_runtime::raw; diff --git a/src/types/binary.rs b/src/types/binary.rs index 98dba5d27..f9434fc53 100644 --- a/src/types/binary.rs +++ b/src/types/binary.rs @@ -4,7 +4,7 @@ use crate::context::internal::Env; use crate::context::{Context, Lock}; use crate::handle::Managed; use crate::result::JsResult; -use crate::types::internal::ValueInternal; +use crate::types::private::ValueInternal; use crate::types::{build, Object, Value}; use neon_runtime; use neon_runtime::raw; diff --git a/src/types/boxed.rs b/src/types/boxed.rs index d687e2064..867e81157 100644 --- a/src/types/boxed.rs +++ b/src/types/boxed.rs @@ -8,7 +8,7 @@ use crate::context::internal::Env; use crate::context::{Context, FinalizeContext}; use crate::handle::{Handle, Managed}; use crate::object::Object; -use crate::types::internal::ValueInternal; +use crate::types::private::ValueInternal; use crate::types::Value; type BoxAny = Box; diff --git a/src/types/buffer/types.rs b/src/types/buffer/types.rs index b24a23635..dc0bdcf6d 100644 --- a/src/types/buffer/types.rs +++ b/src/types/buffer/types.rs @@ -6,7 +6,7 @@ use neon_runtime::{raw, TypedArrayType}; use crate::context::{internal::Env, Context}; use crate::handle::{Handle, Managed}; use crate::result::{JsResult, Throw}; -use crate::types::{internal::ValueInternal, Object, Value}; +use crate::types::{private::ValueInternal, Object, Value}; use super::lock::{Ledger, Lock}; use super::{private, BorrowError, Ref, RefMut, TypedArray}; diff --git a/src/types/date.rs b/src/types/date.rs index 76c636f6e..0cd639aa7 100644 --- a/src/types/date.rs +++ b/src/types/date.rs @@ -1,4 +1,4 @@ -use super::{Value, ValueInternal}; +use super::{private::ValueInternal, Value}; use crate::context::internal::Env; use crate::context::Context; use crate::handle::{Handle, Managed}; diff --git a/src/types/error.rs b/src/types/error.rs index 3b89589af..3d3416644 100644 --- a/src/types/error.rs +++ b/src/types/error.rs @@ -8,7 +8,7 @@ use neon_runtime::raw; use crate::context::internal::Env; use crate::context::Context; use crate::result::{NeonResult, Throw}; -use crate::types::internal::ValueInternal; +use crate::types::private::ValueInternal; use crate::types::utf8::Utf8; use crate::types::{build, Handle, Managed, Object, Value}; diff --git a/src/types/mod.rs b/src/types/mod.rs index a356fb8f8..631617856 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -86,10 +86,9 @@ pub(crate) mod error; #[cfg(all(feature = "napi-1", feature = "promise-api"))] pub(crate) mod promise; -pub(crate) mod internal; +pub(crate) mod private; pub(crate) mod utf8; -use self::internal::ValueInternal; use self::utf8::Utf8; use crate::context::internal::Env; use crate::context::{Context, FunctionContext}; @@ -143,7 +142,7 @@ impl SuperType for JsObject { } /// The trait shared by all JavaScript values. -pub trait Value: ValueInternal { +pub trait Value: private::ValueInternal { fn to_string<'a, C: Context<'a>>(self, cx: &mut C) -> JsResult<'a, JsString> { let env = cx.env(); build(env, |out| unsafe { @@ -173,7 +172,7 @@ impl Managed for JsValue { } } -impl ValueInternal for JsValue { +impl private::ValueInternal for JsValue { fn name() -> String { "any".to_string() } @@ -259,7 +258,7 @@ unsafe impl This for JsUndefined { } } -impl ValueInternal for JsUndefined { +impl private::ValueInternal for JsUndefined { fn name() -> String { "undefined".to_string() } @@ -306,7 +305,7 @@ impl Managed for JsNull { } } -impl ValueInternal for JsNull { +impl private::ValueInternal for JsNull { fn name() -> String { "null".to_string() } @@ -358,7 +357,7 @@ impl Managed for JsBoolean { } } -impl ValueInternal for JsBoolean { +impl private::ValueInternal for JsBoolean { fn name() -> String { "boolean".to_string() } @@ -407,7 +406,7 @@ impl Managed for JsString { } } -impl ValueInternal for JsString { +impl private::ValueInternal for JsString { fn name() -> String { "string".to_string() } @@ -528,7 +527,7 @@ impl Managed for JsNumber { } } -impl ValueInternal for JsNumber { +impl private::ValueInternal for JsNumber { fn name() -> String { "number".to_string() } @@ -567,7 +566,7 @@ unsafe impl This for JsObject { } } -impl ValueInternal for JsObject { +impl private::ValueInternal for JsObject { fn name() -> String { "object".to_string() } @@ -667,7 +666,7 @@ impl Managed for JsArray { } } -impl ValueInternal for JsArray { +impl private::ValueInternal for JsArray { fn name() -> String { "Array".to_string() } @@ -717,8 +716,7 @@ impl JsFunction { C: Context<'a>, U: Value, { - use self::internal::FunctionCallback; - use crate::types::internal::Callback; + use self::private::{Callback, FunctionCallback}; build(cx.env(), |out| { let env = cx.env().to_raw(); @@ -852,7 +850,7 @@ impl Managed for JsFunction { } } -impl ValueInternal for JsFunction { +impl private::ValueInternal for JsFunction { fn name() -> String { "function".to_string() } diff --git a/src/types/internal.rs b/src/types/private.rs similarity index 100% rename from src/types/internal.rs rename to src/types/private.rs diff --git a/src/types/promise.rs b/src/types/promise.rs index d816f4b6c..b325fa033 100644 --- a/src/types/promise.rs +++ b/src/types/promise.rs @@ -12,7 +12,7 @@ use crate::handle::Managed; #[cfg(feature = "napi-6")] use crate::lifecycle::{DropData, InstanceData}; use crate::result::JsResult; -use crate::types::{Handle, Object, Value, ValueInternal}; +use crate::types::{private::ValueInternal, Handle, Object, Value}; #[cfg(feature = "channel-api")] use crate::{ context::TaskContext, From 1a0463511afcb92a09444c91ce6764d0808fb405 Mon Sep 17 00:00:00 2001 From: David Herman Date: Fri, 19 Nov 2021 09:45:18 -0800 Subject: [PATCH 3/8] Simplify example code in doc comment. --- src/context/mod.rs | 3 +-- src/types/mod.rs | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/context/mod.rs b/src/context/mod.rs index 4bcb76110..08e37ef9c 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -94,8 +94,7 @@ //! //! while !done { //! done = cx.execute_scoped(|mut cx| { // temporary scope -//! let args: Vec> = vec![]; -//! let obj = next.call(&mut cx, iterator, args)? // temporary object +//! let obj = next.call(&mut cx, iterator, &[])? // temporary object //! .downcast_or_throw::(&mut cx)?; //! let number = obj.get(&mut cx, "value")? // temporary number //! .downcast_or_throw::(&mut cx)? diff --git a/src/types/mod.rs b/src/types/mod.rs index 631617856..27585fbb7 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -83,6 +83,7 @@ pub mod buffer; #[cfg(feature = "napi-5")] pub(crate) mod date; pub(crate) mod error; +pub mod function; #[cfg(all(feature = "napi-1", feature = "promise-api"))] pub(crate) mod promise; From 778ab2806b63f0d7fbd1083f0700b82a7abf5a81 Mon Sep 17 00:00:00 2001 From: David Herman Date: Fri, 19 Nov 2021 10:23:28 -0800 Subject: [PATCH 4/8] Define the Arguments trait and CallOptions / ConstructOptions types. --- src/types/function/mod.rs | 217 ++++++++++++++++++++++++++++++++++ src/types/function/private.rs | 11 ++ 2 files changed, 228 insertions(+) create mode 100644 src/types/function/mod.rs create mode 100644 src/types/function/private.rs diff --git a/src/types/function/mod.rs b/src/types/function/mod.rs new file mode 100644 index 000000000..e86c1e8ee --- /dev/null +++ b/src/types/function/mod.rs @@ -0,0 +1,217 @@ +//! Types and traits for working with JavaScript functions. + +use crate::context::Context; +use crate::handle::Handle; +use crate::result::{JsResult, NeonResult}; +use crate::types::{JsFunction, JsObject, JsValue, Value}; + +use smallvec::smallvec; + +pub(crate) mod private; + +/// A builder for making a JavaScript function call like `parseInt("42")`. +/// +/// The builder methods make it convenient to assemble the call from parts: +/// ``` +/// # use neon::prelude::*; +/// # fn foo(mut cx: FunctionContext) -> JsResult { +/// # let global = cx.global(); +/// # let parse_int = global.get(&mut cx, "parseInt")?; +/// # let parse_int: Handle = parse_int.downcast_or_throw(&mut cx)?; +/// let x: Handle = parse_int +/// .call_with(&cx) +/// .arg(cx.string("42")) +/// .apply(&mut cx)?; +/// # Ok(x) +/// # } +/// ``` +#[derive(Clone)] +pub struct CallOptions<'a> { + pub(crate) callee: Handle<'a, JsFunction>, + pub(crate) this: Option>, + pub(crate) args: private::ArgsVec<'a>, +} + +impl<'a> CallOptions<'a> { + /// Set the value of `this` for the function call. + pub fn this(&mut self, this: Handle<'a, V>) -> &mut Self { + self.this = Some(this.upcast()); + self + } + + /// Add an argument to the arguments list. + pub fn arg(&mut self, arg: Handle<'a, V>) -> &mut Self { + self.args.push(arg.upcast()); + self + } + + /// Replaces the arguments list with the given arguments. + pub fn args>(&mut self, args: A) -> &mut Self { + self.args = args.into_args_vec(); + self + } + + /// Make the function call. If the function returns without throwing, the result value + /// is downcast to the type `V`, throwing a `TypeError` if the downcast fails. + pub fn apply<'b: 'a, V: Value, C: Context<'b>>(&self, cx: &mut C) -> JsResult<'b, V> { + let this = self.this.unwrap_or_else(|| cx.null().upcast()); + let v: Handle = self.callee.call(cx, this, &self.args)?; + v.downcast_or_throw(cx) + } + + /// Make the function call for side effect, discarding the result value. This method is + /// preferable to [`apply()`](CallOptions::apply) when the result value isn't needed, + /// since it doesn't require specifying a result type. + pub fn exec<'b: 'a, C: Context<'b>>(&self, cx: &mut C) -> NeonResult<()> { + let this = self.this.unwrap_or_else(|| cx.null().upcast()); + self.callee.call(cx, this, &self.args)?; + Ok(()) + } +} + +/// A builder for making a JavaScript constructor call like `new Array(16)`. +/// +/// The builder methods make it convenient to assemble the call from parts: +/// ``` +/// # use neon::prelude::*; +/// # fn foo(mut cx: FunctionContext) -> JsResult { +/// # let global = cx.global(); +/// # let url = global.get(&mut cx, "URL")?; +/// # let url: Handle = url.downcast_or_throw(&mut cx)?; +/// let obj = url +/// .construct_with(&cx) +/// .arg(cx.string("https://neon-bindings.com")) +/// .apply(&mut cx)?; +/// # Ok(obj) +/// # } +/// ``` +#[derive(Clone)] +pub struct ConstructOptions<'a> { + pub(crate) callee: Handle<'a, JsFunction>, + pub(crate) args: private::ArgsVec<'a>, +} + +impl<'a> ConstructOptions<'a> { + /// Add an argument to the arguments list. + pub fn arg(&mut self, arg: Handle<'a, V>) -> &mut Self { + self.args.push(arg.upcast()); + self + } + + /// Replaces the arguments list with the given arguments. + pub fn args>(&mut self, args: A) -> &mut Self { + self.args = args.into_args_vec(); + self + } + + /// Make the constructor call. If the function returns without throwing, returns + /// the resulting object. + pub fn apply<'b: 'a, C: Context<'b>>(&self, cx: &mut C) -> JsResult<'b, JsObject> { + self.callee.construct(cx, &self.args) + } +} + +/// The trait for specifying arguments for a function call. This trait is sealed and cannot +/// be implemented by types outside of the Neon crate. +/// +/// **Note:** This trait is implemented for tuples of up to 32 JavaScript values, +/// but for the sake of brevity, only tuples up to size 8 are shown in this documentation. +pub trait Arguments<'a>: private::ArgumentsInternal<'a> {} + +impl<'a> private::ArgumentsInternal<'a> for () { + fn into_args_vec(self) -> private::ArgsVec<'a> { + smallvec![] + } +} + +impl<'a> Arguments<'a> for () {} + +macro_rules! impl_arguments { + { + [ $(($tprefix:ident, $vprefix:ident), )* ]; + []; + } => {}; + + { + [ $(($tprefix:ident, $vprefix:ident), )* ]; + [ $(#[$attr1:meta])? ($tname1:ident, $vname1:ident), $($(#[$attrs:meta])? ($tnames:ident, $vnames:ident), )* ]; + } => { + $(#[$attr1])? + impl<'a, $($tprefix: Value, )* $tname1: Value> private::ArgumentsInternal<'a> for ($(Handle<'a, $tprefix>, )* Handle<'a, $tname1>, ) { + fn into_args_vec(self) -> private::ArgsVec<'a> { + let ($($vprefix, )* $vname1, ) = self; + smallvec![$($vprefix.upcast(),)* $vname1.upcast()] + } + } + + $(#[$attr1])? + impl<'a, $($tprefix: Value, )* $tname1: Value> Arguments<'a> for ($(Handle<'a, $tprefix>, )* Handle<'a, $tname1>, ) {} + + impl_arguments! { + [ $(($tprefix, $vprefix), )* ($tname1, $vname1), ]; + [ $($(#[$attrs])? ($tnames, $vnames), )* ]; + } + }; + } + + impl_arguments! { + []; + [ + (V1, v1), + (V2, v2), + (V3, v3), + (V4, v4), + (V5, v5), + (V6, v6), + (V7, v7), + (V8, v8), + #[doc(hidden)] + (V9, v9), + #[doc(hidden)] + (V10, v10), + #[doc(hidden)] + (V11, v11), + #[doc(hidden)] + (V12, v12), + #[doc(hidden)] + (V13, v13), + #[doc(hidden)] + (V14, v14), + #[doc(hidden)] + (V15, v15), + #[doc(hidden)] + (V16, v16), + #[doc(hidden)] + (V17, v17), + #[doc(hidden)] + (V18, v18), + #[doc(hidden)] + (V19, v19), + #[doc(hidden)] + (V20, v20), + #[doc(hidden)] + (V21, v21), + #[doc(hidden)] + (V22, v22), + #[doc(hidden)] + (V23, v23), + #[doc(hidden)] + (V24, v24), + #[doc(hidden)] + (V25, v25), + #[doc(hidden)] + (V26, v26), + #[doc(hidden)] + (V27, v27), + #[doc(hidden)] + (V28, v28), + #[doc(hidden)] + (V29, v29), + #[doc(hidden)] + (V30, v30), + #[doc(hidden)] + (V31, v31), + #[doc(hidden)] + (V32, v32), + ]; +} diff --git a/src/types/function/private.rs b/src/types/function/private.rs new file mode 100644 index 000000000..e58d328fa --- /dev/null +++ b/src/types/function/private.rs @@ -0,0 +1,11 @@ +use crate::handle::Handle; +use crate::types::JsValue; + +use smallvec::SmallVec; + +pub type ArgsVec<'a> = SmallVec<[Handle<'a, JsValue>; 8]>; + +/// This type marks the `Arguments` trait as sealed. +pub trait ArgumentsInternal<'a> { + fn into_args_vec(self) -> ArgsVec<'a>; +} From 460394f6af0159bcc49cb50331505d73f104922d Mon Sep 17 00:00:00 2001 From: David Herman Date: Fri, 19 Nov 2021 18:49:33 -0800 Subject: [PATCH 5/8] Add `JsFunction::{call_with, construct_with}` --- src/context/mod.rs | 6 ++- src/types/mod.rs | 99 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 2 deletions(-) diff --git a/src/context/mod.rs b/src/context/mod.rs index 08e37ef9c..e47823144 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -94,8 +94,10 @@ //! //! while !done { //! done = cx.execute_scoped(|mut cx| { // temporary scope -//! let obj = next.call(&mut cx, iterator, &[])? // temporary object -//! .downcast_or_throw::(&mut cx)?; +//! let obj: Handle = next // temporary object +//! .call_with(&cx) +//! .this(iterator) +//! .apply(&mut cx)?; //! let number = obj.get(&mut cx, "value")? // temporary number //! .downcast_or_throw::(&mut cx)? //! .value(&mut cx); diff --git a/src/types/mod.rs b/src/types/mod.rs index 27585fbb7..01c60540d 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -90,6 +90,7 @@ pub(crate) mod promise; pub(crate) mod private; pub(crate) mod utf8; +use self::function::{CallOptions, ConstructOptions}; use self::utf8::Utf8; use crate::context::internal::Env; use crate::context::{Context, FunctionContext}; @@ -99,6 +100,7 @@ use crate::object::{Object, This}; use crate::result::{JsResult, JsResultExt, NeonResult, Throw}; use neon_runtime; use neon_runtime::raw; +use smallvec::smallvec; use std::fmt; use std::fmt::Debug; use std::marker::PhantomData; @@ -680,6 +682,83 @@ impl private::ValueInternal for JsArray { impl Object for JsArray {} /// A JavaScript function object. +/// +/// A `JsFunction` may come from an existing JavaScript function, for example +/// by extracting it from the property of another object such as the +/// [global object](crate::context::Context::global), or it may be defined in Rust +/// with [`JsFunction::new()`](JsFunction::new). +/// +/// ## Calling functions +/// +/// Neon provides a convenient syntax for calling JavaScript functions with the +/// [`call_with()`](JsFunction::call_with) method, which produces a [`CallOptions`](CallOptions) +/// struct that can be used to provide the function arguments (and optionally, the binding for +/// `this`) before calling the function: +/// ``` +/// # use neon::prelude::*; +/// # fn foo(mut cx: FunctionContext) -> JsResult { +/// # let global = cx.global(); +/// // Extract the parseInt function from the global object +/// let parse_int: Handle = global +/// .get(&mut cx, "parseInt")? +/// .downcast_or_throw(&mut cx)?; +/// +/// // Call parseInt("42") +/// let x: Handle = parse_int +/// .call_with(&mut cx) +/// .arg(cx.string("42")) +/// .apply(&mut cx)?; +/// # Ok(x) +/// # } +/// ``` +/// +/// ## Calling functions as constructors +/// +/// A `JsFunction` can be called as a constructor (like `new Array(16)` or +/// `new URL("https://neon-bindings.com")`) with the +/// [`construct_with()`](JsFunction::construct_with) method: +/// ``` +/// # use neon::prelude::*; +/// # fn foo(mut cx: FunctionContext) -> JsResult { +/// # let global = cx.global(); +/// // Extract the URL constructor from the global object +/// let url: Handle = global +/// .get(&mut cx, "URL")? +/// .downcast_or_throw(&mut cx)?; +/// +/// // Call new URL("https://neon-bindings.com") +/// let obj = url +/// .construct_with(&cx) +/// .arg(cx.string("https://neon-bindings.com")) +/// .apply(&mut cx)?; +/// # Ok(obj) +/// # } +/// ``` +/// +/// ## Defining functions +/// +/// JavaScript functions can be defined in Rust with the +/// [`JsFunction::new()`](JsFunction::new) constructor, which takes +/// a Rust implementation function and produces a JavaScript function. +/// +/// ``` +/// # use neon::prelude::*; +/// // A function implementation that adds 1 to its first argument +/// fn add1(mut cx: FunctionContext) -> JsResult { +/// let x: Handle = cx.argument(0)?; +/// # #[cfg(feature = "legacy-runtime")] +/// # let v = x.value(); +/// # #[cfg(feature = "napi-1")] +/// let v = x.value(&mut cx); +/// Ok(cx.number(v + 1.0)) +/// } +/// +/// # fn foo(mut cx: FunctionContext) -> JsResult { +/// // Define a new JsFunction implemented with the add1 function +/// let f = JsFunction::new(&mut cx, add1)?; +/// # Ok(f) +/// # } +/// ``` #[repr(C)] #[derive(Clone, Copy)] pub struct JsFunction { @@ -836,6 +915,26 @@ impl JsFunction { } } +impl JsFunction { + /// Create a [`CallOptions`](function::CallOptions) for calling this function. + pub fn call_with<'a, C: Context<'a>>(self, _cx: &C) -> CallOptions<'a> { + CallOptions { + this: None, + callee: Handle::new_internal(self), + args: smallvec![], + } + } + + /// Create a [`ConstructOptions`](function::ConstructOptions) for calling this function + /// as a constructor. + pub fn construct_with<'a, C: Context<'a>>(self, _cx: &C) -> ConstructOptions<'a> { + ConstructOptions { + callee: Handle::new_internal(self), + args: smallvec![], + } + } +} + impl Value for JsFunction {} impl Managed for JsFunction { From bab3b34d6d1b206a111802d7b443af6b128761a7 Mon Sep 17 00:00:00 2001 From: David Herman Date: Fri, 19 Nov 2021 19:28:44 -0800 Subject: [PATCH 6/8] Use call_with and construct_with in tests where it's cleaner --- test/dynamic/native/src/js/eventhandler.rs | 10 +++-- test/napi/src/js/threads.rs | 51 ++++++++-------------- 2 files changed, 23 insertions(+), 38 deletions(-) diff --git a/test/dynamic/native/src/js/eventhandler.rs b/test/dynamic/native/src/js/eventhandler.rs index df612a6ae..7be224717 100644 --- a/test/dynamic/native/src/js/eventhandler.rs +++ b/test/dynamic/native/src/js/eventhandler.rs @@ -111,8 +111,11 @@ declare_types! { if let Some(cb) = cb { thread::spawn(move || { cb.schedule_with(move |cx, this, callback| { - let args : Vec> = vec![cx.string("number").upcast()]; - let result = callback.call(cx, this, args); + let result: JsResult = callback + .call_with(cx) + .this(this) + .arg(cx.string("number")) + .apply(cx); let cmd = match result { Ok(v) => { if let Ok(number) = v.downcast::() { @@ -127,8 +130,7 @@ declare_types! { }, Err(e) => format!("threw {}", e) }; - let args : Vec> = vec![cx.string(cmd).upcast()]; - let _result = callback.call(cx, this, args); + let _result = callback.call_with(cx).this(this).arg(cx.string(cmd)).exec(cx); }); }); } diff --git a/test/napi/src/js/threads.rs b/test/napi/src/js/threads.rs index e8048d5b2..05d034997 100644 --- a/test/napi/src/js/threads.rs +++ b/test/napi/src/js/threads.rs @@ -18,15 +18,7 @@ pub fn thread_callback(mut cx: FunctionContext) -> JsResult { let channel = cx.channel(); std::thread::spawn(move || { - channel.send(move |mut cx| { - let callback = callback.into_inner(&mut cx); - let this = cx.undefined(); - let args = Vec::>::new(); - - callback.call(&mut cx, this, args)?; - - Ok(()) - }) + channel.send(move |mut cx| callback.into_inner(&mut cx).call_with(&cx).exec(&mut cx)) }); Ok(cx.undefined()) @@ -43,13 +35,11 @@ pub fn multi_threaded_callback(mut cx: FunctionContext) -> JsResult std::thread::spawn(move || { channel.send(move |mut cx| { - let callback = callback.into_inner(&mut cx); - let this = cx.undefined(); - let args = [cx.number(i as f64).upcast()]; - - callback.call(&mut cx, this, args)?; - - Ok(()) + callback + .into_inner(&mut cx) + .call_with(&cx) + .arg(cx.number(i as f64)) + .exec(&mut cx) }) }); } @@ -76,13 +66,11 @@ impl AsyncGreeter { std::thread::spawn(move || { channel.send(|mut cx| { - let callback = callback.into_inner(&mut cx); - let this = cx.undefined(); - let args = [cx.string(greeting).upcast()]; - - callback.call(&mut cx, this, args)?; - - Ok(()) + callback + .into_inner(&mut cx) + .call_with(&cx) + .arg(cx.string(greeting)) + .exec(&mut cx) }) }); @@ -97,10 +85,7 @@ impl Finalize for AsyncGreeter { } = self; if let Some(shutdown) = shutdown { - let shutdown = shutdown.into_inner(cx); - let this = cx.undefined(); - let args = Vec::>::new(); - let _ = shutdown.call(cx, this, args); + let _ = shutdown.into_inner(cx).call_with(cx).exec(cx); } callback.drop(cx); @@ -161,13 +146,11 @@ pub fn drop_global_queue(mut cx: FunctionContext) -> JsResult { fn drop(&mut self) { if let Some(callback) = self.callback.take() { self.channel.send(|mut cx| { - let callback = callback.into_inner(&mut cx); - let this = cx.undefined(); - let args = [cx.undefined().upcast()]; - - callback.call(&mut cx, this, args)?; - - Ok(()) + callback + .into_inner(&mut cx) + .call_with(&cx) + .arg(cx.undefined()) + .exec(&mut cx) }); } } From 856410d806e2d521f713cdb01a62d2a806400951 Mon Sep 17 00:00:00 2001 From: David Herman Date: Wed, 24 Nov 2021 13:50:29 -0800 Subject: [PATCH 7/8] Unit tests --- src/types/function/mod.rs | 2 +- test/napi/lib/functions.js | 36 +++++++++++++ test/napi/src/js/functions.rs | 98 +++++++++++++++++++++++++++++++++-- test/napi/src/lib.rs | 36 +++++++++++++ 4 files changed, 166 insertions(+), 6 deletions(-) diff --git a/src/types/function/mod.rs b/src/types/function/mod.rs index e86c1e8ee..a3806de78 100644 --- a/src/types/function/mod.rs +++ b/src/types/function/mod.rs @@ -154,7 +154,7 @@ macro_rules! impl_arguments { }; } - impl_arguments! { +impl_arguments! { []; [ (V1, v1), diff --git a/test/napi/lib/functions.js b/test/napi/lib/functions.js index 76d447fbd..2e80a6404 100644 --- a/test/napi/lib/functions.js +++ b/test/napi/lib/functions.js @@ -14,10 +14,46 @@ describe('JsFunction', function() { assert.equal(addon.call_js_function(function(x) { return x + 1 }), 17); }); + it('call a JsFunction built in JS with call_with', function () { + assert.equal(addon.call_js_function_idiomatically(function(x) { return x + 1 }), 17); + }); + + it('call a JsFunction with zero args', function() { + assert.equal(addon.call_js_function_with_zero_args(), -Infinity); + }); + + it('call a JsFunction with one arg', function() { + assert.equal(addon.call_js_function_with_one_arg(), 1.0); + }); + + it('call a JsFunction with two args', function() { + assert.equal(addon.call_js_function_with_two_args(), 2.0); + }); + + it('call a JsFunction with three args', function() { + assert.equal(addon.call_js_function_with_three_args(), 3.0); + }); + + it('call a JsFunction with four args', function() { + assert.equal(addon.call_js_function_with_four_args(), 4.0); + }); + + it('call a JsFunction with a custom this', function() { + assert.equal(addon.call_js_function_with_custom_this(function() { return this }).secret, 42); + }); + + it('call a JsFunction with a heterogeneously typed tuple', function() { + assert.deepEqual(addon.call_js_function_with_heterogeneous_tuple(), [1, "hello", true]); + }); + it('new a JsFunction', function () { assert.equal(addon.construct_js_function(Date), 1970); }); + it('new a JsFunction with construct_with', function () { + assert.equal(addon.construct_js_function_idiomatically(Date), 1970); + }); + it('got two parameters, a string and a number', function() { addon.check_string_and_number("string", 42); }); diff --git a/test/napi/src/js/functions.rs b/test/napi/src/js/functions.rs index dfc0b7767..ede01ba62 100644 --- a/test/napi/src/js/functions.rs +++ b/test/napi/src/js/functions.rs @@ -19,6 +19,82 @@ pub fn call_js_function(mut cx: FunctionContext) -> JsResult { .or_throw(&mut cx) } +pub fn call_js_function_idiomatically(mut cx: FunctionContext) -> JsResult { + cx.argument::(0)? + .call_with(&cx) + .this(cx.null()) + .arg(cx.number(16.0)) + .apply(&mut cx) +} + +fn get_math_max<'a>(cx: &mut FunctionContext<'a>) -> JsResult<'a, JsFunction> { + let math = cx + .global() + .get(cx, "Math")? + .downcast_or_throw::(cx)?; + let max = math + .get(cx, "max")? + .downcast_or_throw::(cx)?; + Ok(max) +} + +pub fn call_js_function_with_zero_args(mut cx: FunctionContext) -> JsResult { + get_math_max(&mut cx)?.call_with(&cx).apply(&mut cx) +} + +pub fn call_js_function_with_one_arg(mut cx: FunctionContext) -> JsResult { + get_math_max(&mut cx)? + .call_with(&cx) + .arg(cx.number(1.0)) + .apply(&mut cx) +} + +pub fn call_js_function_with_two_args(mut cx: FunctionContext) -> JsResult { + get_math_max(&mut cx)? + .call_with(&cx) + .arg(cx.number(1.0)) + .arg(cx.number(2.0)) + .apply(&mut cx) +} + +pub fn call_js_function_with_three_args(mut cx: FunctionContext) -> JsResult { + get_math_max(&mut cx)? + .call_with(&cx) + .arg(cx.number(1.0)) + .arg(cx.number(2.0)) + .arg(cx.number(3.0)) + .apply(&mut cx) +} + +pub fn call_js_function_with_four_args(mut cx: FunctionContext) -> JsResult { + get_math_max(&mut cx)? + .call_with(&cx) + .arg(cx.number(1.0)) + .arg(cx.number(2.0)) + .arg(cx.number(3.0)) + .arg(cx.number(4.0)) + .apply(&mut cx) +} + +pub fn call_js_function_with_custom_this(mut cx: FunctionContext) -> JsResult { + let custom_this = cx.empty_object(); + let secret = cx.number(42.0); + custom_this.set(&mut cx, "secret", secret)?; + cx.argument::(0)? + .call_with(&cx) + .this(custom_this) + .apply(&mut cx) +} + +pub fn call_js_function_with_heterogeneous_tuple(mut cx: FunctionContext) -> JsResult { + cx.global() + .get(&mut cx, "Array")? + .downcast_or_throw::(&mut cx)? + .call_with(&cx) + .args((cx.number(1.0), cx.string("hello"), cx.boolean(true))) + .apply(&mut cx) +} + pub fn construct_js_function(mut cx: FunctionContext) -> JsResult { let f = cx.argument::(0)?; let zero = cx.number(0.0); @@ -34,6 +110,22 @@ pub fn construct_js_function(mut cx: FunctionContext) -> JsResult { .or_throw(&mut cx) } +pub fn construct_js_function_idiomatically(mut cx: FunctionContext) -> JsResult { + let o = cx + .argument::(0)? + .construct_with(&cx) + .arg(cx.number(0.0)) + .apply(&mut cx)?; + let get_utc_full_year_method = o + .get(&mut cx, "getUTCFullYear")? + .downcast::(&mut cx) + .or_throw(&mut cx)?; + get_utc_full_year_method + .call_with(&cx) + .this(o) + .apply(&mut cx) +} + trait CheckArgument<'a> { fn check_argument(&mut self, i: i32) -> JsResult<'a, V>; } @@ -124,11 +216,7 @@ pub fn throw_and_catch(mut cx: FunctionContext) -> JsResult { pub fn call_and_catch(mut cx: FunctionContext) -> JsResult { let f: Handle = cx.argument(0)?; Ok(cx - .try_catch(|cx| { - let global = cx.global(); - let args: Vec> = vec![]; - f.call(cx, global, args) - }) + .try_catch(|cx| f.call_with(cx).this(cx.global()).apply(cx)) .unwrap_or_else(|err| err)) } diff --git a/test/napi/src/lib.rs b/test/napi/src/lib.rs index 86581656c..425fc1d9b 100644 --- a/test/napi/src/lib.rs +++ b/test/napi/src/lib.rs @@ -143,7 +143,43 @@ fn main(mut cx: ModuleContext) -> NeonResult<()> { cx.export_function("return_js_function", return_js_function)?; cx.export_function("call_js_function", call_js_function)?; + cx.export_function( + "call_js_function_idiomatically", + call_js_function_idiomatically, + )?; + cx.export_function( + "call_js_function_with_zero_args", + call_js_function_with_zero_args, + )?; + cx.export_function( + "call_js_function_with_one_arg", + call_js_function_with_one_arg, + )?; + cx.export_function( + "call_js_function_with_two_args", + call_js_function_with_two_args, + )?; + cx.export_function( + "call_js_function_with_three_args", + call_js_function_with_three_args, + )?; + cx.export_function( + "call_js_function_with_four_args", + call_js_function_with_four_args, + )?; + cx.export_function( + "call_js_function_with_custom_this", + call_js_function_with_custom_this, + )?; + cx.export_function( + "call_js_function_with_heterogeneous_tuple", + call_js_function_with_heterogeneous_tuple, + )?; cx.export_function("construct_js_function", construct_js_function)?; + cx.export_function( + "construct_js_function_idiomatically", + construct_js_function_idiomatically, + )?; cx.export_function("num_arguments", num_arguments)?; cx.export_function("return_this", return_this)?; cx.export_function("require_object_this", require_object_this)?; From b7617ed4a267fd68403ccc48fa64564533dab2df Mon Sep 17 00:00:00 2001 From: David Herman Date: Fri, 3 Dec 2021 11:04:57 -0800 Subject: [PATCH 8/8] Implement suggestions from review: - Use undefined instead of null for default this - Overloaded result type for `construct_with()` - Tests for all of the above --- src/types/function/mod.rs | 10 ++++++---- test/napi/lib/functions.js | 18 ++++++++++++++++++ test/napi/src/js/functions.rs | 27 ++++++++++++++++++++++++++- test/napi/src/lib.rs | 12 ++++++++++++ 4 files changed, 62 insertions(+), 5 deletions(-) diff --git a/src/types/function/mod.rs b/src/types/function/mod.rs index a3806de78..bc7e4793f 100644 --- a/src/types/function/mod.rs +++ b/src/types/function/mod.rs @@ -2,6 +2,7 @@ use crate::context::Context; use crate::handle::Handle; +use crate::object::Object; use crate::result::{JsResult, NeonResult}; use crate::types::{JsFunction, JsObject, JsValue, Value}; @@ -54,7 +55,7 @@ impl<'a> CallOptions<'a> { /// Make the function call. If the function returns without throwing, the result value /// is downcast to the type `V`, throwing a `TypeError` if the downcast fails. pub fn apply<'b: 'a, V: Value, C: Context<'b>>(&self, cx: &mut C) -> JsResult<'b, V> { - let this = self.this.unwrap_or_else(|| cx.null().upcast()); + let this = self.this.unwrap_or_else(|| cx.undefined().upcast()); let v: Handle = self.callee.call(cx, this, &self.args)?; v.downcast_or_throw(cx) } @@ -63,7 +64,7 @@ impl<'a> CallOptions<'a> { /// preferable to [`apply()`](CallOptions::apply) when the result value isn't needed, /// since it doesn't require specifying a result type. pub fn exec<'b: 'a, C: Context<'b>>(&self, cx: &mut C) -> NeonResult<()> { - let this = self.this.unwrap_or_else(|| cx.null().upcast()); + let this = self.this.unwrap_or_else(|| cx.undefined().upcast()); self.callee.call(cx, this, &self.args)?; Ok(()) } @@ -106,8 +107,9 @@ impl<'a> ConstructOptions<'a> { /// Make the constructor call. If the function returns without throwing, returns /// the resulting object. - pub fn apply<'b: 'a, C: Context<'b>>(&self, cx: &mut C) -> JsResult<'b, JsObject> { - self.callee.construct(cx, &self.args) + pub fn apply<'b: 'a, O: Object, C: Context<'b>>(&self, cx: &mut C) -> JsResult<'b, O> { + let v: Handle = self.callee.construct(cx, &self.args)?; + v.downcast_or_throw(cx) } } diff --git a/test/napi/lib/functions.js b/test/napi/lib/functions.js index 2e80a6404..654c1b03a 100644 --- a/test/napi/lib/functions.js +++ b/test/napi/lib/functions.js @@ -42,6 +42,20 @@ describe('JsFunction', function() { assert.equal(addon.call_js_function_with_custom_this(function() { return this }).secret, 42); }); + it('call a JsFunction with the default this', function() { + addon.call_js_function_with_implicit_this(function() { + 'use strict'; // ensure the undefined this isn't replaced with the global object + assert.strictEqual(this, undefined); + }); + }); + + it('exec a JsFunction with the default this', function() { + addon.exec_js_function_with_implicit_this(function() { + 'use strict'; // ensure the undefined this isn't replaced with the global object + assert.strictEqual(this, undefined); + }); + }) + it('call a JsFunction with a heterogeneously typed tuple', function() { assert.deepEqual(addon.call_js_function_with_heterogeneous_tuple(), [1, "hello", true]); }); @@ -54,6 +68,10 @@ describe('JsFunction', function() { assert.equal(addon.construct_js_function_idiomatically(Date), 1970); }); + it('new a JsFunction with construct_with to create an array', function() { + assert.deepEqual(addon.construct_js_function_with_overloaded_result(), [1, 2, 3]); + }); + it('got two parameters, a string and a number', function() { addon.check_string_and_number("string", 42); }); diff --git a/test/napi/src/js/functions.rs b/test/napi/src/js/functions.rs index ede01ba62..23db26d2c 100644 --- a/test/napi/src/js/functions.rs +++ b/test/napi/src/js/functions.rs @@ -86,6 +86,21 @@ pub fn call_js_function_with_custom_this(mut cx: FunctionContext) -> JsResult JsResult { + cx.argument::(0)? + .call_with(&cx) + .arg(cx.number(42)) + .apply(&mut cx) +} + +pub fn exec_js_function_with_implicit_this(mut cx: FunctionContext) -> JsResult { + cx.argument::(0)? + .call_with(&cx) + .arg(cx.number(42)) + .exec(&mut cx)?; + Ok(cx.undefined()) +} + pub fn call_js_function_with_heterogeneous_tuple(mut cx: FunctionContext) -> JsResult { cx.global() .get(&mut cx, "Array")? @@ -111,7 +126,7 @@ pub fn construct_js_function(mut cx: FunctionContext) -> JsResult { } pub fn construct_js_function_idiomatically(mut cx: FunctionContext) -> JsResult { - let o = cx + let o: Handle = cx .argument::(0)? .construct_with(&cx) .arg(cx.number(0.0)) @@ -126,6 +141,16 @@ pub fn construct_js_function_idiomatically(mut cx: FunctionContext) -> JsResult< .apply(&mut cx) } +pub fn construct_js_function_with_overloaded_result(mut cx: FunctionContext) -> JsResult { + let global = cx.global(); + let f: Handle = global.get(&mut cx, "Array")?.downcast_or_throw(&mut cx)?; + f.construct_with(&cx) + .arg(cx.number(1)) + .arg(cx.number(2)) + .arg(cx.number(3)) + .apply(&mut cx) +} + trait CheckArgument<'a> { fn check_argument(&mut self, i: i32) -> JsResult<'a, V>; } diff --git a/test/napi/src/lib.rs b/test/napi/src/lib.rs index 425fc1d9b..aa5a2cdf4 100644 --- a/test/napi/src/lib.rs +++ b/test/napi/src/lib.rs @@ -171,6 +171,14 @@ fn main(mut cx: ModuleContext) -> NeonResult<()> { "call_js_function_with_custom_this", call_js_function_with_custom_this, )?; + cx.export_function( + "call_js_function_with_implicit_this", + call_js_function_with_implicit_this, + )?; + cx.export_function( + "exec_js_function_with_implicit_this", + exec_js_function_with_implicit_this, + )?; cx.export_function( "call_js_function_with_heterogeneous_tuple", call_js_function_with_heterogeneous_tuple, @@ -180,6 +188,10 @@ fn main(mut cx: ModuleContext) -> NeonResult<()> { "construct_js_function_idiomatically", construct_js_function_idiomatically, )?; + cx.export_function( + "construct_js_function_with_overloaded_result", + construct_js_function_with_overloaded_result, + )?; cx.export_function("num_arguments", num_arguments)?; cx.export_function("return_this", return_this)?; cx.export_function("require_object_this", require_object_this)?;