Merge pull request #139 from DenisKolodin/vcomp-drop

Add ScopeHandle for component destroying

examples/dashboard/client/src/main.rs

@@ -6,8 +6,8 @@ extern crate yew;
 use yew::prelude::*;
 use yew::format::{Nothing, Json};
 use yew::services::Task;
-use yew::services::fetch::{FetchService, Request, Response};
-use yew::services::websocket::{WebSocketService, WebSocketHandle, WebSocketStatus};
+use yew::services::fetch::{FetchService, FetchTask, Request, Response};
+use yew::services::websocket::{WebSocketService, WebSocketTask, WebSocketStatus};
 
 struct Context {
     web: FetchService,
@@ -17,7 +17,8 @@ struct Context {
 struct Model {
     fetching: bool,
     data: Option<u32>,
-    ws: Option<WebSocketHandle>,
+    ft: Option<FetchTask>,
+    ws: Option<WebSocketTask>,
 }
 
 enum WsAction {
@@ -68,6 +69,7 @@ impl Component<Context> for Model {
         Model {
             fetching: false,
             data: None,
+            ft: None,
             ws: None,
         }
     }
@@ -85,7 +87,8 @@ impl Component<Context> for Model {
                     }
                 });
                 let request = Request::get("/data.json").body(Nothing).unwrap();
-                context.web.fetch(request, callback);
+                let task = context.web.fetch(request, callback);
+                self.ft = Some(task);
             }
             Msg::WsAction(action) => {
                 match action {
@@ -97,8 +100,8 @@ impl Component<Context> for Model {
                                 WebSocketStatus::Closed => WsAction::Lost.into(),
                             }
                         });
