Merge pull request #728 from Demonthos/handle-aborting-renders

Handle abort diffing

packages/core/src/arena.rs

@@ -1,3 +1,5 @@
+use std::ptr::NonNull;
+
 use crate::{
     nodes::RenderReturn, nodes::VNode, virtual_dom::VirtualDom, AttributeValue, DynamicNode,
     ScopeId,
@@ -17,7 +19,7 @@ pub(crate) struct ElementRef {
     pub path: ElementPath,
 
     // The actual template
-    pub template: *const VNode<'static>,
+    pub template: Option<NonNull<VNode<'static>>>,
 }
 
 #[derive(Clone, Copy)]
@@ -27,9 +29,9 @@ pub enum ElementPath {
 }
 
 impl ElementRef {
-    pub(crate) fn null() -> Self {
+    pub(crate) fn none() -> Self {
         Self {
-            template: std::ptr::null_mut(),
+            template: None,
             path: ElementPath::Root(0),
         }
     }
@@ -44,12 +46,21 @@ impl VirtualDom {
         self.next_reference(template, ElementPath::Root(path))
     }
 
+    pub(crate) fn next_null(&mut self) -> ElementId {
+        let entry = self.elements.vacant_entry();
+        let id = entry.key();
+
+        entry.insert(ElementRef::none());
+        ElementId(id)
+    }
+
     fn next_reference(&mut self, template: &VNode, path: ElementPath) -> ElementId {
         let entry = self.elements.vacant_entry();
         let id = entry.key();
 
         entry.insert(ElementRef {
-            template: template as *const _ as *mut _,
+            // We know this is non-null because it comes from a reference
+            template: Some(unsafe { NonNull::new_unchecked(template as *const _ as *mut _) }),
             path,
         });
         ElementId(id)

packages/core/src/diff.rs

@@ -42,7 +42,7 @@ impl<'b> VirtualDom {
                 // Just move over the placeholder
                 (Aborted(l), Aborted(r)) => r.id.set(l.id.get()),
 
-                // Becomes async, do nothing while we ait
+                // Becomes async, do nothing while we wait
                 (Ready(_nodes), Pending(_fut)) => self.diff_ok_to_async(_nodes, scope),
 
                 // Placeholder becomes something
@@ -65,8 +65,26 @@ impl<'b> VirtualDom {
         //
     }
 
-    fn diff_ok_to_err(&mut self, _l: &'b VNode<'b>, _p: &'b VPlaceholder) {
-        todo!()
+    fn diff_ok_to_err(&mut self, l: &'b VNode<'b>, p: &'b VPlaceholder) {
+        let id = self.next_null();
+        p.id.set(Some(id));
+        self.mutations.push(Mutation::CreatePlaceholder { id });
+
+        let pre_edits = self.mutations.edits.len();
+
+        self.remove_node(l, true);
+
+        // We should always have a remove mutation
+        // Eventually we don't want to generate placeholders, so this might not be true. But it's true today
+        assert!(self.mutations.edits.len() > pre_edits);
+
+        // We want to optimize the replace case to use one less mutation if possible
+        // Since mutations are done in reverse, the last node removed will be the first in the stack
+        // Instead of *just* removing it, we can use the replace mutation
+        match self.mutations.edits.pop().unwrap() {
+            Mutation::Remove { id } => self.mutations.push(Mutation::ReplaceWith { id, m: 1 }),
+            _ => panic!("Expected remove mutation from remove_node"),
+        };
     }
 
     fn diff_node(&mut self, left_template: &'b VNode<'b>, right_template: &'b VNode<'b>) {
@@ -905,12 +923,7 @@ impl<'b> VirtualDom {
 
         match unsafe { self.scopes[scope.0].root_node().extend_lifetime_ref() } {
             RenderReturn::Ready(t) => self.remove_node(t, gen_muts),
-            RenderReturn::Aborted(t) => {
-                if let Some(id) = t.id.get() {
-                    self.try_reclaim(id);
-                }
-                return;
-            }
+            RenderReturn::Aborted(placeholder) => self.remove_placeholder(placeholder, gen_muts),
             _ => todo!(),
         };
 

packages/core/src/virtual_dom.rs

@@ -282,7 +282,7 @@ impl VirtualDom {
         root.provide_context(Rc::new(ErrorBoundary::new(ScopeId(0))));
 
         // the root element is always given element ID 0 since it's the container for the entire tree
-        dom.elements.insert(ElementRef::null());
+        dom.elements.insert(ElementRef::none());
 
         dom
     }
@@ -387,7 +387,7 @@ impl VirtualDom {
         // Loop through each dynamic attribute in this template before moving up to the template's parent.
         while let Some(el_ref) = parent_path {
             // safety: we maintain references of all vnodes in the element slab
-            let template = unsafe { &*el_ref.template };
+            let template = unsafe { el_ref.template.unwrap().as_ref() };
             let node_template = template.template.get();
             let target_path = el_ref.path;