Fix event bubbling inside templates after a hot template reload (#2484)

packages/core/src/arena.rs

@@ -80,11 +80,27 @@ impl VirtualDom {
 }
 
 impl ElementPath {
-    pub(crate) fn is_decendant(&self, small: &&[u8]) -> bool {
+    pub(crate) fn is_decendant(&self, small: &[u8]) -> bool {
-        small.len() <= self.path.len() && *small == &self.path[..small.len()]
+        small.len() <= self.path.len() && small == &self.path[..small.len()]
     }
 }
 
+#[test]
+fn is_decendant() {
+    let event_path = ElementPath {
+        path: &[1, 2, 3, 4, 5],
+    };
+
+    assert!(event_path.is_decendant(&[1, 2, 3, 4, 5]));
+    assert!(event_path.is_decendant(&[1, 2, 3, 4]));
+    assert!(event_path.is_decendant(&[1, 2, 3]));
+    assert!(event_path.is_decendant(&[1, 2]));
+    assert!(event_path.is_decendant(&[1]));
+
+    assert!(!event_path.is_decendant(&[1, 2, 3, 4, 5, 6]));
+    assert!(!event_path.is_decendant(&[2, 3, 4]));
+}
+
 impl PartialEq<&[u8]> for ElementPath {
     fn eq(&self, other: &&[u8]) -> bool {
         self.path.eq(*other)

packages/core/src/diff/node.rs

@@ -561,8 +561,12 @@ impl VNode {
 
         // If this is a debug build, we need to check that the paths are in the correct order because hot reloading can cause scrambled states
         #[cfg(debug_assertions)]
-        let (attrs_sorted, nodes_sorted) =
+        let (attrs_sorted, nodes_sorted) = {
-            { (sort_bfs(template.attr_paths), sort_bfs(template.node_paths)) };
+            (
+                crate::nodes::sort_bfo(template.attr_paths),
+                crate::nodes::sort_bfo(template.node_paths),
+            )
+        };
         #[cfg(debug_assertions)]
         let (mut attrs, mut nodes) = {
             (
@@ -959,54 +963,3 @@ fn matching_components<'a>(
         })
         .collect()
 }
-
-#[cfg(debug_assertions)]
-fn sort_bfs(paths: &[&'static [u8]]) -> Vec<(usize, &'static [u8])> {
-    let mut with_indecies = paths.iter().copied().enumerate().collect::<Vec<_>>();
-    with_indecies.sort_unstable_by(|(_, a), (_, b)| {
-        let mut a = a.iter();
-        let mut b = b.iter();
-        loop {
-            match (a.next(), b.next()) {
-                (Some(a), Some(b)) => {
-                    if a != b {
-                        return a.cmp(b);
-                    }
-                }
-                // The shorter path goes first
-                (None, Some(_)) => return std::cmp::Ordering::Less,
-                (Some(_), None) => return std::cmp::Ordering::Greater,
-                (None, None) => return std::cmp::Ordering::Equal,
-            }
-        }
-    });
-    with_indecies
-}
-
-#[test]
-#[cfg(debug_assertions)]
-fn sorting() {
-    let r: [(usize, &[u8]); 5] = [
-        (0, &[0, 1]),
-        (1, &[0, 2]),
-        (2, &[1, 0]),
-        (3, &[1, 0, 1]),
-        (4, &[1, 2]),
-    ];
-    assert_eq!(
-        sort_bfs(&[&[0, 1,], &[0, 2,], &[1, 0,], &[1, 0, 1,], &[1, 2,],]),
-        r
-    );
-    let r: [(usize, &[u8]); 6] = [
-        (0, &[0]),
-        (1, &[0, 1]),
-        (2, &[0, 1, 2]),
-        (3, &[1]),
-        (4, &[1, 2]),
-        (5, &[2]),
-    ];
-    assert_eq!(
-        sort_bfs(&[&[0], &[0, 1], &[0, 1, 2], &[1], &[1, 2], &[2],]),
-        r
-    );
-}

packages/core/src/nodes.rs

@@ -424,6 +424,22 @@ impl Template {
             .iter()
             .all(|root| matches!(root, Dynamic { .. } | DynamicText { .. }))
     }
+
+    /// Iterate over the attribute paths in order along with the original indexes for each path
+    pub(crate) fn breadth_first_attribute_paths(
+        &self,
+    ) -> impl Iterator<Item = (usize, &'static [u8])> {
+        // In release mode, hot reloading is disabled and everything is in breadth first order already
+        #[cfg(not(debug_assertions))]
+        {
+            self.attr_paths.iter().copied().enumerate()
+        }
+        // If we are in debug mode, hot reloading may have messed up the order of the paths. We need to sort them
+        #[cfg(debug_assertions)]
+        {
+            sort_bfo(self.attr_paths).into_iter()
+        }
+    }
 }
 
 /// A statically known node in a layout.
@@ -1012,3 +1028,54 @@ pub trait HasAttributes {
         volatile: bool,
     ) -> Self;
 }
+
+#[cfg(debug_assertions)]
+pub(crate) fn sort_bfo(paths: &[&'static [u8]]) -> Vec<(usize, &'static [u8])> {
+    let mut with_indecies = paths.iter().copied().enumerate().collect::<Vec<_>>();
+    with_indecies.sort_unstable_by(|(_, a), (_, b)| {
+        let mut a = a.iter();
+        let mut b = b.iter();
+        loop {
+            match (a.next(), b.next()) {
+                (Some(a), Some(b)) => {
+                    if a != b {
+                        return a.cmp(b);
+                    }
+                }
+                // The shorter path goes first
+                (None, Some(_)) => return std::cmp::Ordering::Less,
+                (Some(_), None) => return std::cmp::Ordering::Greater,
+                (None, None) => return std::cmp::Ordering::Equal,
+            }
+        }
+    });
+    with_indecies
+}
+
+#[test]
+#[cfg(debug_assertions)]
+fn sorting() {
+    let r: [(usize, &[u8]); 5] = [
+        (0, &[0, 1]),
+        (1, &[0, 2]),
+        (2, &[1, 0]),
+        (3, &[1, 0, 1]),
+        (4, &[1, 2]),
+    ];
+    assert_eq!(
+        sort_bfo(&[&[0, 1,], &[0, 2,], &[1, 0,], &[1, 0, 1,], &[1, 2,],]),
+        r
+    );
+    let r: [(usize, &[u8]); 6] = [
+        (0, &[0]),
+        (1, &[0, 1]),
+        (2, &[0, 1, 2]),
+        (3, &[1]),
+        (4, &[1, 2]),
+        (5, &[2]),
+    ];
+    assert_eq!(
+        sort_bfo(&[&[0], &[0, 1], &[0, 1, 2], &[1], &[1, 2], &[2],]),
+        r
+    );
+}

packages/core/src/virtual_dom.rs

@@ -429,7 +429,7 @@ impl VirtualDom {
 
         if let Some(Some(parent_path)) = self.elements.get(element.0).copied() {
             if bubbles {
-                self.handle_bubbling_event(Some(parent_path), name, Event::new(data, bubbles));
+                self.handle_bubbling_event(parent_path, name, Event::new(data, bubbles));
             } else {
                 self.handle_non_bubbling_event(parent_path, name, Event::new(data, bubbles));
             }
@@ -799,14 +799,10 @@ impl VirtualDom {
         level = "trace",
         name = "VirtualDom::handle_bubbling_event"
     )]
-    fn handle_bubbling_event(
+    fn handle_bubbling_event(&mut self, parent: ElementRef, name: &str, uievent: Event<dyn Any>) {
-        &mut self,
-        mut parent: Option<ElementRef>,
-        name: &str,
-        uievent: Event<dyn Any>,
-    ) {
         // If the event bubbles, we traverse through the tree until we find the target element.
         // Loop through each dynamic attribute (in a depth first order) in this template before moving up to the template's parent.
+        let mut parent = Some(parent);
         while let Some(path) = parent {
             let mut listeners = vec![];
 
@@ -815,13 +811,13 @@ impl VirtualDom {
             let target_path = path.path;
 
             // Accumulate listeners into the listener list bottom to top
-            for (idx, attrs) in el_ref.dynamic_attrs.iter().enumerate() {
+            for (idx, this_path) in node_template.breadth_first_attribute_paths() {
-                let this_path = node_template.attr_paths[idx];
+                let attrs = &*el_ref.dynamic_attrs[idx];
 
                 for attr in attrs.iter() {
                     // Remove the "on" prefix if it exists, TODO, we should remove this and settle on one
                     if attr.name.trim_start_matches("on") == name
-                        && target_path.is_decendant(&this_path)
+                        && target_path.is_decendant(this_path)
                     {
                         listeners.push(&attr.value);
 
@@ -842,6 +838,7 @@ impl VirtualDom {
                 "Calling {} listeners",
                 listeners.len()
             );
+            tracing::info!("Listeners: {:?}", listeners);
             for listener in listeners.into_iter().rev() {
                 if let AttributeValue::Listener(listener) = listener {
                     self.runtime.rendering.set(false);
@@ -870,10 +867,10 @@ impl VirtualDom {
         let node_template = el_ref.template.get();
         let target_path = node.path;
 
-        for (idx, attr) in el_ref.dynamic_attrs.iter().enumerate() {
+        for (idx, this_path) in node_template.breadth_first_attribute_paths() {
-            let this_path = node_template.attr_paths[idx];
+            let attrs = &*el_ref.dynamic_attrs[idx];
 
-            for attr in attr.iter() {
+            for attr in attrs.iter() {
                 // Remove the "on" prefix if it exists, TODO, we should remove this and settle on one
                 // Only call the listener if this is the exact target element.
                 if attr.name.trim_start_matches("on") == name && target_path == this_path {