소스 검색

fix: remove nodes is in a happier state

Jonathan Kelley 3 년 전
부모
커밋
11f6b93889

+ 3 - 0
packages/core-macro/src/rsx/component.rs

@@ -135,11 +135,14 @@ impl ToTokens for Component {
             None => quote! { None },
         };
 
+        let fn_name = self.name.segments.last().unwrap().ident.to_string();
+
         tokens.append_all(quote! {
             __cx.component(
                 #name,
                 #builder,
                 #key_token,
+                #fn_name
             )
         })
     }

+ 25 - 12
packages/core/src/diff.rs

@@ -26,7 +26,10 @@ impl<'b> AsyncDiffState<'b> {
         self.scope_stack.push(scopeid);
         let scope = self.scopes.get_scope(scopeid).unwrap();
         self.element_stack.push(scope.container);
+
         self.diff_node(old, new);
+
+        self.mutations.mark_dirty_scope(scopeid);
     }
 
     pub fn diff_node(&mut self, old_node: &'b VNode<'b>, new_node: &'b VNode<'b>) {
@@ -87,13 +90,6 @@ impl<'b> AsyncDiffState<'b> {
                 self.diff_fragment_nodes(old, new)
             }
 
-            // The normal pathway still works, but generates slightly weird instructions
-            // This pathway ensures uses the ReplaceAll, not the InsertAfter and remove
-            (Placeholder(_), Fragment(_)) => {
-                log::debug!("replacing placeholder with fragment {:?}", new_node);
-                self.replace_node(old_node, new_node);
-            }
-
             // Anything else is just a basic replace and create
             (
                 Component(_) | Fragment(_) | Text(_) | Element(_) | Placeholder(_),
@@ -201,7 +197,8 @@ impl<'b> AsyncDiffState<'b> {
         };
 
         log::info!(
-            "created component {:?} with parent {:?} and originator {:?}",
+            "created component \"{}\", id: {:?} parent {:?} orig: {:?}",
+            vcomponent.fn_name,
             new_idx,
             parent_idx,
             vcomponent.originator
@@ -442,7 +439,7 @@ impl<'b> AsyncDiffState<'b> {
     // to an element, and appending makes sense.
     fn diff_children(&mut self, old: &'b [VNode<'b>], new: &'b [VNode<'b>]) {
         if std::ptr::eq(old, new) {
-            log::debug!("skipping fragment diff - fragment is the same");
+            log::debug!("skipping fragment diff - fragment is the same {:?}", old);
             return;
         }
 
@@ -499,7 +496,7 @@ impl<'b> AsyncDiffState<'b> {
         // );
 
         for (new, old) in new.iter().zip(old.iter()) {
-            log::debug!("diffing nonkeyed {:#?} {:#?}", old, new);
+            // log::debug!("diffing nonkeyed {:#?} {:#?}", old, new);
             self.diff_node(old, new);
         }
     }
@@ -849,10 +846,15 @@ impl<'b> AsyncDiffState<'b> {
             }
 
             VNode::Component(c) => {
-                let node = self.scopes.fin_head(c.scope.get().unwrap());
+                log::info!("Replacing component {:?}", old);
+                let scope_id = c.scope.get().unwrap();
+                let node = self.scopes.fin_head(scope_id);
+
+                self.enter_scope(scope_id);
+
                 self.replace_inner(node, nodes_created);
 
-                let scope_id = c.scope.get().unwrap();
+                log::info!("Replacing component x2 {:?}", old);
 
                 // we can only remove components if they are actively being diffed
                 if self.scope_stack.contains(&c.originator) {
@@ -860,6 +862,7 @@ impl<'b> AsyncDiffState<'b> {
 
                     self.scopes.try_remove(scope_id).unwrap();
                 }
+                self.leave_scope();
             }
         }
     }
@@ -902,15 +905,25 @@ impl<'b> AsyncDiffState<'b> {
                 }
 
                 VNode::Component(c) => {
+                    self.enter_scope(c.scope.get().unwrap());
+
                     let scope_id = c.scope.get().unwrap();
                     let root = self.scopes.root_node(scope_id);
                     self.remove_nodes([root], gen_muts);
 
+                    log::info!(
+                        "trying to remove scope {:?}, stack is {:#?}, originator is {:?}",
+                        scope_id,
+                        self.scope_stack,
+                        c.originator
+                    );
+
                     // we can only remove this node if the originator is actively
                     if self.scope_stack.contains(&c.originator) {
                         log::debug!("I am allowed to remove component because scope stack contains originator. Scope stack: {:#?} {:#?} {:#?}", self.scope_stack, c.originator, c.scope);
                         self.scopes.try_remove(scope_id).unwrap();
                     }
+                    self.leave_scope();
                 }
             }
         }

+ 3 - 3
packages/core/src/mutations.rs

@@ -17,7 +17,7 @@ use std::{any::Any, fmt::Debug};
 /// Mutations are the only link between the RealDOM and the VirtualDOM.
 pub struct Mutations<'a> {
     pub edits: Vec<DomEdit<'a>>,
-    pub dirty_scopes: FxHashSet<ScopeId>,
+    pub diffed_scopes: FxHashSet<ScopeId>,
     pub refs: Vec<NodeRefMutation<'a>>,
 }
 
@@ -118,7 +118,7 @@ impl<'a> Mutations<'a> {
         Self {
             edits: Vec::new(),
             refs: Vec::new(),
-            dirty_scopes: Default::default(),
+            diffed_scopes: Default::default(),
         }
     }
 
