fix: clearTimeout illegal invocation with bundler (#187) (#283)

* fix: `clearTimeout` illegal invocation in browser (#187)

* fix: return type of `Timeout/Interval` accepts both number and `NodeJS.Timeout`

* chore: simplify clearTimeout/clearInterval binding

refer #283 (comment) for background

* CI: setup node tests

Author: flisky

.github/workflows/tests.yml

@@ -76,6 +76,43 @@ jobs:
             wasm-pack test --headless --firefox --chrome crates/$x --no-default-features
           done
 
+  node_tests:
+    name: Node Tests
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - uses: actions-rs/toolchain@v1
+        with:
+          toolchain: stable
+          target: wasm32-unknown-unknown
+          override: true
+          profile: minimal
+
+      - name: Install wasm-pack
+        run: curl https://rustwasm.github.io/wasm-pack/installer/init.sh -sSf | sh
+
+      - uses: actions/cache@v3
+        with:
+          path: |
+            ~/.cargo/registry
+            ~/.cargo/git
+            target
+          key: cargo-${{ runner.os }}-node-tests-${{ hashFiles('**/Cargo.toml') }}
+          restore-keys: |
+            cargo-${{ runner.os }}-node-tests-
+            cargo-${{ runner.os }}-
+
+      - name: Run tests
+        run: |
+          for x in $(ls crates); do
+            # gloo-net is tested separately
+            if [[ "$x" == "net" ]]; then
+              continue
+            fi
+            wasm-pack test --node crates/$x --all-features
+            wasm-pack test --node crates/$x --no-default-features
+          done
+
   test-worker:
     name: Test gloo-worker
     runs-on: ubuntu-latest

CONTRIBUTING.md

@@ -17,6 +17,7 @@ yourself.
   - [Building](#building)
   - [Testing](#testing)
     - [Wasm Headless Browser Tests](#wasm-headless-browser-tests)
+    - [Wasm Node Tests](#wasm-node-tests)
     - [Non-Wasm Tests](#non-wasm-tests)
   - [Formatting](#formatting)
   - [Updating `README.md`s](#updating-readmemds)
@@ -81,6 +82,14 @@ To run headless browser tests for a particular crate:
 wasm-pack test crates/my-particular-crate --headless --firefox # or --safari or --chrome
 ```
 
+#### Wasm Node Tests
+
+To run tests in Node.js:
+
+```shell
+wasm-pack test crates/my-particular-crate --node
+```
+
 #### Non-Wasm Tests
 
 You can run the non-Wasm tests (e.g. doc tests) for every Gloo crate with:

crates/timers/src/callback.rs

@@ -7,16 +7,16 @@ use wasm_bindgen::{JsCast, JsValue};
 #[wasm_bindgen]
 extern "C" {
     #[wasm_bindgen(js_name = "setTimeout", catch)]
-    fn set_timeout(handler: &Function, timeout: i32) -> Result<i32, JsValue>;
+    fn set_timeout(handler: &Function, timeout: i32) -> Result<JsValue, JsValue>;
 
     #[wasm_bindgen(js_name = "setInterval", catch)]
-    fn set_interval(handler: &Function, timeout: i32) -> Result<i32, JsValue>;
+    fn set_interval(handler: &Function, timeout: i32) -> Result<JsValue, JsValue>;
 
     #[wasm_bindgen(js_name = "clearTimeout")]
-    fn clear_timeout(handle: i32);
+    fn clear_timeout(handle: JsValue) -> JsValue;
 
     #[wasm_bindgen(js_name = "clearInterval")]
-    fn clear_interval(handle: i32);
+    fn clear_interval(handle: JsValue) -> JsValue;
 }
 
 /// A scheduled timeout.
@@ -28,15 +28,15 @@ extern "C" {
 #[derive(Debug)]
 #[must_use = "timeouts cancel on drop; either call `forget` or `drop` explicitly"]
 pub struct Timeout {
-    id: Option<i32>,
+    id: Option<JsValue>,
     closure: Option<Closure<dyn FnMut()>>,
 }
 
 impl Drop for Timeout {
     /// Disposes of the timeout, dually cancelling this timeout by calling
     /// `clearTimeout` directly.
     fn drop(&mut self) {
-        if let Some(id) = self.id {
+        if let Some(id) = self.id.take() {
             clear_timeout(id);
         }
     }
@@ -90,7 +90,7 @@ impl Timeout {
     ///     // Do stuff...
     /// }).forget();
     /// ```
-    pub fn forget(mut self) -> i32 {
+    pub fn forget(mut self) -> JsValue {
         let id = self.id.take().unwrap_throw();
         self.closure.take().unwrap_throw().forget();
         id
@@ -130,15 +130,15 @@ impl Timeout {
 #[derive(Debug)]
 #[must_use = "intervals cancel on drop; either call `forget` or `drop` explicitly"]
 pub struct Interval {
-    id: Option<i32>,
+    id: Option<JsValue>,
     closure: Option<Closure<dyn FnMut()>>,
 }
 
 impl Drop for Interval {
     /// Disposes of the interval, dually cancelling this interval by calling
     /// `clearInterval` directly.
     fn drop(&mut self) {
-        if let Some(id) = self.id {
+        if let Some(id) = self.id.take() {
             clear_interval(id);
         }
     }
@@ -190,7 +190,7 @@ impl Interval {
     ///     // Do stuff...
     /// }).forget();
     /// ```
-    pub fn forget(mut self) -> i32 {
+    pub fn forget(mut self) -> JsValue {
         let id = self.id.take().unwrap_throw();
         self.closure.take().unwrap_throw().forget();
         id

crates/timers/tests/node.rs

@@ -0,0 +1,114 @@
+#![cfg(all(target_family = "wasm", feature = "futures"))]
+
+use futures_channel::{mpsc, oneshot};
+use futures_util::{
+    future::{select, Either, FutureExt},
+    stream::StreamExt,
+};
+use gloo_timers::{
+    callback::{Interval, Timeout},
+    future::{sleep, IntervalStream, TimeoutFuture},
+};
+use std::cell::Cell;
+use std::rc::Rc;
+use std::time::Duration;
+use wasm_bindgen_test::*;
+
+#[wasm_bindgen_test]
+async fn timeout() {
+    let (sender, receiver) = oneshot::channel();
+    Timeout::new(1, || sender.send(()).unwrap()).forget();
+    receiver.await.unwrap();
+}
+
+#[wasm_bindgen_test]
+async fn timeout_cancel() {
+    let cell = Rc::new(Cell::new(false));
+
+    let t = Timeout::new(1, {
+        let cell = cell.clone();
+        move || {
+            cell.set(true);
+            panic!("should have been cancelled");
+        }
+    });
+    t.cancel();
+
+    let (sender, receiver) = oneshot::channel();
+
+    Timeout::new(2, move || {
+        sender.send(()).unwrap();
+        assert_eq!(cell.get(), false);
+    })
+    .forget();
+
+    receiver.await.unwrap();
+}
+
+#[wasm_bindgen_test]
+async fn timeout_future() {
+    TimeoutFuture::new(1).await;
+}
+
+#[wasm_bindgen_test]
+async fn timeout_future_cancel() {
+    let cell = Rc::new(Cell::new(false));
+
+    let a = TimeoutFuture::new(1).map({
+        let cell = cell.clone();
+        move |_| {
+            assert_eq!(cell.get(), false);
+            1
+        }
+    });
+
+    let b = TimeoutFuture::new(2).map({
+        let cell = cell.clone();
+        move |_| {
+            cell.set(true);
+            2u32
+        }
+    });
+
+    let (who, other) = match select(a, b).await {
+        Either::Left(x) => x,
+        Either::Right(_) => panic!("Timer for 2 ms finished before timer for 1 ms"),
+    };
+    assert_eq!(who, 1);
+    // Drop `b` so that its timer is canceled.
+    drop(other);
+    TimeoutFuture::new(3).await;
+    // We should never have fired `b`'s timer.
+    assert_eq!(cell.get(), false);
+}
+
+#[wasm_bindgen_test]
+async fn interval() {
+    let (mut sender, receiver) = mpsc::channel(1);
+    let i = Interval::new(1, move || {
+        if !sender.is_closed() {
+            sender.try_send(()).unwrap()
+        }
+    });
+
+    let results: Vec<_> = receiver.take(5).collect().await;
+    drop(i);
+    assert_eq!(results.len(), 5);
+}
+
+#[wasm_bindgen_test]
+async fn interval_cancel() {
+    let i = Interval::new(10, move || {
+        panic!("This should never be called");
+    });
+    i.cancel();
+
+    // This keeps us live for long enough that if any erroneous Interval callbacks fired, we'll have seen them.
+    sleep(Duration::from_millis(100)).await;
+}
+
+#[wasm_bindgen_test]
+async fn interval_stream() {
+    let results: Vec<_> = IntervalStream::new(1).take(5).collect().await;
+    assert_eq!(results.len(), 5);
+}

crates/timers/tests/web.rs

@@ -1,6 +1,4 @@
-//! Test suite for the Web and headless browsers.
-
-#![cfg(all(target_arch = "wasm32", feature = "futures"))]
+#![cfg(all(target_family = "wasm", feature = "futures"))]
 
 use futures_channel::{mpsc, oneshot};
 use futures_util::{