Przeglądaj źródła

fix: all the bugs!

Jonathan Kelley 3 lat temu
rodzic
commit
478255f

+ 8 - 4
packages/core/src/context.rs

@@ -191,7 +191,7 @@ impl<'src> Context<'src> {
     ///     rsx!(cx, div { "hello {state.0}" })
     /// }
     /// ```
-    pub fn provide_state<T>(self, value: T) -> Option<Rc<T>>
+    pub fn provide_state<T>(self, value: T)
     where
         T: 'static,
     {
@@ -200,7 +200,7 @@ impl<'src> Context<'src> {
             .borrow_mut()
             .insert(TypeId::of::<T>(), Rc::new(value))
             .map(|f| f.downcast::<T>().ok())
-            .flatten()
+            .flatten();
     }
 
     /// Try to retrive a SharedState with type T from the any parent Scope.
@@ -279,13 +279,17 @@ impl<'src> Context<'src> {
         Output: 'src,
         Init: FnOnce(usize) -> State,
         Run: FnOnce(&'src mut State) -> Output,
-        Cleanup: FnOnce(&mut State) + 'static,
+        Cleanup: FnOnce(Box<State>) + 'static,
     {
         // If the idx is the same as the hook length, then we need to add the current hook
         if self.scope.hooks.at_end() {
             self.scope.hooks.push_hook(
                 initializer(self.scope.hooks.len()),
-                Box::new(|raw| cleanup(raw.downcast_mut::<State>().unwrap())),
+                Box::new(|raw| {
+                    //
+                    let s = raw.downcast::<State>().unwrap();
+                    cleanup(s);
+                }),
             );
         }
 

+ 23 - 31
packages/core/src/diff.rs

@@ -221,18 +221,12 @@ impl<'bump> DiffMachine<'bump> {
             MountType::Replace { old } => {
                 if let Some(old_id) = old.try_mounted_id() {
                     self.mutations.replace_with(old_id, nodes_created as u32);
+                    self.remove_nodes(Some(old));
                 } else {
-                    let mut iter = RealChildIterator::new(old, self.vdom);
-                    let first = iter.next().unwrap();
-                    self.mutations
-                        .replace_with(first.mounted_id(), nodes_created as u32);
-                    self.remove_nodes(iter);
-                }
-            }
-
-            MountType::ReplaceByElementId { el } => {
-                if let Some(old) = el {
-                    self.mutations.replace_with(old, nodes_created as u32);
+                    if let Some(id) = self.find_first_element_id(old) {
+                        self.mutations.replace_with(id, nodes_created as u32);
+                    }
+                    self.remove_nodes(Some(old));
                 }
             }
 
@@ -376,6 +370,8 @@ impl<'bump> DiffMachine<'bump> {
 
         let new_component = self.vdom.get_scope_mut(new_idx).unwrap();
 
+        log::debug!("initializing component {:?}", new_idx);
+
         // Run the scope for one iteration to initialize it
         if new_component.run_scope(self.vdom) {
             // Take the node that was just generated from running the component
@@ -413,7 +409,7 @@ impl<'bump> DiffMachine<'bump> {
             (Fragment(old), Fragment(new)) => self.diff_fragment_nodes(old, new),
             (Anchor(old), Anchor(new)) => new.dom_id.set(old.dom_id.get()),
             (Suspended(old), Suspended(new)) => self.diff_suspended_nodes(old, new),
-            (Element(old), Element(new)) => self.diff_element_nodes(old, new, new_node),
+            (Element(old), Element(new)) => self.diff_element_nodes(old, new, old_node, new_node),
 
             // Anything else is just a basic replace and create
             (
@@ -439,6 +435,7 @@ impl<'bump> DiffMachine<'bump> {
         &mut self,
         old: &'bump VElement<'bump>,
         new: &'bump VElement<'bump>,
+        old_node: &'bump VNode<'bump>,
         new_node: &'bump VNode<'bump>,
     ) {
         let root = old.dom_id.get().unwrap();
@@ -452,9 +449,7 @@ impl<'bump> DiffMachine<'bump> {
             // issue is that we need the "vnode" but this method only has the velement
             self.stack.push_nodes_created(0);
             self.stack.push(DiffInstruction::Mount {
-                and: MountType::ReplaceByElementId {
-                    el: old.dom_id.get(),
-                },
+                and: MountType::Replace { old: old_node },
             });
             self.create_element_node(new, new_node);
             return;
@@ -546,6 +541,7 @@ impl<'bump> DiffMachine<'bump> {
 
         // Make sure we're dealing with the same component (by function pointer)
         if old.user_fc == new.user_fc {
+            log::debug!("Diffing component {:?} - {:?}", new.user_fc, scope_addr);
             //
             self.stack.scope_stack.push(scope_addr);
 
@@ -554,7 +550,6 @@ impl<'bump> DiffMachine<'bump> {
 
             // make sure the component's caller function is up to date
             let scope = self.vdom.get_scope_mut(scope_addr).unwrap();
-
             scope.update_scope_dependencies(new.caller, ScopeChildren(new.children));
 
             // React doesn't automatically memoize, but we do.
@@ -569,8 +564,6 @@ impl<'bump> DiffMachine<'bump> {
             }
 
             self.stack.scope_stack.pop();
-
-            self.seen_scopes.insert(scope_addr);
         } else {
             self.stack
                 .create_node(new_node, MountType::Replace { old: old_node });
@@ -624,17 +617,14 @@ impl<'bump> DiffMachine<'bump> {
                 self.stack.create_children(new, MountType::Append);
             }
             (_, []) => {
-                for node in old {
-                    self.remove_nodes(Some(node));
-                }
+                self.remove_nodes(old);
             }
             ([VNode::Anchor(old_anchor)], [VNode::Anchor(new_anchor)]) => {
                 old_anchor.dom_id.set(new_anchor.dom_id.get());
             }
-            ([VNode::Anchor(anchor)], _) => {
-                let el = anchor.dom_id.get();
+            ([VNode::Anchor(_)], _) => {
                 self.stack
-                    .create_children(new, MountType::ReplaceByElementId { el });
+                    .create_children(new, MountType::Replace { old: &old[0] });
             }
             (_, [VNode::Anchor(_)]) => {
                 self.replace_and_create_many_with_one(old, &new[0]);
@@ -743,13 +733,13 @@ impl<'bump> DiffMachine<'bump> {
             Some(count) => count,
             None => return,
         };
-        log::debug!(
-            "Left offset, right offset, {}, {}",
-            left_offset,
-            right_offset,
-        );
+        // log::debug!(
+        //     "Left offset, right offset, {}, {}",
+        //     left_offset,
+        //     right_offset,
+        // );
 
-        log::debug!("stack before lo is {:#?}", self.stack.instructions);
+        // log::debug!("stack before lo is {:#?}", self.stack.instructions);
         // Ok, we now hopefully have a smaller range of children in the middle
         // within which to re-order nodes with the same keys, remove old nodes with
         // now-unused keys, and create new nodes with fresh keys.
@@ -1126,9 +1116,11 @@ impl<'bump> DiffMachine<'bump> {
 
                 VNode::Component(c) => {
                     let scope_id = c.associated_scope.get().unwrap();
-                    let scope = self.vdom.get_scope(scope_id).unwrap();
+                    let scope = self.vdom.get_scope_mut(scope_id).unwrap();
                     let root = scope.root_node();
                     self.remove_nodes(Some(root));
+                    let mut s = self.vdom.try_remove(scope_id).unwrap();
+                    s.hooks.clear_hooks();
                 }
             }
         }

+ 0 - 1
packages/core/src/diff_stack.rs

@@ -30,7 +30,6 @@ pub enum MountType<'a> {
     Absorb,
     Append,
     Replace { old: &'a VNode<'a> },
-    ReplaceByElementId { el: Option<ElementId> },
     InsertAfter { other_node: &'a VNode<'a> },
     InsertBefore { other_node: &'a VNode<'a> },
 }

+ 9 - 7
packages/core/src/hooklist.rs

@@ -12,7 +12,7 @@ use std::{
 /// Todo: this could use its very own bump arena, but that might be a tad overkill
 #[derive(Default)]
 pub(crate) struct HookList {
-    vals: RefCell<Vec<(UnsafeCell<Box<dyn Any>>, Box<dyn FnOnce(&mut dyn Any)>)>>,
+    vals: RefCell<Vec<(UnsafeCell<Box<dyn Any>>, Box<dyn FnOnce(Box<dyn Any>)>)>>,
     idx: Cell<usize>,
 }
 
@@ -35,7 +35,7 @@ impl HookList {
         self.idx.set(0);
     }
 
-    pub(crate) fn push_hook<T: 'static>(&self, new: T, cleanup: Box<dyn FnOnce(&mut dyn Any)>) {
+    pub(crate) fn push_hook<T: 'static>(&self, new: T, cleanup: Box<dyn FnOnce(Box<dyn Any>)>) {
         self.vals
             .borrow_mut()
             .push((UnsafeCell::new(Box::new(new)), cleanup))
@@ -52,14 +52,16 @@ impl HookList {
     pub(crate) fn at_end(&self) -> bool {
         self.cur_idx() >= self.len()
     }
-}
 
-// When the scope is dropped, we want to call the cleanup function for each of the hooks
-impl Drop for HookList {
-    fn drop(&mut self) {
+    pub fn clear_hooks(&mut self) {
         self.vals
             .borrow_mut()
             .drain(..)
-            .for_each(|(mut state, cleanup)| cleanup(state.get_mut()));
+            .for_each(|(mut state, cleanup)| {
+                //
+                let s: Box<dyn Any> = state.into_inner();
+                cleanup(s)
+                //
+            });
     }
 }

+ 2 - 2
packages/core/src/resources.rs

@@ -63,8 +63,8 @@ impl ResourcePool {
 
     /// return the id, freeing the space of the original node
     pub fn collect_garbage(&self, id: ElementId) {
-        todo!("garabge collection currently WIP")
-        // self.raw_elements.remove(id.0);
+        let els = unsafe { &mut *self.raw_elements.get() };
+        els.remove(id.0);
     }
 
     pub fn insert_scope_with_key(&self, f: impl FnOnce(ScopeId) -> Scope) -> ScopeId {

+ 23 - 12
packages/core/src/scheduler.rs

@@ -185,7 +185,11 @@ impl Scheduler {
             sender: sender.clone(),
             schedule_any_immediate: {
                 let sender = sender.clone();
-                Rc::new(move |id| sender.unbounded_send(SchedulerMsg::Immediate(id)).unwrap())
+                Rc::new(move |id| {
+                    //
+                    log::debug!("scheduling immedate! {:?}", id);
+                    sender.unbounded_send(SchedulerMsg::Immediate(id)).unwrap()
+                })
             },
             // todo: we want to get the futures out of the scheduler message
             // the scheduler message should be send/sync
@@ -374,23 +378,28 @@ impl Scheduler {
         if machine.stack.is_empty() {
             let shared = self.pool.clone();
 
+            self.dirty_scopes
+                .retain(|id| shared.get_scope(*id).is_some());
             self.dirty_scopes.sort_by(|a, b| {
                 let h1 = shared.get_scope(*a).unwrap().height;
                 let h2 = shared.get_scope(*b).unwrap().height;
-                h1.cmp(&h2)
+                h1.cmp(&h2).reverse()
             });
 
             if let Some(scopeid) = self.dirty_scopes.pop() {
-                log::info!("handlng dirty scope {:#?}", scopeid);
+                log::info!("handlng dirty scope {:?}", scopeid);
                 if !ran_scopes.contains(&scopeid) {
                     ran_scopes.insert(scopeid);
-
-                    let mut component = self.pool.get_scope_mut(scopeid).unwrap();
-                    if component.run_scope(&self.pool) {
-                        let (old, new) = (component.frames.wip_head(), component.frames.fin_head());
-                        // let (old, new) = (component.frames.wip_head(), component.frames.fin_head());
-                        machine.stack.scope_stack.push(scopeid);
-                        machine.stack.push(DiffInstruction::Diff { new, old });
+                    log::debug!("about to run scope {:?}", scopeid);
+
+                    if let Some(component) = self.pool.get_scope_mut(scopeid) {
+                        if component.run_scope(&self.pool) {
+                            let (old, new) =
+                                (component.frames.wip_head(), component.frames.fin_head());
+                            // let (old, new) = (component.frames.wip_head(), component.frames.fin_head());
+                            machine.stack.scope_stack.push(scopeid);
+                            machine.stack.push(DiffInstruction::Diff { new, old });
+                        }
                     }
                 }
             }
@@ -522,8 +531,6 @@ impl Scheduler {
             }
         }
 
-        log::debug!("work with deadline completed: {:#?}", committed_mutations);
-
         committed_mutations
     }
 
@@ -562,6 +569,8 @@ impl Scheduler {
             .get_scope_mut(base_scope)
             .expect("The base scope should never be moved");
 
+        log::debug!("rebuild {:?}", base_scope);
+
         // We run the component. If it succeeds, then we can diff it and add the changes to the dom.
         if cur_component.run_scope(&self.pool) {
             diff_machine
@@ -588,6 +597,8 @@ impl Scheduler {
             .get_scope_mut(base_scope)
             .expect("The base scope should never be moved");
 
+        log::debug!("hard diff {:?}", base_scope);
+
         if cur_component.run_scope(&self.pool) {
             let mut diff_machine = DiffMachine::new(Mutations::new(), &mut self.pool);
             diff_machine.cfg.force_diff = true;

+ 1 - 0
packages/core/src/scope.rs

@@ -218,6 +218,7 @@ impl Scope {
         caller: &'creator_node dyn for<'b> Fn(&'b Scope) -> DomTree<'b>,
         child_nodes: ScopeChildren,
     ) {
+        log::debug!("Updating scope dependencies {:?}", self.our_arena_idx);
         let caller = caller as *const _;
         self.caller = unsafe { std::mem::transmute(caller) };
 

+ 9 - 8
packages/hooks/src/use_shared_state.rs

@@ -112,10 +112,10 @@ impl<'a, T: 'static> UseSharedState<'a, T> {
     }
 
     pub fn notify_consumers(self) {
-        if !self.needs_notification.get() {
-            self.needs_notification.set(true);
-            self.root.borrow_mut().notify_consumers();
-        }
+        // if !self.needs_notification.get() {
+        self.root.borrow_mut().notify_consumers();
+        //     self.needs_notification.set(true);
+        // }
     }
 
     pub fn read_write(&self) -> (Ref<'_, T>, &Self) {
@@ -161,16 +161,17 @@ where
 ///
 ///
 ///
-pub fn use_provide_state<'a, T: 'static>(cx: Context<'a>, f: impl FnOnce() -> T) -> Option<()> {
+pub fn use_provide_state<'a, T: 'static>(cx: Context<'a>, f: impl FnOnce() -> T) {
     cx.use_hook(
         |_| {
-            cx.provide_state(ProvidedStateInner {
+            let state: ProvidedState<T> = RefCell::new(ProvidedStateInner {
                 value: Rc::new(RefCell::new(f())),
                 notify_any: cx.schedule_update_any(),
                 consumers: HashSet::new(),
-            })
+            });
+            cx.provide_state(state)
         },
-        |inner| inner.as_ref().and_then(|_| Some(())),
+        |inner| {},
         |_| {},
     )
 }

+ 1 - 1
packages/web/src/lib.rs

@@ -176,7 +176,7 @@ pub async fn run_with_props<T: 'static + Send>(root: FC<T>, root_props: T, cfg:
         work_loop.wait_for_raf().await;
 
         for mut edit in mutations {
-            log::debug!("edits are {:#?}", edit);
+            // log::debug!("edits are {:#?}", edit);
             // actually apply our changes during the animation frame
             websys_dom.process_edits(&mut edit.edits);
         }