@@ -223,7 +223,7 @@ impl<'a> Mutations<'a> {
     }
 
     pub(crate) fn mark_dirty_scope(&mut self, scope: ScopeId) {
-        self.dirty_scopes.insert(scope);
+        self.diffed_scopes.insert(scope);
     }
 }
 

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

@@ -184,6 +184,7 @@ impl Debug for VNode<'_> {
             }
             VNode::Component(comp) => s
                 .debug_struct("VNode::VComponent")
+                .field("name", &comp.fn_name)
                 .field("fnptr", &comp.user_fc)
                 .field("key", &comp.key)
                 .field("scope", &comp.scope)
@@ -390,6 +391,7 @@ pub struct VComponent<'src> {
     pub scope: Cell<Option<ScopeId>>,
     pub can_memoize: bool,
     pub user_fc: FcSlot,
+    pub fn_name: &'static str,
     pub props: RefCell<Option<Box<dyn AnyProps + 'src>>>,
 }
 
@@ -540,6 +542,7 @@ impl<'a> NodeFactory<'a> {
         component: fn(Scope<'a, P>) -> Element,
         props: P,
         key: Option<Arguments>,
+        fn_name: &'static str,
     ) -> VNode<'a>
     where
         P: Properties + 'a,
@@ -550,6 +553,7 @@ impl<'a> NodeFactory<'a> {
             can_memoize: P::IS_STATIC,
             user_fc: component as *mut std::os::raw::c_void,
             originator: self.scope.scope_id(),
+            fn_name,
             props: RefCell::new(Some(Box::new(VComponentProps {
                 // local_props: RefCell::new(Some(props)),
                 // heap_props: RefCell::new(None),

+ 17 - 7
packages/core/src/scopes.rs

@@ -113,13 +113,22 @@ impl ScopeArena {
         if let Some(old_scope) = self.free_scopes.borrow_mut().pop() {
             // reuse the old scope
             let scope = unsafe { &mut *old_scope };
-            scope.props.get_mut().replace(vcomp);
+
+            scope.container = container;
+            scope.our_arena_idx = new_scope_id;
             scope.parent_scope = parent_scope;
             scope.height = height;
-            scope.subtree.set(subtree);
-            scope.our_arena_idx = new_scope_id;
-            scope.container = container;
             scope.fnptr = fc_ptr;
+            scope.props.get_mut().replace(vcomp);
+            scope.subtree.set(subtree);
+            scope.frames[0].reset();
+            scope.frames[1].reset();
+            scope.shared_contexts.get_mut().clear();
+            scope.items.get_mut().listeners.clear();
+            scope.items.get_mut().borrowed_props.clear();
+            scope.hook_idx.set(0);
+            scope.hook_vals.get_mut().clear();
+
             let any_item = self.scopes.borrow_mut().insert(new_scope_id, scope);
             debug_assert!(any_item.is_none());
         } else {
@@ -177,7 +186,7 @@ impl ScopeArena {
         let scope = unsafe { &mut *self.scopes.borrow_mut().remove(&id).unwrap() };
         scope.reset();
 
-        self.free_scopes.borrow_mut().push(scope);
+        // self.free_scopes.borrow_mut().push(scope);
 
         Some(())
     }
@@ -204,8 +213,9 @@ impl ScopeArena {
     }
 
     pub fn collect_garbage(&self, id: ElementId) {
-        let node = self.nodes.borrow_mut().remove(id.0);
-        log::debug!("collecting garbage for {:?}, {:?}", id, unsafe { &*node });
+        let node = self.nodes.borrow_mut().get(id.0).unwrap().clone();
+        // let node = self.nodes.borrow_mut().remove(id.0);
+        // log::debug!("collecting garbage for {:?}, {:?}", id, unsafe { &*node });
     }
 
     /// This method cleans up any references to data held within our hook list. This prevents mutable aliasing from

+ 2 - 12
packages/core/src/virtual_dom.rs

@@ -462,14 +462,6 @@ impl VirtualDom {
             self.dirty_scopes
                 .retain(|id| scopes.get_scope(*id).is_some());
 
-            log::debug!("dirty_scopes: {:?}", self.dirty_scopes);
-
-            for id in &self.dirty_scopes {
-                let scope = scopes.get_scope(*id).unwrap();
-
-                log::debug!("dirty scope: {:?} with height {:?}", id, scope.height);
-            }
-
             self.dirty_scopes.sort_by(|a, b| {
                 let h1 = scopes.get_scope(*a).unwrap().height;
                 let h2 = scopes.get_scope(*b).unwrap().height;
@@ -479,10 +471,7 @@ impl VirtualDom {
             log::debug!("dirty_scopes: {:?}", self.dirty_scopes);
 
             if let Some(scopeid) = self.dirty_scopes.pop() {
-                log::debug!("diffing scope {:?}", scopeid);
                 if !ran_scopes.contains(&scopeid) {
-                    log::debug!("running scope scope {:?}", scopeid);
-
                     ran_scopes.insert(scopeid);
 
                     self.scopes.run_scope(scopeid);
@@ -491,7 +480,8 @@ impl VirtualDom {
 
                     let AsyncDiffState { mutations, .. } = diff_state;
 
-                    for scope in &mutations.dirty_scopes {
+                    log::debug!("succesffuly resolved scopes {:?}", mutations.diffed_scopes);
+                    for scope in &mutations.diffed_scopes {
                         self.dirty_scopes.remove(scope);
                     }