-                        let handle = context.ws.connect("ws://localhost:9001/", callback, notification);
-                        self.ws = Some(handle);
+                        let task = context.ws.connect("ws://localhost:9001/", callback, notification);
+                        self.ws = Some(task);
                     }
                     WsAction::SendData => {
                         let request = WsRequest {

examples/npm_and_rest/src/gravatar.rs

@@ -1,5 +1,5 @@
 use yew::format::{Nothing, Json};
-use yew::services::fetch::{FetchService, FetchHandle, Request, Response};
+use yew::services::fetch::{FetchService, FetchTask, Request, Response};
 use yew::html::Callback;
 
 #[derive(Deserialize, Debug)]
@@ -28,7 +28,7 @@ impl GravatarService {
         }
     }
 
-    pub fn profile(&mut self, hash: &str, callback: Callback<Result<Profile, ()>>) -> FetchHandle {
+    pub fn profile(&mut self, hash: &str, callback: Callback<Result<Profile, ()>>) -> FetchTask {
         let url = format!("https://gravatar.com/{}", hash);
         let handler = move |response: Response<Json<Result<Profile, ()>>>| {
             let (_, Json(data)) = response.into_parts();

examples/npm_and_rest/src/main.rs

@@ -6,6 +6,7 @@ extern crate stdweb;
 extern crate yew;
 
 use yew::prelude::*;
+use yew::services::fetch::FetchTask;
 
 // Own services implementation
 mod gravatar;
@@ -21,6 +22,7 @@ struct Context {
 struct Model {
     profile: Option<Profile>,
     exchanges: Vec<String>,
+    task: Option<FetchTask>,
 }
 
 enum Msg {
@@ -37,6 +39,7 @@ impl Component<Context> for Model {
         Model {
             profile: None,
             exchanges: Vec::new(),
+            task: None,
         }
     }
 
@@ -44,7 +47,8 @@ impl Component<Context> for Model {
         match msg {
             Msg::Gravatar => {
                 let callback = context.send_back(Msg::GravatarReady);
-                context.gravatar.profile("205e460b479e2e5b48aec07710c08d50", callback);
+                let task = context.gravatar.profile("205e460b479e2e5b48aec07710c08d50", callback);
+                self.task = Some(task);
             }
             Msg::GravatarReady(Ok(profile)) => {
                 self.profile = Some(profile);

src/html.rs

@@ -198,17 +198,21 @@ impl<CTX, COMP: Component<CTX>> Clone for ScopeSender<CTX, COMP> {
 impl<CTX, COMP: Component<CTX>> ScopeSender<CTX, COMP> {
     /// Send the message and schedule an update.
     pub fn send(&mut self, update: ComponentUpdate<CTX, COMP>) {
-        self.tx.send(update).expect("app lost the receiver!");
-        let bind = &self.bind;
-        js! { @(no_return)
-            // Schedule to call the loop handler
-            // IMPORTANT! If call loop function immediately
-            // it stops handling other messages and the first
-            // one will be fired.
-            var bind = @{bind};
-            // Put bind holder instad of callback function, because
-            // scope could be dropped and `loop` function will be changed
-            window._yew_schedule_(bind);
+        if let Ok(()) = self.tx.send(update) {
+            let bind = &self.bind;
+            js! { @(no_return)
+                // Schedule to call the loop handler
+                // IMPORTANT! If call loop function immediately
+                // it stops handling other messages and the first
+                // one will be fired.
+                var bind = @{bind};
+                // Put bind holder instad of callback function, because
+                // scope could be dropped and `loop` function will be changed
+                window._yew_schedule_(bind);
+            }
+        } else {
+            eprintln!("Can't send message to a component. Receiver lost! \
+                       Maybe Task lives longer than a component instance.");
         }
     }
 }
@@ -237,6 +241,13 @@ impl<CTX, COMP: Component<CTX>> ScopeBuilder<CTX, COMP> {
         }
     }
 
+    /// Return handler to a scope. Warning! Don't use more than one handle!
+    pub fn handle(&self) -> ScopeHandle {
+        ScopeHandle {
+            bind: self.bind.clone(),
+        }
+    }
+
     pub fn build(self, context: SharedContext<CTX>) -> Scope<CTX, COMP> {
         Scope {
             tx: self.tx,
@@ -306,7 +317,7 @@ where
     /// will render the model to a virtual DOM tree.
     pub fn mount(self, element: Element) {
         clear_element(&element);
-        self.mount_in_place(element, None, None, None);
+        self.mount_in_place(element, None, None, None)
     }
 
     // TODO Consider to use &Node instead of Element as parent
@@ -363,7 +374,26 @@ where
             var callback = @{callback};
             bind.loop = callback;
         }
-        // TODO `Drop` should drop the callback
+    }
+}
+
+/// This handle keeps the reference to a detached scope to prevent memory leaks.
+pub struct ScopeHandle {
+    bind: Value,
+}
+
+impl ScopeHandle {
+    /// Destroy the scope (component's loop).
+    pub fn destroy(self) {
+        let bind = &self.bind;
+        js! { @(no_return)
+            var destroy = function() {
+                var bind = @{bind};
+                bind.loop.drop();
+                bind.loop = function() { };
+            };
+            setTimeout(destroy, 0);
+        }
     }
 }
 

src/services/fetch.rs

@@ -26,7 +26,7 @@ enum FetchError {
 }
 
 /// A handle to control sent requests. Can be canceled with a `Task::cancel` call.
-pub struct FetchHandle(Option<Value>);
+pub struct FetchTask(Option<Value>);
 
 
 /// A service to fetch resources.
@@ -90,7 +90,7 @@ impl FetchService {
 
 
 
-    pub fn fetch<IN, OUT: 'static>(&mut self, request: Request<IN>, callback: Callback<Response<OUT>>) -> FetchHandle
+    pub fn fetch<IN, OUT: 'static>(&mut self, request: Request<IN>, callback: Callback<Response<OUT>>) -> FetchTask
     where
         IN: Into<Storable>,
         OUT: From<Restorable>,
@@ -170,11 +170,14 @@ impl FetchService {
             });
             return handle;
         };
-        FetchHandle(Some(handle))
+        FetchTask(Some(handle))
     }
 }
 
-impl Task for FetchHandle {
+impl Task for FetchTask {
+    fn is_active(&self) -> bool {
+        self.0.is_some()
+    }
     fn cancel(&mut self) {
         // Fetch API doesn't support request cancelling
         // and we should use this workaround with a flag.
@@ -187,3 +190,11 @@ impl Task for FetchHandle {
         }
     }
 }
+
+impl Drop for FetchTask {
+    fn drop(&mut self) {
+        if self.is_active() {
+            self.cancel();
+        }
+    }
+}

src/services/interval.rs

@@ -8,7 +8,7 @@ use super::{Task, to_ms};
 
 /// A handle which helps to cancel interval. Uses
 /// [clearInterval](https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/clearInterval).
-pub struct IntervalHandle(Option<Value>);
+pub struct IntervalTask(Option<Value>);
 
 /// A service to send messages on every elapsed interval.
 pub struct IntervalService {
@@ -22,7 +22,7 @@ impl IntervalService {
 
     /// Sets interval which will call send a messages returned by a converter
     /// on every intarval expiration.
-    pub fn spawn(&mut self, duration: Duration, callback: Callback<()>) -> IntervalHandle
+    pub fn spawn(&mut self, duration: Duration, callback: Callback<()>) -> IntervalTask
     {
         let callback = move || {
             callback.emit(());
@@ -39,11 +39,14 @@ impl IntervalService {
                 callback,
             };
         };
-        IntervalHandle(Some(handle))
+        IntervalTask(Some(handle))
     }
 }
 
-impl Task for IntervalHandle {
+impl Task for IntervalTask {
+    fn is_active(&self) -> bool {
+        self.0.is_some()
+    }
     fn cancel(&mut self) {
         let handle = self.0.take().expect("tried to cancel interval twice");
         js! { @(no_return)
@@ -53,3 +56,11 @@ impl Task for IntervalHandle {
         }
     }
 }
+
+impl Drop for IntervalTask {
+    fn drop(&mut self) {
+        if self.is_active() {
+            self.cancel();
+        }
+    }
+}

src/services/mod.rs

@@ -13,8 +13,11 @@ pub mod websocket;
 
 use std::time::Duration;
 
-/// An universal interface to service's routine. At least could be canceled.
-pub trait Task {
+/// An universal task of a service.
+/// It have to be canceled when dropped.
+pub trait Task: Drop {
+    /// Returns `true` if task is active.
+    fn is_active(&self) -> bool;
     /// Cancel current service's routine.
     fn cancel(&mut self);
 }

src/services/timeout.rs

@@ -7,7 +7,7 @@ use html::Callback;
 use super::{Task, to_ms};
 
 /// A handle to cancel a timeout task.
-pub struct TimeoutHandle(Option<Value>);
+pub struct TimeoutTask(Option<Value>);
 
 /// An service to set a timeout.
 pub struct TimeoutService {
@@ -20,7 +20,7 @@ impl TimeoutService {
     }
 
     /// Sets timeout which send a messages from a `converter` after `duration`.
-    pub fn spawn(&mut self, duration: Duration, callback: Callback<()>) -> TimeoutHandle {
+    pub fn spawn(&mut self, duration: Duration, callback: Callback<()>) -> TimeoutTask {
         let callback = move || {
             callback.emit(());
         };
@@ -37,11 +37,14 @@ impl TimeoutService {
                 callback,
             };
         };
-        TimeoutHandle(Some(handle))
+        TimeoutTask(Some(handle))
     }
 }
 
-impl Task for TimeoutHandle {
+impl Task for TimeoutTask {
+    fn is_active(&self) -> bool {
+        self.0.is_some()
+    }
     fn cancel(&mut self) {
         let handle = self.0.take().expect("tried to cancel timeout twice");
         js! { @(no_return)
@@ -51,3 +54,11 @@ impl Task for TimeoutHandle {
         }
     }
 }
+
+impl Drop for TimeoutTask {
+    fn drop(&mut self) {
+        if self.is_active() {
+            self.cancel();
+        }
+    }
+}

src/services/websocket.rs

@@ -15,7 +15,7 @@ pub enum WebSocketStatus {
 }
 
 /// A handle to control current websocket connection. Implements `Task` and could be canceled.
-pub struct WebSocketHandle(Option<Value>);
+pub struct WebSocketTask(Option<Value>);
 
 /// A websocket service attached to a user context.
 pub struct  WebSocketService {
@@ -29,7 +29,7 @@ impl WebSocketService {
 
     /// Connects to a server by a weboscket connection. Needs two functions to generate
     /// data and notification messages.
-    pub fn connect<OUT: 'static>(&mut self, url: &str, callback: Callback<OUT>, notification: Callback<WebSocketStatus>) -> WebSocketHandle
+    pub fn connect<OUT: 'static>(&mut self, url: &str, callback: Callback<OUT>, notification: Callback<WebSocketStatus>) -> WebSocketTask
     where
         OUT: From<Restorable>,
     {
@@ -69,17 +69,17 @@ impl WebSocketService {
                 socket,
             };
         };
-        WebSocketHandle(Some(handle))
+        WebSocketTask(Some(handle))
     }
 }
 
-impl WebSocketHandle {
+impl WebSocketTask {
     /// Sends data to a websocket connection.
     pub fn send<IN>(&mut self, data: IN)
     where
         IN: Into<Storable>
     {
-        if let WebSocketHandle(Some(ref handle)) = *self {
+        if let WebSocketTask(Some(ref handle)) = *self {
             if let Some(body) = data.into() {
                 js! { @(no_return)
                     var handle = @{handle};
@@ -92,7 +92,10 @@ impl WebSocketHandle {
     }
 }
 
-impl Task for WebSocketHandle {
+impl Task for WebSocketTask {
+    fn is_active(&self) -> bool {
+        self.0.is_some()
+    }
     fn cancel(&mut self) {
         let handle = self.0.take().expect("tried to close websocket twice");
         js! { @(no_return)
@@ -101,3 +104,11 @@ impl Task for WebSocketHandle {
         }
     }
 }
+
+impl Drop for WebSocketTask {
+    fn drop(&mut self) {
+        if self.is_active() {
+            self.cancel();
+        }
+    }
+}

src/virtual_dom/vcomp.rs

@@ -5,13 +5,14 @@ use std::cell::RefCell;
 use std::marker::PhantomData;
 use std::any::TypeId;
 use stdweb::web::{INode, Node, Element, document};
-use html::{ScopeBuilder, SharedContext, Component, Renderable, ComponentUpdate, ScopeSender, Callback, ScopeEnv, NodeCell};
+use html::{ScopeBuilder, SharedContext, Component, Renderable, ComponentUpdate, ScopeSender, Callback, ScopeEnv, NodeCell, ScopeHandle};
 use stdweb::unstable::TryInto;
 use super::{Reform, VDiff, VNode};
 
 struct Hidden;
 
 type AnyProps = (TypeId, *mut Hidden);
+type HandleCell = Rc<RefCell<Option<ScopeHandle>>>;
 
 /// A virtual component.
 pub struct VComp<CTX, COMP: Component<CTX>> {
@@ -21,6 +22,7 @@ pub struct VComp<CTX, COMP: Component<CTX>> {
     blind_sender: Box<FnMut(AnyProps)>,
     generator: Box<FnMut(SharedContext<CTX>, Element, Option<Node>, AnyProps)>,
     activators: Vec<Rc<RefCell<Option<ScopeSender<CTX, COMP>>>>>,
+    handle: Option<ScopeHandle>,
     _parent: PhantomData<COMP>,
 }
 
@@ -32,6 +34,7 @@ impl<CTX: 'static, COMP: Component<CTX>> VComp<CTX, COMP> {
     {
         let cell: NodeCell = Rc::new(RefCell::new(None));
         let builder: ScopeBuilder<CTX, CHILD> = ScopeBuilder::new();
+        let handle = Some(builder.handle());
         let mut sender = builder.sender();
         let mut builder = Some(builder);
         let occupied = cell.clone();
@@ -47,7 +50,8 @@ impl<CTX: 'static, COMP: Component<CTX>> VComp<CTX, COMP> {
 
             let builder = builder.take().expect("tried to mount component twice");
             let opposite = obsolete.map(VNode::VRef);
-            builder.build(context).mount_in_place(element, opposite, Some(occupied.clone()), Some(props));
+            builder.build(context)
+                .mount_in_place(element, opposite, Some(occupied.clone()), Some(props));
         };
         let mut previous_props = None;
         let blind_sender = move |(type_id, raw): AnyProps| {
@@ -74,6 +78,7 @@ impl<CTX: 'static, COMP: Component<CTX>> VComp<CTX, COMP> {
             blind_sender: Box::new(blind_sender),
             generator: Box::new(generator),
             activators: Vec::new(),
+            handle,
             _parent: PhantomData,
         };
         (properties, comp)
@@ -103,6 +108,7 @@ impl<CTX: 'static, COMP: Component<CTX>> VComp<CTX, COMP> {
         // Grab a sender and a cell (element's reference) to reuse it later
         self.blind_sender = other.blind_sender;
         self.cell = other.cell;
+        self.handle = other.handle;
     }
 }
 
@@ -196,6 +202,11 @@ where
 
     /// Remove VComp from parent.
     fn remove(self, parent: &Node) -> Option<Node> {
+        // Destroy the loop. It's impossible to use `Drop`,
+        // because parts can be reused with `grab_sender_of`.
+        if let Some(handle) = self.handle {
+            handle.destroy();
+        }
         // Keep the sibling in the cell and send a message `Drop` to a loop
         self.cell.borrow_mut().take().and_then(|node| {
             let sibling = node.next_sibling();

tests/vlist_test.rs

@@ -13,7 +13,7 @@ impl Component<Ctx> for Comp {
     type Msg = ();
     type Properties = ();
 
-    fn create(_: &mut Env<Ctx, Self>) -> Self {
+    fn create(_: Self::Properties, _: &mut Env<Ctx, Self>) -> Self {
         Comp
     }
 

tests/vtag_test.rs

@@ -12,7 +12,7 @@ impl Component<Ctx> for Comp {
     type Msg = ();
     type Properties = ();
 
-    fn create(_: &mut Env<Ctx, Self>) -> Self {
+    fn create(_: Self::Properties, _: &mut Env<Ctx, Self>) -> Self {
         Comp
     }