Apply suggestions from code review

Co-authored-by: Ivan Enderlin <ivan@mnt.io>

lib/js-api/src/externals/memory.rs

@@ -41,6 +41,19 @@ extern "C" {
     /// Takes the number of pages to grow (64KiB in size) and returns the
     /// previous size of memory, in pages.
     ///
+    /// # Reimplementation
+    ///
+    /// We re-implement `WebAssembly.Memory.grow` because it is
+    /// different from what `wasm-bindgen` declares. It marks the function
+    /// as `catch`, which means it can throw an exception.
+    ///
+    /// See [the opened patch](https://github.com/rustwasm/wasm-bindgen/pull/2599).
+    ///
+    /// # Exceptions
+    ///
+    /// A `RangeError` is thrown if adding pages would exceed the maximum
+    /// memory.
+    ///
     /// [MDN documentation](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly/Memory/grow)
     #[wasm_bindgen(catch, method, js_namespace = WebAssembly)]
     pub fn grow(this: &JSMemory, pages: u32) -> Result<u32, JsValue>;
@@ -141,7 +154,7 @@ impl Memory {
     /// modify the memory contents in any way including by calling a wasm
     /// function that writes to the memory or by resizing the memory.
     pub unsafe fn data_unchecked(&self) -> &[u8] {
-        unimplemented!("direct data pointer access is not possible in js");
+        unimplemented!("direct data pointer access is not possible in JavaScript");
     }
 
     /// Retrieve a mutable slice of the memory contents.
@@ -155,21 +168,20 @@ impl Memory {
     /// by resizing this Memory.
     #[allow(clippy::mut_from_ref)]
     pub unsafe fn data_unchecked_mut(&self) -> &mut [u8] {
-        unimplemented!("direct data pointer access is not possible in js");
+        unimplemented!("direct data pointer access is not possible in JavaScript");
     }
 
     /// Returns the pointer to the raw bytes of the `Memory`.
     pub fn data_ptr(&self) -> *mut u8 {
-        unimplemented!("direct data pointer access is not possible in js");
+        unimplemented!("direct data pointer access is not possible in JavaScript");
     }
 
     /// Returns the size (in bytes) of the `Memory`.
     pub fn data_size(&self) -> u64 {
-        let bytes = js_sys::Reflect::get(&self.vm_memory.memory.buffer(), &"byteLength".into())
+        js_sys::Reflect::get(&self.vm_memory.memory.buffer(), &"byteLength".into())
             .unwrap()
             .as_f64()
-            .unwrap() as u64;
-        return bytes;
+            .unwrap() as _
     }
 
     /// Returns the size (in [`Pages`]) of the `Memory`.

lib/js-api/src/instance.rs

@@ -3,7 +3,6 @@ use crate::exports::Exports;
 use crate::externals::Extern;
 use crate::module::Module;
 use crate::store::Store;
-// use crate::{HostEnvInitError, LinkError, RuntimeError};
 use crate::resolver::Resolver;
 use js_sys::WebAssembly;
 use std::fmt;
@@ -42,25 +41,8 @@ pub enum InstantiationError {
     /// A runtime error occured while invoking the start function
     #[cfg_attr(feature = "std", error("Start error: {0}"))]
     Start(String),
-    // /// Error occurred when initializing the host environment.
-    // #[error(transparent)]
-    // HostEnvInitialization(HostEnvInitError),
 }
 
-// impl From<wasmer_engine::InstantiationError> for InstantiationError {
-//     fn from(other: wasmer_engine::InstantiationError) -> Self {
-//         match other {
-//             wasmer_engine::InstantiationError::Link(e) => Self::Link(e),
-//             wasmer_engine::InstantiationError::Start(e) => Self::Start(e),
-//         }
-//     }
-// }
-
-// impl From<HostEnvInitError> for InstantiationError {
-//     fn from(other: HostEnvInitError) -> Self {
-//         Self::HostEnvInitialization(other)
-//     }
-// }
 
 impl Instance {
     /// Creates a new `Instance` from a WebAssembly [`Module`] and a

lib/js-api/src/lib.rs

@@ -133,8 +133,6 @@ pub use crate::types::{
 pub use crate::types::{Val as Value, ValType as Type};
 pub use crate::utils::is_wasm;
 
-// #[cfg(feature = "experimental-reference-types-extern-ref")]
-// pub use wasmer_types::ExternRef;
 pub use wasmer_types::{
     Atomically, Bytes, ExportIndex, GlobalInit, LocalFunctionIndex, MemoryView, Pages, ValueType,
     WASM_MAX_PAGES, WASM_MIN_PAGES, WASM_PAGE_SIZE,

lib/js-api/src/trap.rs

@@ -44,10 +44,6 @@ impl fmt::Display for RuntimeErrorSource {
     }
 }
 
-// fn _assert_trap_is_sync_and_send(t: &Trap) -> (&dyn Sync, &dyn Send) {
-//     (t, t)
-// }
-
 impl RuntimeError {
     /// Creates a new generic `RuntimeError` with the given `message`.
     ///

lib/js-api/src/types.rs

@@ -28,7 +28,7 @@ pub fn param_from_js(ty: &ValType, js_val: &JsValue) -> Val {
         ValType::I64 => Val::I64(js_val.as_f64().unwrap() as _),
         ValType::F32 => Val::F32(js_val.as_f64().unwrap() as _),
         ValType::F64 => Val::F64(js_val.as_f64().unwrap()),
-        _ => unimplemented!("The type is not yet supported in the JS Function API"),
+        t => unimplemented!("The type `{:?}` is not yet supported in the JS Function API", t),
     }
 }
 
@@ -40,7 +40,7 @@ impl AsJs for Val {
             Self::F32(f) => JsValue::from_f64(*f as f64),
             Self::F64(f) => JsValue::from_f64(*f),
             Self::FuncRef(func) => func.as_ref().unwrap().exported.function.clone().into(),
-            _ => unimplemented!("The type is not yet supported in the JS Function API"),
+            v => unimplemented!("The value `{:?}` is not yet supported in the JS Function API", v),
         }
     }
 }