Do not detach child elements if parent element is about to be detached (#2420)

* Make a use_hook hook with the new Hook trait.

* Implement Lifetime.

* Rewrites function signature.

* Only apply lifetime if there're other lifetimes.

* Cleanup signature rewrite logic.

* Rewrite hook body.

* Port some built-in hooks.

* Finish porting all built-in hooks.

* Port tests.

* Fix tests.

* Migrate to macro-based hooks.

* Fix HookContext, add tests on non-possible locations.

* Fix stderr for trybuild.

* Add 1 more test case.

* Adjust doc location.

* Pretty print hook signature.

* Fix Items & std::ops::Fn*.

* Add use_memo.

* Optimise Implementation of hooks.

* Use Box to capture function value only.

* Detect whether needs boxing.

* Add args if boxing not needed.

* Enforce hook number.

* Deduplicate use_effect.

* Optimise Implementation.

* Update documentation.

* Fix website test. Strip BoxedHook implementation from it.

* Allow doc string.

* Workaround doc tests.

* Optimise codebase & documentation.

* Fix website test.

* Reduce implementation complexity.

* Destructor is no more.

* Do not detach child element if parent element is about to be detached as well.

* Fix tests.

packages/yew/src/app_handle.rs

@@ -35,7 +35,7 @@ where
 
     /// Schedule the app for destruction
     pub fn destroy(mut self) {
-        self.scope.destroy()
+        self.scope.destroy(false)
     }
 }
 

packages/yew/src/html/component/lifecycle.rs

