Refactor and simplify Callback (#2301)

* yeet Callback::once * yeet Callback::reform * remove passive overrides Event's passive state will be determined by sane defaults set by Yew * clippy * docs * make CI happy * Revert "yeet Callback::reform" This reverts commit 33e7770 * why? I literally tick-marked reformat before commit. Does it not use rustfmt??? * Update packages/yew/src/callback.rs Co-authored-by: Julius Lungys <[email protected]> Co-authored-by: Julius Lungys <[email protected]>
yewstack · Dec 28, 2021 · 3758e6d · 3758e6d
1 parent eab8857
commit 3758e6d
Show file tree

Hide file tree

Showing 10 changed files with 79 additions and 272 deletions.
diff --git a/examples/function_todomvc/src/components/entry.rs b/examples/function_todomvc/src/components/entry.rs
@@ -37,21 +37,29 @@ pub fn entry(props: &EntryProps) -> Html {
         class.push("completed");
     }
 
+    let ontoggle = {
+        let ontoggle = props.ontoggle.clone();
+        move |_| ontoggle.emit(id)
+    };
+
+    let onremove = {
+        let onremove = props.onremove.clone();
+        move |_| onremove.emit(id)
+    };
+
     html! {
         <li {class}>
             <div class="view">
                 <input
                     type="checkbox"
                     class="toggle"
                     checked={props.entry.completed}
-                    onclick={props.ontoggle.reform(move |_| id)}
+                    onclick={ontoggle}
                 />
-                <label ondblclick={Callback::once(move |_| {
-                    edit_toggle.toggle();
-                })}>
+                <label ondblclick={move |_| edit_toggle.clone().toggle()}>
                     { &props.entry.description }
                 </label>
-                <button class="destroy" onclick={props.onremove.reform(move |_| id)} />
+                <button class="destroy" onclick={onremove} />
             </div>
             <EntryEdit entry={props.entry.clone()} onedit={props.onedit.clone()} editing={is_editing} />
         </li>

diff --git a/examples/function_todomvc/src/components/filter.rs b/examples/function_todomvc/src/components/filter.rs
@@ -18,11 +18,16 @@ pub fn Filter(props: &FilterProps) -> Html {
         "not-selected"
     };
 
+    let onset_filter = {
+        let onset_filter = props.onset_filter.clone();
+        move |_| onset_filter.emit(filter)
+    };
+
     html! {
         <li>
             <a class={cls}
                href={props.filter.as_href()}
-               onclick={props.onset_filter.reform(move |_| filter)}
+               onclick={onset_filter}
             >
                 { props.filter }
             </a>

diff --git a/examples/function_todomvc/src/hooks/use_bool_toggle.rs b/examples/function_todomvc/src/hooks/use_bool_toggle.rs
@@ -1,9 +1,10 @@
 use std::ops::Deref;
 use std::rc::Rc;
-use yew::functional::use_hook;
+use yew::{use_state_eq, UseStateHandle};
 
+#[derive(Clone)]
 pub struct UseBoolToggleHandle {
-    value: bool,
+    value: UseStateHandle<bool>,
     toggle: Rc<dyn Fn()>,
 }
 
@@ -34,37 +35,28 @@ impl Deref for UseBoolToggleHandle {
 /// ...
 /// let value = use_bool_toggle(false);
 /// ...
-/// <button onclick={Callback::once(move |_| {
-///     value.toggle();
-///     // This will toggle the value to true.
-///     // Then render.
-///     // Post render it will toggle back to false skipping the render.
-/// })}>
+/// let onclick = {
+///     let value = value.clone();
+///     move |_| {
+///         value.toggle();
+///         // This will toggle the value to true.
+///         // Then render.
+///         // Post render it will toggle back to false skipping the render.
+///     }
+/// }
+/// <button {onclick}>{ "Click me" }</button>
 /// ...
 /// ```
 pub fn use_bool_toggle(default: bool) -> UseBoolToggleHandle {
-    use_hook(
-        || default,
-        move |hook, updater| {
-            updater.post_render(move |state: &mut bool| {
-                if *state != default {
-                    *state = default;
-                }
-                false
-            });
+    let state = use_state_eq(|| default);
 
-            let toggle = Rc::new(move || {
-                updater.callback(move |st: &mut bool| {
-                    *st = !*st;
-                    true
-                })
-            });
+    let toggle = {
+        let state = state.clone();
+        Rc::new(move || state.set(!*state))
+    };
 
-            UseBoolToggleHandle {
-                value: *hook,
-                toggle,
-            }
-        },
-        |_| {},
-    )
+    UseBoolToggleHandle {
+        value: state,
+        toggle,
+    }
 }
diff --git a/packages/yew-agent/src/link.rs b/packages/yew-agent/src/link.rs
@@ -91,28 +91,6 @@ impl<AGN: Agent> AgentLink<AGN> {
         closure.into()
     }
 
-    /// This method creates a [`Callback`] from [`FnOnce`] which returns a Future
-    /// which returns a message to be sent back to the agent.
-    ///
-    /// # Panics
-    /// If the future panics, then the promise will not resolve, and
-    /// will leak.
-    pub fn callback_future_once<FN, FU, IN, M>(&self, function: FN) -> Callback<IN>
-    where
-        M: Into<AGN::Message>,
-        FU: Future<Output = M> + 'static,
-        FN: FnOnce(IN) -> FU + 'static,
-    {
-        let link = self.clone();
-
-        let closure = move |input: IN| {
-            let future: FU = function(input);
-            link.send_future(future);
-        };
-
-        Callback::once(closure)
-    }
-
     /// This method processes a Future that returns a message and sends it back to the agent.
     ///
     /// # Panics

diff --git a/packages/yew/src/callback.rs b/packages/yew/src/callback.rs
@@ -5,7 +5,6 @@
 //! - [Timer](https://github.com/yewstack/yew/tree/master/examples/timer)
 
 use crate::html::ImplicitClone;
-use std::cell::RefCell;
 use std::fmt;
 use std::rc::Rc;
 
@@ -16,104 +15,51 @@ use std::rc::Rc;
 /// Callbacks should be used from JS callbacks or `setTimeout` calls.
 /// </aside>
 /// An `Rc` wrapper is used to make it cloneable.
-pub enum Callback<IN> {
-    /// A callback which can be called multiple times with optional modifier flags
-    Callback {
-        /// A callback which can be called multiple times
-        cb: Rc<dyn Fn(IN)>,
-
-        /// Setting `passive` to [Some] explicitly makes the event listener passive or not.
-        /// Yew sets sane defaults depending on the type of the listener.
-        /// See
-        /// [addEventListener](https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener).
-        passive: Option<bool>,
-    },
-
-    /// A callback which can only be called once. The callback will panic if it is
-    /// called more than once.
-    CallbackOnce(Rc<CallbackOnce<IN>>),
+pub struct Callback<IN, OUT = ()> {
+    /// A callback which can be called multiple times
+    pub(crate) cb: Rc<dyn Fn(IN) -> OUT>,
 }
 
-type CallbackOnce<IN> = RefCell<Option<Box<dyn FnOnce(IN)>>>;
-
-impl<IN, F: Fn(IN) + 'static> From<F> for Callback<IN> {
+impl<IN, OUT, F: Fn(IN) -> OUT + 'static> From<F> for Callback<IN, OUT> {
     fn from(func: F) -> Self {
-        Callback::Callback {
-            cb: Rc::new(func),
-            passive: None,
-        }
+        Callback { cb: Rc::new(func) }
     }
 }
 
-impl<IN> Clone for Callback<IN> {
+impl<IN, OUT> Clone for Callback<IN, OUT> {
     fn clone(&self) -> Self {
-        match self {
-            Callback::Callback { cb, passive } => Callback::Callback {
-                cb: cb.clone(),
-                passive: *passive,
-            },
-            Callback::CallbackOnce(cb) => Callback::CallbackOnce(cb.clone()),
+        Self {
+            cb: self.cb.clone(),
         }
     }
 }
 
 #[allow(clippy::vtable_address_comparisons)]
-impl<IN> PartialEq for Callback<IN> {
-    fn eq(&self, other: &Callback<IN>) -> bool {
-        match (&self, &other) {
-            (Callback::CallbackOnce(cb), Callback::CallbackOnce(other_cb)) => {
-                Rc::ptr_eq(cb, other_cb)
-            }
-            (
-                Callback::Callback { cb, passive },
-                Callback::Callback {
-                    cb: rhs_cb,
-                    passive: rhs_passive,
-                },
-            ) => Rc::ptr_eq(cb, rhs_cb) && passive == rhs_passive,
-            _ => false,
-        }
+impl<IN, OUT> PartialEq for Callback<IN, OUT> {
+    fn eq(&self, other: &Callback<IN, OUT>) -> bool {
+        let (Callback { cb }, Callback { cb: rhs_cb }) = (self, other);
+        Rc::ptr_eq(cb, rhs_cb)
     }
 }
 
-impl<IN> fmt::Debug for Callback<IN> {
+impl<IN, OUT> fmt::Debug for Callback<IN, OUT> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let data = match self {
-            Callback::Callback { .. } => "Callback<_>",
-            Callback::CallbackOnce(_) => "CallbackOnce<_>",
-        };
-
-        f.write_str(data)
+        write!(f, "Callback<_>")
     }
 }
 
-impl<IN> Callback<IN> {
+impl<IN, OUT> Callback<IN, OUT> {
     /// This method calls the callback's function.
-    pub fn emit(&self, value: IN) {
-        match self {
-            Callback::Callback { cb, .. } => cb(value),
-            Callback::CallbackOnce(rc) => {
-                let cb = rc.replace(None);
-                let f = cb.expect("callback contains `FnOnce` which has already been used");
-                f(value)
-            }
-        };
-    }
-
-    /// Creates a callback from an `FnOnce`. The programmer is responsible for ensuring
-    /// that the callback is only called once. If it is called more than once, the callback
-    /// will panic.
-    pub fn once<F>(func: F) -> Self
-    where
-        F: FnOnce(IN) + 'static,
-    {
-        Callback::CallbackOnce(Rc::new(RefCell::new(Some(Box::new(func)))))
+    pub fn emit(&self, value: IN) -> OUT {
+        (*self.cb)(value)
     }
+}
 
+impl<IN> Callback<IN> {
     /// Creates a "no-op" callback which can be used when it is not suitable to use an
     /// `Option<Callback>`.
     pub fn noop() -> Self {
-        Self::from(|_| {})
+        Self::from(|_| ())
     }
 }
 
@@ -123,9 +69,9 @@ impl<IN> Default for Callback<IN> {
     }
 }
 
-impl<IN: 'static> Callback<IN> {
-    /// Changes the input type of the callback to another.
-    /// Works like the `map` method but in the opposite direction.
+impl<IN: 'static, OUT: 'static> Callback<IN, OUT> {
+    /// Creates a new callback from another callback and a function
+    /// That when emited will call that function and will emit the original callback
     pub fn reform<F, T>(&self, func: F) -> Callback<T>
     where
         F: Fn(T) -> IN + 'static,
@@ -139,4 +85,4 @@ impl<IN: 'static> Callback<IN> {
     }
 }
 
-impl<T> ImplicitClone for Callback<T> {}
+impl<IN, OUT> ImplicitClone for Callback<IN, OUT> {}