Browse Source

pass event propagation test

Evan Almloff 1 year ago
parent
commit
9552ab6c1c

+ 30 - 18
packages/core/src/arena.rs

@@ -25,7 +25,7 @@ pub struct ElementRef<'a> {
     pub(crate) path: ElementPath,
 
     // The actual template
-    pub(crate) template: *const VNode<'a>,
+    pub(crate) template: Option<*const VNode<'a>>,
 
     // The scope the element belongs to
     pub(crate) scope: ScopeId,
@@ -42,10 +42,20 @@ impl VirtualDom {
     }
 
     pub(crate) fn next_element_ref(&mut self, element_ref: ElementRef) -> BubbleId {
-        BubbleId(
+        let new_id = BubbleId(
             self.element_refs
                 .insert(unsafe { std::mem::transmute(element_ref) }),
-        )
+        );
+
+        // Set this id to be dropped when the scope is rerun
+        if let Some(scope) = self.runtime.current_scope_id() {
+            self.scopes[scope.0]
+                .element_refs_to_drop
+                .borrow_mut()
+                .push(new_id);
+        }
+
+        new_id
     }
 
     pub(crate) fn reclaim(&mut self, el: ElementId) {
@@ -84,7 +94,7 @@ impl VirtualDom {
     }
 
     pub(crate) fn update_template_bubble(&mut self, bubble_id: BubbleId, node: *const VNode) {
-        self.element_refs[bubble_id.0].template = unsafe { std::mem::transmute(node) };
+        self.element_refs[bubble_id.0].template = Some(unsafe { std::mem::transmute(node) });
     }
 
     // Drop a scope and all its children
@@ -124,10 +134,6 @@ impl VirtualDom {
     }
 
     fn drop_scope_inner(&mut self, node: &VNode) {
-        if let Some(id) = node.parent.get() {
-            self.element_refs.remove(id.0);
-        }
-
         node.dynamic_nodes.iter().for_each(|node| match node {
             DynamicNode::Component(c) => {
                 if let Some(f) = c.scope.get() {
@@ -141,24 +147,30 @@ impl VirtualDom {
             DynamicNode::Fragment(nodes) => {
                 nodes.iter().for_each(|node| self.drop_scope_inner(node))
             }
-            DynamicNode::Placeholder(placeholder) => {
-                if let Some(id) = placeholder.parent.get() {
-                    self.element_refs.remove(id.0);
-                }
-            }
+            DynamicNode::Placeholder(_) => {}
             DynamicNode::Text(_) => {}
         });
     }
 
     /// Descend through the tree, removing any borrowed props and listeners
-    pub(crate) fn ensure_drop_safety(&self, scope_id: ScopeId) {
+    pub(crate) fn ensure_drop_safety(&mut self, scope_id: ScopeId) {
         let scope = &self.scopes[scope_id.0];
 
+        {
+            // Drop all element refs that could be invalidated when the component was rerun
+            let mut element_refs = self.scopes[scope_id.0].element_refs_to_drop.borrow_mut();
+            let element_refs_slab = &mut self.element_refs;
+            for element_ref in element_refs.drain(..) {
+                println!("Dropping element ref {:?}", element_ref);
+                element_refs_slab[element_ref.0].template = None;
+            }
+        }
+
         // make sure we drop all borrowed props manually to guarantee that their drop implementation is called before we
         // run the hooks (which hold an &mut Reference)
         // recursively call ensure_drop_safety on all children
-        let mut props = scope.borrowed_props.borrow_mut();
-        props.drain(..).for_each(|comp| {
+        let props = { scope.borrowed_props.borrow_mut().clone() };
+        for comp in props {
             let comp = unsafe { &*comp };
             match comp.scope.get() {
                 Some(child) if child != scope_id => self.ensure_drop_safety(child),
@@ -167,10 +179,10 @@ impl VirtualDom {
             if let Ok(mut props) = comp.props.try_borrow_mut() {
                 *props = None;
             }
-        });
+        }
 
         // Now that all the references are gone, we can safely drop our own references in our listeners.
-        let mut listeners = scope.attributes_to_drop.borrow_mut();
+        let mut listeners = self.scopes[scope_id.0].attributes_to_drop.borrow_mut();
         listeners.drain(..).for_each(|listener| {
             let listener = unsafe { &*listener };
             match &listener.value {

+ 6 - 5
packages/core/src/create.rs

@@ -187,7 +187,7 @@ impl<'b> VirtualDom {
                     path: ElementPath {
                         path: &template.template.get().node_paths[idx],
                     },
-                    template: template,
+                    template: Some(template),
                     scope: self.runtime.current_scope_id().unwrap_or(ScopeId(0)),
                 };
                 self.create_dynamic_node(template_ref, node)
@@ -197,7 +197,7 @@ impl<'b> VirtualDom {
                     path: ElementPath {
                         path: &template.template.get().node_paths[idx],
                     },
-                    template: template,
+                    template: Some(template),
                     scope: self.runtime.current_scope_id().unwrap_or(ScopeId(0)),
                 };
                 parent.set(Some(self.next_element_ref(template_ref)));
@@ -286,7 +286,7 @@ impl<'b> VirtualDom {
                 path: ElementPath {
                     path: &template.template.get().node_paths[idx],
                 },
-                template,
+                template: Some(template),
                 scope: self.runtime.current_scope_id().unwrap_or(ScopeId(0)),
             };
             let m = self.create_dynamic_node(boundary_ref, &template.dynamic_nodes[idx]);
@@ -340,7 +340,7 @@ impl<'b> VirtualDom {
                 let path = &template.template.get().attr_paths[idx];
                 let element_ref = ElementRef {
                     path: ElementPath { path },
-                    template,
+                    template: Some(template),
                     scope: self.runtime.current_scope_id().unwrap_or(ScopeId(0)),
                 };
                 self.set_template(id, element_ref);
@@ -546,9 +546,10 @@ impl<'b> VirtualDom {
         match unsafe { self.run_scope(scope).extend_lifetime_ref() } {
             // Create the component's root element
             Ready(t) => {
+                let created = self.create_scope(scope, t);
                 self.assign_boundary_ref(parent, t);
                 component.bubble_id.set(t.parent.get());
-                self.create_scope(scope, t)
+                created
             }
             Aborted(t) => self.mount_aborted(t, parent),
         }

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

@@ -144,7 +144,7 @@ impl<'b> VirtualDom {
             .enumerate()
             .for_each(|(dyn_node_idx, (left_node, right_node))| {
                 let current_ref = ElementRef {
-                    template: right_template,
+                    template: Some(right_template),
                     path: ElementPath {
                         path: left_template.template.get().node_paths[dyn_node_idx],
                     },
@@ -176,7 +176,7 @@ impl<'b> VirtualDom {
                 right.id.set(left.id.get());
                 right.parent.set(left.parent.get());
                 // Update the template
-                self.update_template(left.id.get().unwrap(), parent.template);
+                self.update_template(left.id.get().unwrap(), parent.template.unwrap());
             },
             (Component(left), Component(right)) => self.diff_vcomponent(left, right, Some(parent)),
             (Placeholder(left), Fragment(right)) => self.replace_placeholder(left, *right, parent),
@@ -227,6 +227,16 @@ impl<'b> VirtualDom {
         // The target scopestate still has the reference to the old props, so there's no need to update anything
         // This also implicitly drops the new props since they're not used
         if left.static_props && unsafe { old.as_ref().unwrap().memoize(new.as_ref()) } {
+            if let Some(bubble_id) = right.bubble_id.get() {
+                if let RenderReturn::Ready(new_node) = unsafe {
+                    self.scopes[scope_id.0]
+                        .current_frame()
+                        .try_load_node()
+                        .expect("Call rebuild before diffing")
+                } {
+                    self.update_template_bubble(bubble_id, new_node);
+                }
+            }
             return;
         }
 

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

@@ -36,6 +36,7 @@ impl VirtualDom {
 
             borrowed_props: Default::default(),
             attributes_to_drop: Default::default(),
+            element_refs_to_drop: Default::default(),
         }));
 
         let context =

+ 2 - 1
packages/core/src/scopes.rs

@@ -2,7 +2,7 @@ use crate::{
     any_props::AnyProps,
     any_props::VProps,
     bump_frame::BumpFrame,
-    innerlude::ErrorBoundary,
+    innerlude::{BubbleId, ErrorBoundary},
     innerlude::{DynamicNode, EventHandler, VComponent, VText},
     lazynodes::LazyNodes,
     nodes::{IntoAttributeValue, IntoDynNode, RenderReturn},
@@ -95,6 +95,7 @@ pub struct ScopeState {
 
     pub(crate) borrowed_props: RefCell<Vec<*const VComponent<'static>>>,
     pub(crate) attributes_to_drop: RefCell<Vec<*const Attribute<'static>>>,
+    pub(crate) element_refs_to_drop: RefCell<Vec<BubbleId>>,
 
     pub(crate) props: Option<Box<dyn AnyProps<'static>>>,
 }

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

@@ -371,7 +371,7 @@ impl VirtualDom {
             // Loop through each dynamic attribute (in a depth first order) 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() };
                 let node_template = template.template.get();
                 let target_path = el_ref.path;
 
@@ -421,7 +421,7 @@ impl VirtualDom {
             // Otherwise, we just call the listener on the target element
             if 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() };
                 let node_template = template.template.get();
                 let target_path = el_ref.path;
 

+ 2 - 2
packages/core/tests/event_propagation.rs

@@ -16,7 +16,7 @@ fn events_propagate() {
     // break reference....
     for _ in 0..5 {
         dom.mark_dirty(ScopeId(0));
-        _ = dom.rebuild();
+        _ = dom.render_immediate();
     }
 
     // Lower click is registered
@@ -26,7 +26,7 @@ fn events_propagate() {
     // break reference....
     for _ in 0..5 {
         dom.mark_dirty(ScopeId(0));
-        _ = dom.rebuild();
+        _ = dom.render_immediate();
     }
 
     // Stop propagation occurs