@@ -179,6 +179,7 @@ impl<COMP: BaseComponent> Runnable for UpdateRunner<COMP> {
 
 pub(crate) struct DestroyRunner<COMP: BaseComponent> {
     pub(crate) state: Shared<Option<ComponentState<COMP>>>,
+    pub(crate) parent_to_detach: bool,
 }
 
 impl<COMP: BaseComponent> Runnable for DestroyRunner<COMP> {
@@ -190,7 +191,7 @@ impl<COMP: BaseComponent> Runnable for DestroyRunner<COMP> {
             state.component.destroy(&state.context);
 
             if let Some(ref m) = state.parent {
-                state.root_node.detach(m);
+                state.root_node.detach(m, self.parent_to_detach);
                 state.node_ref.set(None);
             }
         }

packages/yew/src/html/component/scope.rs

@@ -115,7 +115,7 @@ impl AnyScope {
 pub(crate) trait Scoped {
     fn to_any(&self) -> AnyScope;
     fn root_vnode(&self) -> Option<Ref<'_, VNode>>;
-    fn destroy(&mut self);
+    fn destroy(&mut self, parent_to_detach: bool);
     fn shift_node(&self, parent: Element, next_sibling: NodeRef);
 }
 
@@ -136,9 +136,10 @@ impl<COMP: BaseComponent> Scoped for Scope<COMP> {
     }
 
     /// Process an event to destroy a component
-    fn destroy(&mut self) {
+    fn destroy(&mut self, parent_to_detach: bool) {
         scheduler::push_component_destroy(DestroyRunner {
             state: self.state.clone(),
+            parent_to_detach,
         });
         // Not guaranteed to already have the scheduler started
         scheduler::start();
@@ -387,6 +388,7 @@ mod feat_ssr {
 
             scheduler::push_component_destroy(DestroyRunner {
                 state: self.state.clone(),
+                parent_to_detach: false,
             });
             scheduler::start();
         }

packages/yew/src/virtual_dom/mod.rs

@@ -497,7 +497,9 @@ impl Default for Attributes {
 /// This trait provides features to update a tree by calculating a difference against another tree.
 pub(crate) trait VDiff {
     /// Remove self from parent.
-    fn detach(&mut self, parent: &Element);
+    ///
+    /// Parent to detach is `true` if the parent element will also be detached.
+    fn detach(&mut self, parent: &Element, parent_to_detach: bool);
 
     /// Move elements from one parent to another parent.
     /// This is currently only used by `VSuspense` to preserve component state without detaching

packages/yew/src/virtual_dom/vcomp.rs

@@ -243,8 +243,8 @@ impl<COMP: BaseComponent> Mountable for PropsWrapper<COMP> {
 }
 
 impl VDiff for VComp {
-    fn detach(&mut self, _parent: &Element) {
-        self.take_scope().destroy();
+    fn detach(&mut self, _parent: &Element, parent_to_detach: bool) {
+        self.take_scope().destroy(parent_to_detach);
     }
 
     fn shift(&self, _previous_parent: &Element, next_parent: &Element, next_sibling: NodeRef) {
@@ -276,7 +276,7 @@ impl VDiff for VComp {
                 }
             }
 
-            ancestor.detach(parent);
+            ancestor.detach(parent, false);
         }
 
         self.scope = Some(mountable.mount(
@@ -595,7 +595,7 @@ mod tests {
         elem.apply(&scope, &parent, NodeRef::default(), None);
         let parent_node = parent.deref();
         assert_eq!(node_ref.get(), parent_node.first_child());
-        elem.detach(&parent);
+        elem.detach(&parent, false);
         assert!(node_ref.get().is_none());
     }
 }

packages/yew/src/virtual_dom/vlist.rs

@@ -154,7 +154,7 @@ impl VList {
         while diff < 0 {
             let mut r = rights_it.next().unwrap();
             test_log!("removing: {:?}", r);
-            r.detach(parent);
+            r.detach(parent, false);
             diff += 1;
         }
 
@@ -268,7 +268,7 @@ impl VList {
         // Remove any extra rights
         for (_, (mut r, _)) in rights_diff.drain() {
             test_log!("removing: {:?}", r);
-            r.detach(parent);
+            r.detach(parent, false);
         }
 
         // Diff matching children at the start
@@ -307,9 +307,9 @@ mod feat_ssr {
 }
 
 impl VDiff for VList {
-    fn detach(&mut self, parent: &Element) {
+    fn detach(&mut self, parent: &Element, parent_to_detach: bool) {
         for mut child in self.children.drain(..) {
-            child.detach(parent);
+            child.detach(parent, parent_to_detach);
         }
     }
 

packages/yew/src/virtual_dom/vnode.rs

@@ -126,19 +126,19 @@ impl VNode {
 
 impl VDiff for VNode {
     /// Remove VNode from parent.
-    fn detach(&mut self, parent: &Element) {
+    fn detach(&mut self, parent: &Element, parent_to_detach: bool) {
         match *self {
-            VNode::VTag(ref mut vtag) => vtag.detach(parent),
-            VNode::VText(ref mut vtext) => vtext.detach(parent),
-            VNode::VComp(ref mut vcomp) => vcomp.detach(parent),
-            VNode::VList(ref mut vlist) => vlist.detach(parent),
+            VNode::VTag(ref mut vtag) => vtag.detach(parent, parent_to_detach),
+            VNode::VText(ref mut vtext) => vtext.detach(parent, parent_to_detach),
+            VNode::VComp(ref mut vcomp) => vcomp.detach(parent, parent_to_detach),
+            VNode::VList(ref mut vlist) => vlist.detach(parent, parent_to_detach),
             VNode::VRef(ref node) => {
                 if parent.remove_child(node).is_err() {
                     console::warn!("Node not found to remove VRef");
                 }
             }
-            VNode::VPortal(ref mut vportal) => vportal.detach(parent),
-            VNode::VSuspense(ref mut vsuspense) => vsuspense.detach(parent),
+            VNode::VPortal(ref mut vportal) => vportal.detach(parent, parent_to_detach),
+            VNode::VSuspense(ref mut vsuspense) => vsuspense.detach(parent, parent_to_detach),
         }
     }
 
@@ -183,12 +183,13 @@ impl VDiff for VNode {
             }
             VNode::VRef(ref mut node) => {
                 if let Some(mut ancestor) = ancestor {
+                    // We always remove VRef in case it's meant to be used somewhere else.
                     if let VNode::VRef(n) = &ancestor {
                         if node == n {
                             return NodeRef::new(node.clone());
                         }
                     }
-                    ancestor.detach(parent);
+                    ancestor.detach(parent, false);
                 }
                 super::insert_node(node, parent, next_sibling.get().as_ref());
                 NodeRef::new(node.clone())

packages/yew/src/virtual_dom/vportal.rs

@@ -17,8 +17,8 @@ pub struct VPortal {
 }
 
 impl VDiff for VPortal {
-    fn detach(&mut self, _: &Element) {
-        self.node.detach(&self.host);
+    fn detach(&mut self, _: &Element, _parent_to_detach: bool) {
+        self.node.detach(&self.host, false);
         self.sibling_ref.set(None);
     }
 
@@ -43,7 +43,7 @@ impl VDiff for VPortal {
                 } = old_portal;
                 if old_host != self.host {
                     // Remount the inner node somewhere else instead of diffing
-                    node.detach(&old_host);
+                    node.detach(&old_host, false);
                     None
                 } else if old_sibling != self.next_sibling {
                     // Move the node, but keep the state
@@ -54,7 +54,7 @@ impl VDiff for VPortal {
                 }
             }
             Some(mut node) => {
-                node.detach(parent);
+                node.detach(parent, false);
                 None
             }
             None => None,

packages/yew/src/virtual_dom/vsuspense.rs

@@ -48,14 +48,14 @@ impl VSuspense {
 }
 
 impl VDiff for VSuspense {
-    fn detach(&mut self, parent: &Element) {
+    fn detach(&mut self, parent: &Element, parent_to_detach: bool) {
         if self.suspended {
-            self.fallback.detach(parent);
+            self.fallback.detach(parent, parent_to_detach);
             if let Some(ref m) = self.detached_parent {
-                self.children.detach(m);
+                self.children.detach(m, false);
             }
         } else {
-            self.children.detach(parent);
+            self.children.detach(parent, parent_to_detach);
         }
     }
 
@@ -82,7 +82,7 @@ impl VDiff for VSuspense {
             Some(VNode::VSuspense(mut m)) => {
                 // We only preserve the child state if they are the same suspense.
                 if m.key != self.key || self.detached_parent != m.detached_parent {
-                    m.detach(parent);
+                    m.detach(parent, false);
 
                     (false, None, None)
                 } else {
@@ -90,7 +90,7 @@ impl VDiff for VSuspense {
                 }
             }
             Some(mut m) => {
-                m.detach(parent);
+                m.detach(parent, false);
                 (false, None, None)
             }
             None => (false, None, None),
@@ -136,7 +136,7 @@ impl VDiff for VSuspense {
             }
 
             (false, true) => {
-                fallback_ancestor.unwrap().detach(parent);
+                fallback_ancestor.unwrap().detach(parent, false);
 
                 children_ancestor.as_ref().unwrap().shift(
                     detached_parent,

packages/yew/src/virtual_dom/vtag.rs

@@ -476,7 +476,7 @@ impl VTag {
 
 impl VDiff for VTag {
     /// Remove VTag from parent.
-    fn detach(&mut self, parent: &Element) {
+    fn detach(&mut self, parent: &Element, parent_to_detach: bool) {
         let node = self
             .reference
             .take()
@@ -486,10 +486,15 @@ impl VDiff for VTag {
 
         // recursively remove its children
         if let VTagInner::Other { children, .. } = &mut self.inner {
-            children.detach(&node);
+            // This tag will be removed, so there's no point to remove any child.
+            children.detach(&node, true);
         }
-        if parent.remove_child(&node).is_err() {
-            console::warn!("Node not found to remove VTag");
+        if !parent_to_detach {
+            let result = parent.remove_child(&node);
+
+            if result.is_err() {
+                console::warn!("Node not found to remove VTag");
+            }
         }
         // It could be that the ref was already reused when rendering another element.
         // Only unset the ref it still belongs to our node
@@ -555,7 +560,7 @@ impl VDiff for VTag {
                 } else {
                     let el = self.create_element(parent);
                     super::insert_node(&el, parent, ancestor.first_node().as_ref());
-                    ancestor.detach(parent);
+                    ancestor.detach(parent, false);
                     (None, el)
                 }
             }
@@ -605,7 +610,7 @@ impl VDiff for VTag {
                         if !new.is_empty() {
                             new.apply(parent_scope, &el, NodeRef::default(), Some(old.into()));
                         } else if !old.is_empty() {
-                            old.detach(&el);
+                            old.detach(&el, false);
                         }
                     }
                     // Can not happen, because we checked for tag equability above
@@ -1191,7 +1196,7 @@ mod tests {
         elem.apply(&scope, &parent, NodeRef::default(), None);
         let parent_node = parent.deref();
         assert_eq!(node_ref.get(), parent_node.first_child());
-        elem.detach(&parent);
+        elem.detach(&parent, false);
         assert!(node_ref.get().is_none());
     }
 

packages/yew/src/virtual_dom/vtext.rs

@@ -55,13 +55,17 @@ impl std::fmt::Debug for VText {
 
 impl VDiff for VText {
     /// Remove VText from parent.
-    fn detach(&mut self, parent: &Element) {
+    fn detach(&mut self, parent: &Element, parent_to_detach: bool) {
         let node = self
             .reference
             .take()
             .expect("tried to remove not rendered VText from DOM");
-        if parent.remove_child(&node).is_err() {
-            console::warn!("Node not found to remove VText");
+        if !parent_to_detach {
+            let result = parent.remove_child(&node);
+
+            if result.is_err() {
+                console::warn!("Node not found to remove VText");
+            }
         }
     }
 
@@ -99,7 +103,7 @@ impl VDiff for VText {
                 return NodeRef::new(text_node.into());
             }
 
-            ancestor.detach(parent);
+            ancestor.detach(parent, false);
         }
 
         let text_node = document().create_text_node(&self.text);