From e3011d629e2f2f03cfed5d0da27bd8a37b1490c4 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Thu, 9 Aug 2018 13:08:30 -0700 Subject: [PATCH 1/4] js-sys: Promise methods should take JS things by shared reference --- crates/futures/src/lib.rs | 2 +- crates/js-sys/src/lib.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/futures/src/lib.rs b/crates/futures/src/lib.rs index 992bc4e1..41bdd4e3 100644 --- a/crates/futures/src/lib.rs +++ b/crates/futures/src/lib.rs @@ -54,7 +54,7 @@ //! pub fn new() -> NextTick { //! // Create a resolved promise that will run its callbacks on the next //! // tick of the micro task queue. -//! let promise = js_sys::Promise::resolve(JsValue::NULL); +//! let promise = js_sys::Promise::resolve(&JsValue::NULL); //! // Convert the promise into a `JsFuture`. //! let inner = JsFuture::from(promise); //! NextTick { inner } diff --git a/crates/js-sys/src/lib.rs b/crates/js-sys/src/lib.rs index 9452d0a8..25320c24 100644 --- a/crates/js-sys/src/lib.rs +++ b/crates/js-sys/src/lib.rs @@ -3109,7 +3109,7 @@ extern { /// /// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/all #[wasm_bindgen(static_method_of = Promise)] - pub fn all(obj: JsValue) -> Promise; + pub fn all(obj: &JsValue) -> Promise; /// The `Promise.race(iterable)` method returns a promise that resolves or /// rejects as soon as one of the promises in the iterable resolves or @@ -3117,14 +3117,14 @@ extern { /// /// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/race #[wasm_bindgen(static_method_of = Promise)] - pub fn race(obj: JsValue) -> Promise; + pub fn race(obj: &JsValue) -> Promise; /// The `Promise.reject(reason)` method returns a `Promise` object that is /// rejected with the given reason. /// /// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/reject #[wasm_bindgen(static_method_of = Promise)] - pub fn reject(obj: JsValue) -> Promise; + pub fn reject(obj: &JsValue) -> Promise; /// The `Promise.resolve(value)` method returns a `Promise` object that is /// resolved with the given value. If the value is a promise, that promise @@ -3134,7 +3134,7 @@ extern { /// /// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/resolve #[wasm_bindgen(static_method_of = Promise)] - pub fn resolve(obj: JsValue) -> Promise; + pub fn resolve(obj: &JsValue) -> Promise; /// The `catch()` method returns a `Promise` and deals with rejected cases /// only. It behaves the same as calling `Promise.prototype.then(undefined, From 96ad97a9f924d04c3ed56d98fb164ce1cfeaf444 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Thu, 9 Aug 2018 13:08:51 -0700 Subject: [PATCH 2/4] js-sys: Document that new bindings should take JS things by shared ref --- crates/js-sys/src/lib.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/js-sys/src/lib.rs b/crates/js-sys/src/lib.rs index 25320c24..b822f35e 100644 --- a/crates/js-sys/src/lib.rs +++ b/crates/js-sys/src/lib.rs @@ -31,17 +31,20 @@ use wasm_bindgen::prelude::*; // * Keep imports in alphabetical order. // // * Rename imports with `js_name = ...` according to the note about `camelCase` -// and `snake_case` in the module's documentation above. +// and `snake_case` in the module's documentation above. // // * Include the one sentence summary of the import from the MDN link in the -// module's documentation above, and the MDN link itself. +// module's documentation above, and the MDN link itself. // // * If a function or method can throw an exception, make it catchable by adding -// `#[wasm_bindgen(catch)]`. +// `#[wasm_bindgen(catch)]`. // // * Add a new `#[test]` into the appropriate file in the // `crates/js-sys/tests/wasm/` directory. If the imported function or method // can throw an exception, make sure to also add test coverage for that case. +// +// * Arguments that are `JsValue`s or imported JavaScript types should be taken +// by reference. #[wasm_bindgen] extern "C" { From f9ac4e9c90cd42243a45052fab1896c2cb097482 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Thu, 9 Aug 2018 16:17:34 -0700 Subject: [PATCH 3/4] Always bind static operations to their class For example, `Promise.resolve` must always be called with the `Promise` constructor as its `this`, or else it will throw an error. --- crates/cli-support/src/js/mod.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index e52e6721..eb28b4ad 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/crates/cli-support/src/js/mod.rs @@ -1868,24 +1868,28 @@ impl<'a, 'b> SubContext<'a, 'b> { ), } } else { - let location = if *is_static { "" } else { ".prototype" }; + let (location, binding) = if *is_static { + ("", format!(".bind({})", class)) + } else { + (".prototype", "".into()) + }; match kind { shared::OperationKind::Regular => { - format!("{}{}.{}", class, location, import.function.name) + format!("{}{}.{}{}", class, location, import.function.name, binding) } shared::OperationKind::Getter(g) => { self.cx.expose_get_inherited_descriptor(); format!( - "GetOwnOrInheritedPropertyDescriptor({}{}, '{}').get", - class, location, g, + "GetOwnOrInheritedPropertyDescriptor({}{}, '{}').get{}", + class, location, g, binding, ) } shared::OperationKind::Setter(s) => { self.cx.expose_get_inherited_descriptor(); format!( - "GetOwnOrInheritedPropertyDescriptor({}{}, '{}').set", - class, location, s, + "GetOwnOrInheritedPropertyDescriptor({}{}, '{}').set{}", + class, location, s, binding, ) } shared::OperationKind::IndexingGetter => panic!("indexing getter should be structural"), From ff835948828113251bf046c3fc92946f03be1f88 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Thu, 9 Aug 2018 16:21:12 -0700 Subject: [PATCH 4/4] futures: Add sanity tests for conversion between Promises and Futures Part of #614 --- .travis.yml | 4 +-- crates/futures/Cargo.toml | 3 +++ crates/futures/tests/tests.rs | 51 +++++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 2 deletions(-) create mode 100755 crates/futures/tests/tests.rs diff --git a/.travis.yml b/.travis.yml index 2721f4c3..bfd64e01 100644 --- a/.travis.yml +++ b/.travis.yml @@ -62,9 +62,9 @@ matrix: - cargo test --target wasm32-unknown-unknown --features serde-serialize # Make sure the `std` feature works if disabled - cargo test --target wasm32-unknown-unknown -p no-std - # Make sure the `wasm-bindgen-futures` tests pass. Right now, this just - # verifies that the example program in the crate level docs compiles. + # Make sure the `wasm-bindgen-futures` tests pass. - cargo test -p wasm-bindgen-futures + - cargo test -p wasm-bindgen-futures --target wasm32-unknown-unknown addons: firefox: latest if: branch = master diff --git a/crates/futures/Cargo.toml b/crates/futures/Cargo.toml index bffa5588..1c521f1d 100644 --- a/crates/futures/Cargo.toml +++ b/crates/futures/Cargo.toml @@ -7,3 +7,6 @@ authors = ["The wasm-bindgen Developers"] futures = "0.1.20" js-sys = { path = "../js-sys", version = '0.2.0' } wasm-bindgen = { path = "../..", version = '0.2.15' } + +[target.'cfg(target_arch = "wasm32")'.dev-dependencies] +wasm-bindgen-test = { path = '../test', version = '0.2.15' } diff --git a/crates/futures/tests/tests.rs b/crates/futures/tests/tests.rs new file mode 100755 index 00000000..a6225737 --- /dev/null +++ b/crates/futures/tests/tests.rs @@ -0,0 +1,51 @@ +#![feature(use_extern_macros)] +#![cfg(target_arch = "wasm32")] + +extern crate futures; +extern crate js_sys; +extern crate wasm_bindgen; +extern crate wasm_bindgen_futures; +extern crate wasm_bindgen_test; + +use futures::Future; +use wasm_bindgen::prelude::*; +use wasm_bindgen_futures::{future_to_promise, JsFuture}; +use wasm_bindgen_test::*; + +#[wasm_bindgen_test(async)] +fn promise_resolve_is_ok_future() -> impl Future { + let p = js_sys::Promise::resolve(&JsValue::from(42)); + JsFuture::from(p) + .map(|x| { + assert_eq!(x, 42); + }).map_err(|_| unreachable!()) +} + +#[wasm_bindgen_test(async)] +fn promise_reject_is_error_future() -> impl Future { + let p = js_sys::Promise::reject(&JsValue::from(42)); + JsFuture::from(p).map(|_| unreachable!()).or_else(|e| { + assert_eq!(e, 42); + Ok(()) + }) +} + +#[wasm_bindgen_test(async)] +fn ok_future_is_resolved_promise() -> impl Future { + let f = futures::future::ok(JsValue::from(42)); + let p = future_to_promise(f); + JsFuture::from(p) + .map(|x| { + assert_eq!(x, 42); + }).map_err(|_| unreachable!()) +} + +#[wasm_bindgen_test(async)] +fn error_future_is_rejected_promise() -> impl Future { + let f = futures::future::err(JsValue::from(42)); + let p = future_to_promise(f); + JsFuture::from(p).map(|_| unreachable!()).or_else(|e| { + assert_eq!(e, 42); + Ok(()) + }) +}