Mark js_sys::Promise as #[must_use]

[silvanshade]

I originally opened an issue about this at rust-lang/rust-clippy#4936. Please look there for the rationale.

[Pauan]

To be clear, JS Promises are always eager, which means they always run immediately. So even if you immediately discard the Promise, the computation (and side effects) will still happen. So there are legitimate reasons for discarding Promises.

Having said that, I'm in favor of this change, since it helps to prevent bugs (especially bugs with error handling), and users can always do let _ = some_promise(); if they want to intentionally discard it.

[silvanshade]

Thanks for clarifying that! The few places I had to silence the warnings after this change make more sense to me now.

[alexcrichton]

Sounds reasonable to me, thanks @darinmorrison!

[zachreizner]

I recently ran into this warning about having to use a Promise. My code was already working, but I decided I should fix the warning as I always do, just like with Result. However, unlike Result, there is nothing I can really do with the Promise at this point that wouldn't just generate more Promise objects. I already called then2 to handle resolve and reject, but that method returns yet another Promise. I always considered using let _ = as an exception to handling must_use return values. In other words, I would have a good reason why I'm choosing to ignore errors. In the case of Promise, this is no longer the exception but the rule. I will always have to end my Promise by assigning to let _ = or suffer the compiler warning. To me, using let _ is a bit of a code smell and this #[muse_use] is causing them to propagate.

Or perhaps I'm missing something and there is something better to do with my unused Promises? Perhaps add that suggestion to the #[must_use] attribute so that I can find out what that is.

[silvanshade]

I don't think there is anything wrong with using let _ = ... for the cases that need it. However, if you are having to use it excessively, it may be that there are better ways to structure your code to avoid that. If you have some examples to share it would be easier to provide concrete suggestions.

[zachreizner]

Sure. I have my (simplified) module using fetch below:

use std::sync::Arc;
use wasm_bindgen::{JsCast, JsValue};
use wasm_bindgen::closure::Closure;
use web_sys::console;

/// Used to fetch buffers at a fixed URL, using web_sys.
pub struct Fetcher {
    base_url: String,
}

impl Fetcher {
    pub fn new(base_url: String) -> Self {
        Self { base_url }
    }

    pub fn fetch<F>(&self, label: &str, finish: F)
    where
        F: 'static + FnOnce(Arc<[u8]>),
    {
        let window = match web_sys::window() {
            Some(w) => w,
            None => return,
        };

        let url = format!("{}/{}", self.base_url, label);
        console::log_2(&"url".into(), &url.clone().into());

        let resolve = Closure::once(move |resp: JsValue| {
            console::log_1(&"resolved".into());
            let resp: web_sys::Response = resp.dyn_into().unwrap();
            let resolve = Closure::once(move |array: JsValue| {
                let array = js_sys::Uint8Array::new(&array);
                finish(array.to_vec().into());
            });
            let _ = resp.array_buffer().unwrap().then(&resolve);
            resolve.forget();
        });

        let reject = Closure::once(move |resp: JsValue| {
            console::log_2(&"rejected".into(), &resp);
        });
        let _ = window.fetch_with_str(&url).then2(&resolve, &reject);
        resolve.forget();
        reject.forget();
    }
}

[silvanshade]

@zachreizner Thanks for providing an example.

I think part of the issue here is that programming directly with Promise is very low level and not very natural in Rust with regard to control flow.

Here is an attempt to rewrite your example using async, JsFuture (from wasm-bindgen-futures) and the ? operator. It does not use let _ = ... and instead of using the finish callback, it just returns the result. I haven't tested it, but it compiles and hopefully gives an idea of an alternative approach.

use wasm_bindgen::{prelude::*, JsCast, JsValue};
use wasm_bindgen_futures::JsFuture;
use web_sys::console;

/// Used to fetch buffers at a fixed URL, using web_sys.
pub struct Fetcher {
    base_url: String,
}

impl Fetcher {
    pub fn new(base_url: String) -> Self {
        Self { base_url }
    }

    pub async fn fetch<F>(&self, label: &str) -> Result<js_sys::Uint8Array, JsValue> {
        let window = web_sys::window().unwrap_throw();

        let url = format!("{}/{}", self.base_url, label);
        console::log_2(&"url".into(), &url.clone().into());

        let promise = window.fetch_with_str(&url);
        let future = JsFuture::from(promise);

        match future.await {
            Ok(resp) => {
                console::log_1(&"resolved".into());
                let resp: web_sys::Response = resp.dyn_into().unwrap();
                let buffer = resp.array_buffer()?;
                let buffer = JsFuture::from(buffer).await?;
                let array = js_sys::Uint8Array::new(&buffer);
                Ok(array)
            }
            Err(resp) => {
                console::log_2(&"rejected".into(), &resp);
                Err(resp)
            }
        }
    }
}

[zachreizner]

@darinmorrison Thanks for responding to my example so thoroughly.

My original version of this fetch method actually did use JsFuture, although yours is much cleaner and mine wasn't an async function. Instead mine had used spawn_local to drive the future. Sadly this did not work because of an issue with winit::EventLoop.

If I were to follow your example, I'm unsure how I would actually go about executing the async future without spawn_local, which seems broken in the linked issue. Perhaps this is just consequence of living on the bleeding edge of the Rust/Web ecosystem.