WIP fix event bubbling

packages/core/src/arena.rs

@@ -1,5 +1,3 @@
-use std::ptr::NonNull;
-
 use crate::{
     innerlude::DirtyScope, nodes::RenderReturn, nodes::VNode, virtual_dom::VirtualDom,
     AttributeValue, DynamicNode, ScopeId,
@@ -21,13 +19,13 @@ pub struct ElementId(pub usize);
 #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Default)]
 pub struct BubbleId(pub usize);
 
-#[derive(Debug, Clone)]
-pub struct ElementRef {
+#[derive(Debug, Clone, Copy)]
+pub struct ElementRef<'a> {
     // the pathway of the real element inside the template
     pub(crate) path: ElementPath,
 
     // The actual template
-    pub(crate) template: NonNull<VNode<'static>>,
+    pub(crate) template: &'a VNode<'a>,
 
     // The scope the element belongs to
     pub(crate) scope: ScopeId,
@@ -35,7 +33,7 @@ pub struct ElementRef {
 
 #[derive(Clone, Copy, Debug)]
 pub struct ElementPath {
-    path: &'static [u8],
+    pub(crate) path: &'static [u8],
 }
 
 impl VirtualDom {
@@ -43,22 +41,11 @@ impl VirtualDom {
         ElementId(self.elements.insert(None))
     }
 
-    pub(crate) fn next_element_ref(&mut self, template: &VNode, path: &'static [u8]) -> BubbleId {
-        self.next_reference(template, ElementPath { path })
-    }
-
-    fn next_reference(&mut self, template: &VNode, path: ElementPath) -> BubbleId {
-        let entry = self.element_refs.vacant_entry();
-        let id = entry.key();
-        let scope = self.runtime.current_scope_id().unwrap_or(ScopeId(0));
-
-        entry.insert(ElementRef {
-            // We know this is non-null because it comes from a reference
-            template: unsafe { NonNull::new_unchecked(template as *const _ as *mut _) },
-            path,
-            scope,
-        });
-        BubbleId(id)
+    pub(crate) fn next_element_ref(&mut self, element_ref: ElementRef) -> BubbleId {
+        BubbleId(
+            self.element_refs
+                .insert(unsafe { std::mem::transmute(element_ref) }),
+        )
     }
 
     pub(crate) fn reclaim(&mut self, el: ElementId) {
@@ -77,15 +64,13 @@ impl VirtualDom {
         self.elements.try_remove(el.0).map(|_| ())
     }
 
-    pub(crate) fn set_template(&mut self, el: ElementId, node: &VNode, path: &'static [u8]) {
+    pub(crate) fn set_template(&mut self, el: ElementId, element_ref: ElementRef) {
         match self.elements[el.0] {
             Some(bubble_id) => {
-                self.element_refs[bubble_id.0].path = ElementPath { path };
-                self.element_refs[bubble_id.0].template =
-                    unsafe { NonNull::new_unchecked(node as *const _ as *mut _) };
+                self.element_refs[bubble_id.0] = unsafe { std::mem::transmute(element_ref) };
             }
             None => {
-                self.elements[el.0] = Some(self.next_reference(node, ElementPath { path }));
+                self.elements[el.0] = Some(self.next_element_ref(element_ref));
             }
         }
     }

packages/core/src/create.rs

@@ -1,6 +1,7 @@
 use crate::any_props::AnyProps;
-use crate::diff::BoundaryRef;
-use crate::innerlude::{BorrowedAttributeValue, VComponent, VPlaceholder, VText};
+use crate::innerlude::{
+    BorrowedAttributeValue, ElementPath, ElementRef, VComponent, VPlaceholder, VText,
+};
 use crate::mutations::Mutation;
 use crate::mutations::Mutation::*;
 use crate::nodes::VNode;
@@ -182,7 +183,14 @@ impl<'b> VirtualDom {
         use DynamicNode::*;
         match &template.dynamic_nodes[idx] {
             node @ Component { .. } | node @ Fragment(_) => {
-                self.create_dynamic_node(template, idx, node, idx)
+                let template_ref = ElementRef {
+                    path: ElementPath {
+                        path: &template.template.get().node_paths[idx],
+                    },
+                    template: &template,
+                    scope: self.runtime.current_scope_id().unwrap_or(ScopeId(0)),
+                };
+                self.create_dynamic_node(template_ref, node)
             }
             Placeholder(VPlaceholder { id }) => {
                 let id = self.set_slot(id);
@@ -266,7 +274,14 @@ impl<'b> VirtualDom {
             .map(|sorted_index| dynamic_nodes[sorted_index].0);
 
         for idx in reversed_iter {
-            let m = self.create_dynamic_node(template, idx, &template.dynamic_nodes[idx], idx);
+            let boundary_ref = ElementRef {
+                path: ElementPath {
+                    path: &template.template.get().node_paths[idx],
+                },
+                template,
+                scope: self.runtime.current_scope_id().unwrap_or(ScopeId(0)),
+            };
+            let m = self.create_dynamic_node(boundary_ref, &template.dynamic_nodes[idx]);
             if m > 0 {
                 // The path is one shorter because the top node is the root
                 let path = &template.template.get().node_paths[idx][1..];
@@ -315,7 +330,12 @@ impl<'b> VirtualDom {
         match &attribute.value {
             AttributeValue::Listener(_) => {
                 let path = &template.template.get().attr_paths[idx];
-                self.set_template(id, template, path);
+                let element_ref = ElementRef {
+                    path: ElementPath { path },
+                    template,
+                    scope: self.runtime.current_scope_id().unwrap_or(ScopeId(0)),
+                };
+                self.set_template(id, element_ref);
                 self.mutations.push(NewEventListener {
                     // all listeners start with "on"
                     name: &unbounded_name[2..],
@@ -446,19 +466,19 @@ impl<'b> VirtualDom {
 
     pub(crate) fn create_dynamic_node(
         &mut self,
-        parent: BoundaryRef<'b>,
+        parent: ElementRef<'b>,
         node: &'b DynamicNode<'b>,
     ) -> usize {
         use DynamicNode::*;
         match node {
             Text(text) => self.create_dynamic_text(parent, text),
             Placeholder(place) => self.create_placeholder(place, parent),
-            Component(component) => self.create_component_node(parent, component),
+            Component(component) => self.create_component_node(Some(parent), component),
             Fragment(frag) => frag.iter().map(|child| self.create(child)).sum(),
         }
     }
 
-    fn create_dynamic_text(&mut self, parent: BoundaryRef<'b>, text: &'b VText<'b>) -> usize {
+    fn create_dynamic_text(&mut self, parent: ElementRef<'b>, text: &'b VText<'b>) -> usize {
         // Allocate a dynamic element reference for this text node
         let new_id = self.next_element();
 
@@ -471,7 +491,7 @@ impl<'b> VirtualDom {
         // Add the mutation to the list
         self.mutations.push(HydrateText {
             id: new_id,
-            path: &parent.parent.template.get().node_paths[parent.dyn_node_idx][1..],
+            path: &parent.path.path[1..],
             value,
         });
 
@@ -482,7 +502,7 @@ impl<'b> VirtualDom {
     pub(crate) fn create_placeholder(
         &mut self,
         placeholder: &VPlaceholder,
-        parent: BoundaryRef<'b>,
+        parent: ElementRef<'b>,
     ) -> usize {
         // Allocate a dynamic element reference for this text node
         let id = self.next_element();
@@ -492,7 +512,7 @@ impl<'b> VirtualDom {
 
         // Assign the ID to the existing node in the template
         self.mutations.push(AssignId {
-            path: &parent.parent.template.get().node_paths[parent.dyn_node_idx][1..],
+            path: &parent.path.path[1..],
             id,
         });
 
@@ -502,7 +522,7 @@ impl<'b> VirtualDom {
 
     pub(super) fn create_component_node(
         &mut self,
-        parent: BoundaryRef<'b>,
+        parent: Option<ElementRef<'b>>,
         component: &'b VComponent<'b>,
     ) -> usize {
         use RenderReturn::*;

packages/core/src/diff.rs

@@ -1,7 +1,10 @@
 use crate::{
     any_props::AnyProps,
     arena::ElementId,
-    innerlude::{BorrowedAttributeValue, DirtyScope, VComponent, VPlaceholder, VText},
+    innerlude::{
+        BorrowedAttributeValue, DirtyScope, ElementPath, ElementRef, VComponent, VPlaceholder,
+        VText,
+    },
     mutations::Mutation,
     nodes::RenderReturn,
     nodes::{DynamicNode, VNode},
@@ -81,7 +84,10 @@ impl<'b> VirtualDom {
                 if let Some(&template) = map.get(&byte_index) {
                     right_template.template.set(template);
                     if template != left_template.template.get() {
-                        return self.replace(left_template, [right_template], todo!());
+                        let parent = left_template.parent.take().map(|parent_id| unsafe {
+                            std::mem::transmute(self.element_refs[parent_id.0])
+                        });
+                        return self.replace(left_template, [right_template], parent);
                     }
                 }
             }
@@ -99,7 +105,7 @@ impl<'b> VirtualDom {
 
         // If the templates are different by name, we need to replace the entire template
         if templates_are_different(left_template, right_template) {
-            return self.light_diff_templates(left_template, right_template, todo!());
+            return self.light_diff_templates(left_template, right_template);
         }
 
         // If the templates are the same, we can diff the attributes and children
@@ -129,9 +135,12 @@ impl<'b> VirtualDom {
             .zip(right_template.dynamic_nodes.iter())
             .enumerate()
             .for_each(|(dyn_node_idx, (left_node, right_node))| {
-                let current_ref = BoundaryRef {
-                    parent: right_template,
-                    dyn_node_idx,
+                let current_ref = ElementRef {
+                    template: right_template.into(),
+                    path: ElementPath {
+                        path: left_template.template.get().node_paths[dyn_node_idx],
+                    },
+                    scope: self.runtime.scope_stack.borrow().last().copied().unwrap(),
                 };
                 self.diff_dynamic_node(left_node, right_node, current_ref);
             });
@@ -150,13 +159,13 @@ impl<'b> VirtualDom {
         &mut self,
         left_node: &'b DynamicNode<'b>,
         right_node: &'b DynamicNode<'b>,
-        parent: BoundaryRef<'b>,
+        parent: ElementRef<'b>,
     ) {
         match (left_node, right_node) {
             (Text(left), Text(right)) => self.diff_vtext(left, right),
             (Fragment(left), Fragment(right)) => self.diff_non_empty_fragment(left, right, parent),
             (Placeholder(left), Placeholder(right)) => right.id.set(left.id.get()),
-            (Component(left), Component(right)) => self.diff_vcomponent(left, right, parent),
+            (Component(left), Component(right)) => self.diff_vcomponent(left, right, Some(parent)),
             (Placeholder(left), Fragment(right)) => self.replace_placeholder(left, *right, parent),
             (Fragment(left), Placeholder(right)) => self.node_to_placeholder(left, right),
             _ => todo!("This is an usual custom case for dynamic nodes. We don't know how to handle it yet."),
@@ -179,7 +188,7 @@ impl<'b> VirtualDom {
         &mut self,
         left: &'b VComponent<'b>,
         right: &'b VComponent<'b>,
-        parent: BoundaryRef<'b>,
+        parent: Option<ElementRef<'b>>,
     ) {
         if std::ptr::eq(left, right) {
             return;
@@ -224,7 +233,7 @@ impl<'b> VirtualDom {
         &mut self,
         right: &'b VComponent<'b>,
         left: &'b VComponent<'b>,
-        parent: BoundaryRef<'b>,
+        parent: Option<ElementRef<'b>>,
     ) {
         let m = self.create_component_node(parent, right);
 
@@ -280,17 +289,16 @@ impl<'b> VirtualDom {
     ///     Component { ..props }
     /// }
     /// ```
-    fn light_diff_templates(
-        &mut self,
-        left: &'b VNode<'b>,
-        right: &'b VNode<'b>,
-        parent: BoundaryRef<'b>,
-    ) {
+    fn light_diff_templates(&mut self, left: &'b VNode<'b>, right: &'b VNode<'b>) {
+        let parent = left
+            .parent
+            .take()
+            .map(|parent_id| unsafe { std::mem::transmute(self.element_refs[parent_id.0]) });
         match matching_components(left, right) {
             None => self.replace(left, [right], parent),
             Some(components) => components
                 .into_iter()
-                .for_each(|(idx, l, r)| self.diff_vcomponent(l, r, parent)),
+                .for_each(|(l, r)| self.diff_vcomponent(l, r, parent)),
         }
     }
 
@@ -312,7 +320,7 @@ impl<'b> VirtualDom {
         &mut self,
         old: &'b [VNode<'b>],
         new: &'b [VNode<'b>],
-        parent: BoundaryRef<'b>,
+        parent: ElementRef<'b>,
     ) {
         let new_is_keyed = new[0].key.is_some();
         let old_is_keyed = old[0].key.is_some();
@@ -344,7 +352,7 @@ impl<'b> VirtualDom {
         &mut self,
         old: &'b [VNode<'b>],
         new: &'b [VNode<'b>],
-        parent: BoundaryRef<'b>,
+        parent: ElementRef<'b>,
     ) {
         use std::cmp::Ordering;
 
@@ -385,7 +393,7 @@ impl<'b> VirtualDom {
         &mut self,
         old: &'b [VNode<'b>],
         new: &'b [VNode<'b>],
-        parent: BoundaryRef<'b>,
+        parent: ElementRef<'b>,
     ) {
         if cfg!(debug_assertions) {
             let mut keys = rustc_hash::FxHashSet::default();
@@ -464,7 +472,7 @@ impl<'b> VirtualDom {
         &mut self,
         old: &'b [VNode<'b>],
         new: &'b [VNode<'b>],
-        parent: BoundaryRef<'b>,
+        parent: ElementRef<'b>,
     ) -> Option<(usize, usize)> {
         let mut left_offset = 0;
 
@@ -523,7 +531,7 @@ impl<'b> VirtualDom {
         &mut self,
         old: &'b [VNode<'b>],
         new: &'b [VNode<'b>],
-        parent: BoundaryRef<'b>,
+        parent: ElementRef<'b>,
     ) {
         /*
         1. Map the old keys into a numerical ordering based on indices.
@@ -581,7 +589,7 @@ impl<'b> VirtualDom {
         if shared_keys.is_empty() {
             if old.get(0).is_some() {
                 self.remove_nodes(&old[1..]);
-                self.replace(&old[0], new, parent);
+                self.replace(&old[0], new, Some(parent));
             } else {
                 // I think this is wrong - why are we appending?
                 // only valid of the if there are no trailing elements
@@ -762,7 +770,7 @@ impl<'b> VirtualDom {
     fn create_children(
         &mut self,
         nodes: impl IntoIterator<Item = &'b VNode<'b>>,
-        parent: BoundaryRef<'b>,
+        parent: Option<ElementRef<'b>>,
     ) -> usize {
         nodes.into_iter().fold(0, |acc, child| {
             self.assign_boundary_ref(parent, child);
@@ -774,9 +782,9 @@ impl<'b> VirtualDom {
         &mut self,
         new: &'b [VNode<'b>],
         before: &'b VNode<'b>,
-        parent: BoundaryRef<'b>,
+        parent: ElementRef<'b>,
     ) {
-        let m = self.create_children(new, parent);
+        let m = self.create_children(new, Some(parent));
         let id = self.find_first_element(before);
         self.mutations.push(Mutation::InsertBefore { id, m })
     }
@@ -785,9 +793,9 @@ impl<'b> VirtualDom {
         &mut self,
         new: &'b [VNode<'b>],
         after: &'b VNode<'b>,
-        parent: BoundaryRef<'b>,
+        parent: ElementRef<'b>,
     ) {
-        let m = self.create_children(new, parent);
+        let m = self.create_children(new, Some(parent));
         let id = self.find_last_element(after);
         self.mutations.push(Mutation::InsertAfter { id, m })
     }
@@ -797,9 +805,9 @@ impl<'b> VirtualDom {
         &mut self,
         l: &'b VPlaceholder,
         r: impl IntoIterator<Item = &'b VNode<'b>>,
-        parent: BoundaryRef<'b>,
+        parent: ElementRef<'b>,
     ) {
-        let m = self.create_children(r, parent);
+        let m = self.create_children(r, Some(parent));
         let id = l.id.get().unwrap();
         self.mutations.push(Mutation::ReplaceWith { id, m });
         self.reclaim(id);
@@ -809,7 +817,7 @@ impl<'b> VirtualDom {
         &mut self,
         left: &'b VNode<'b>,
         right: impl IntoIterator<Item = &'b VNode<'b>>,
-        parent: BoundaryRef<'b>,
+        parent: Option<ElementRef<'b>>,
     ) {
         let m = self.create_children(right, parent);
 
@@ -1031,13 +1039,15 @@ impl<'b> VirtualDom {
         }
     }
 
-    pub(crate) fn assign_boundary_ref(&mut self, parent: BoundaryRef, child: &VNode<'_>) {
-        // assign the parent
-        let path = parent.parent.template.get().node_paths[parent.dyn_node_idx];
-
-        child
-            .parent
-            .set(Some(self.next_element_ref(parent.parent, path)));
+    pub(crate) fn assign_boundary_ref(
+        &mut self,
+        parent: Option<ElementRef<'b>>,
+        child: &'b VNode<'b>,
+    ) {
+        if let Some(parent) = parent {
+            // assign the parent of the child
+            child.parent.set(Some(self.next_element_ref(parent)));
+        }
     }
 }
 
@@ -1060,7 +1070,7 @@ fn templates_are_different(left_template: &VNode, right_template: &VNode) -> boo
 fn matching_components<'a>(
     left: &'a VNode<'a>,
     right: &'a VNode<'a>,
-) -> Option<Vec<(usize, &'a VComponent<'a>, &'a VComponent<'a>)>> {
+) -> Option<Vec<(&'a VComponent<'a>, &'a VComponent<'a>)>> {
     let left_template = left.template.get();
     let right_template = right.template.get();
     if left_template.roots.len() != right_template.roots.len() {
@@ -1072,8 +1082,7 @@ fn matching_components<'a>(
         .roots
         .iter()
         .zip(right_template.roots.iter())
-        .enumerate()
-        .map(|(idx, (l, r))| {
+        .map(|(l, r)| {
             let (l, r) = match (l, r) {
                 (TemplateNode::Dynamic { id: l }, TemplateNode::Dynamic { id: r }) => (l, r),
                 _ => return None,
@@ -1084,7 +1093,7 @@ fn matching_components<'a>(
                 _ => return None,
             };
 
-            Some((idx, l, r))
+            Some((l, r))
         })
         .collect()
 }
@@ -1116,9 +1125,3 @@ fn is_dyn_node_only_child(node: &VNode, idx: usize) -> bool {
         _ => false,
     }
 }
-
-// A parent that can be used for bubbling
-pub(crate) struct BoundaryRef<'b> {
-    pub parent: &'b VNode<'b>,
-    pub dyn_node_idx: usize,
-}

packages/core/src/virtual_dom.rs

@@ -183,7 +183,7 @@ pub struct VirtualDom {
     pub(crate) templates: FxHashMap<TemplateId, FxHashMap<usize, Template<'static>>>,
 
     // Every element is actually a dual reference - one to the template and the other to the dynamic node in that template
-    pub(crate) element_refs: Slab<ElementRef>,
+    pub(crate) element_refs: Slab<ElementRef<'static>>,
 
     // The element ids that are used in the renderer
     pub(crate) elements: Slab<Option<BubbleId>>,
@@ -353,13 +353,11 @@ impl VirtualDom {
         | | |       <-- no, broke early
         |           <-- no, broke early
         */
-        println!("calling {:?}", element);
         let element = match self.elements.get(element.0) {
             Some(Some(el)) => el,
             _ => return,
         };
         let mut parent_path = self.element_refs.get(element.0).cloned();
-        println!("parent path {:?}", parent_path);
         let mut listeners = vec![];
 
         // We will clone this later. The data itself is wrapped in RC to be used in callbacks if required
@@ -374,7 +372,6 @@ impl VirtualDom {
             while let Some(el_ref) = parent_path {
                 // safety: we maintain references of all vnodes in the element slab
                 let template = el_ref.template;
-                let template = unsafe { template.as_ref() };
                 let node_template = template.template.get();
                 let target_path = el_ref.path;
 
@@ -425,7 +422,6 @@ impl VirtualDom {
             if let Some(el_ref) = parent_path {
                 // safety: we maintain references of all vnodes in the element slab
                 let template = el_ref.template;
-                let template = unsafe { template.as_ref() };
                 let node_template = template.template.get();
                 let target_path = el_ref.path;
 

packages/core/tests/event_propagation.rs

@@ -0,0 +1,64 @@
+use dioxus::prelude::*;
+use dioxus_core::ElementId;
+use std::{rc::Rc, sync::Mutex};
+
+static CLICKS: Mutex<usize> = Mutex::new(0);
+
+#[test]
+fn events_propagate() {
+    let mut dom = VirtualDom::new(app);
+    _ = dom.rebuild();
+
+    // Top-level click is registered
+    dom.handle_event("click", Rc::new(MouseData::default()), ElementId(1), true);
+    assert_eq!(*CLICKS.lock().unwrap(), 1);
+
+    // break reference....
+    for _ in 0..5 {
+        dom.mark_dirty(ScopeId(0));
+        _ = dom.rebuild();
+    }
+
+    // Lower click is registered
+    dom.handle_event("click", Rc::new(MouseData::default()), ElementId(2), true);
+    assert_eq!(*CLICKS.lock().unwrap(), 3);
+
+    // break reference....
+    for _ in 0..5 {
+        dom.mark_dirty(ScopeId(0));
+        _ = dom.rebuild();
+    }
+
+    // Stop propagation occurs
+    dom.handle_event("click", Rc::new(MouseData::default()), ElementId(2), true);
+    assert_eq!(*CLICKS.lock().unwrap(), 3);
+}
+
+fn app(cx: Scope) -> Element {
+    render! {
+        div {
+            onclick: move |_| {
+                println!("top clicked");
+                *CLICKS.lock().unwrap() += 1;
+            },
+
+            problematic_child {}
+        }
+    }
+}
+
+fn problematic_child(cx: Scope) -> Element {
+    render! {
+        button {
+            onclick: move |evt| {
+                let mut clicks = CLICKS.lock().unwrap();
+
+                if *clicks == 3 {
+                    evt.stop_propagation();
+                } else {
+                    *clicks += 1;
+                }
+            }
+        }
+    }
+}

packages/core/tests/fuzzing.rs

@@ -271,10 +271,9 @@ fn create_random_element(cx: Scope<DepthProps>) -> Element {
                 )
                 .into_boxed_str(),
             ));
-            // println!("{template:#?}");
             let node = VNode {
                 key: None,
-                parent: None,
+                parent: Default::default(),
                 template: Cell::new(template),
                 root_ids: bumpalo::collections::Vec::new_in(cx.bump()).into(),
                 dynamic_nodes: {