Browse Source

Merge pull request #775 from Demonthos/don't-reclaim-ids-when-dropping-a-scope

Fix duplicated work on dropping scopes
Jon Kelley 2 years ago
parent
commit
45abbb7a06

+ 13 - 29
packages/core/src/arena.rs

@@ -88,7 +88,9 @@ impl VirtualDom {
     }
 
     // Drop a scope and all its children
-    pub(crate) fn drop_scope(&mut self, id: ScopeId) {
+    //
+    // Note: This will not remove any ids from the arena
+    pub(crate) fn drop_scope(&mut self, id: ScopeId, recursive: bool) {
         self.dirty_scopes.remove(&DirtyScope {
             height: self.scopes[id.0].height,
             id,
@@ -96,19 +98,14 @@ impl VirtualDom {
 
         self.ensure_drop_safety(id);
 
-        if let Some(root) = self.scopes[id.0].as_ref().try_root_node() {
-            if let RenderReturn::Ready(node) = unsafe { root.extend_lifetime_ref() } {
-                self.drop_scope_inner(node)
-            }
-        }
-        if let Some(root) = unsafe { self.scopes[id.0].as_ref().previous_frame().try_load_node() } {
-            if let RenderReturn::Ready(node) = unsafe { root.extend_lifetime_ref() } {
-                self.drop_scope_inner(node)
+        if recursive {
+            if let Some(root) = self.scopes[id.0].as_ref().try_root_node() {
+                if let RenderReturn::Ready(node) = unsafe { root.extend_lifetime_ref() } {
+                    self.drop_scope_inner(node)
+                }
             }
         }
 
-        self.scopes[id.0].props.take();
-
         let scope = &mut self.scopes[id.0];
 
         // Drop all the hooks once the children are dropped
@@ -121,37 +118,24 @@ impl VirtualDom {
         for task_id in scope.spawned_tasks.borrow_mut().drain() {
             scope.tasks.remove(task_id);
         }
+
+        self.scopes.remove(id.0);
     }
 
     fn drop_scope_inner(&mut self, node: &VNode) {
-        node.clear_listeners();
         node.dynamic_nodes.iter().for_each(|node| match node {
             DynamicNode::Component(c) => {
                 if let Some(f) = c.scope.get() {
-                    self.drop_scope(f);
+                    self.drop_scope(f, true);
                 }
                 c.props.take();
             }
             DynamicNode::Fragment(nodes) => {
                 nodes.iter().for_each(|node| self.drop_scope_inner(node))
             }
-            DynamicNode::Placeholder(t) => {
-                if let Some(id) = t.id.get() {
-                    self.try_reclaim(id);
-                }
-            }
-            DynamicNode::Text(t) => {
-                if let Some(id) = t.id.get() {
-                    self.try_reclaim(id);
-                }
-            }
+            DynamicNode::Placeholder(_) => {}
+            DynamicNode::Text(_) => {}
         });
-
-        for id in &node.root_ids {
-            if id.0 != 0 {
-                self.try_reclaim(id);
-            }
-        }
     }
 
     /// Descend through the tree, removing any borrowed props and listeners

+ 1 - 1
packages/core/src/diff.rs

@@ -934,7 +934,7 @@ impl<'b> VirtualDom {
         *comp.props.borrow_mut() = unsafe { std::mem::transmute(props) };
 
         // Now drop all the resouces
-        self.drop_scope(scope);
+        self.drop_scope(scope, false);
     }
 
     fn find_first_element(&self, node: &'b VNode<'b>) -> ElementId {

+ 0 - 8
packages/core/src/nodes.rs

@@ -195,14 +195,6 @@ impl<'a> VNode<'a> {
             }
         }
     }
-
-    pub(crate) fn clear_listeners(&self) {
-        for attr in self.dynamic_attrs {
-            if let AttributeValue::Listener(l) = &attr.value {
-                l.borrow_mut().take();
-            }
-        }
-    }
 }
 
 /// A static layout of a UI tree that describes a set of dynamic and static nodes.

+ 1 - 1
packages/core/src/virtual_dom.rs

@@ -682,6 +682,6 @@ impl VirtualDom {
 impl Drop for VirtualDom {
     fn drop(&mut self) {
         // Simply drop this scope which drops all of its children
-        self.drop_scope(ScopeId(0));
+        self.drop_scope(ScopeId(0), true);
     }
 }