Browse Source

switch to bubble ids on vnodes to fix nodes under fragments pointing to an invalid parent

Evan Almloff 1 year ago
parent
commit
35b643e23f

+ 17 - 36
packages/core/src/arena.rs

@@ -1,8 +1,9 @@
+use std::ptr::NonNull;
+
 use crate::{
     innerlude::DirtyScope, nodes::RenderReturn, nodes::VNode, virtual_dom::VirtualDom,
     AttributeValue, DynamicNode, ScopeId,
 };
-use std::ptr::NonNull;
 
 /// An Element's unique identifier.
 ///
@@ -18,15 +19,15 @@ pub struct ElementId(pub usize);
 /// unmounted, then the `BubbleId` will be reused for a new component.
 #[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))]
 #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Default)]
-pub struct BubbleId(pub usize);
+pub struct VNodeId(pub usize);
 
 #[derive(Debug, Clone, Copy)]
-pub struct ElementRef<'a> {
+pub struct ElementRef {
     // the pathway of the real element inside the template
     pub(crate) path: ElementPath,
 
     // The actual template
-    pub(crate) template: Option<NonNull<VNode<'a>>>,
+    pub(crate) template: VNodeId,
 
     // The scope the element belongs to
     pub(crate) scope: ScopeId,
@@ -42,11 +43,12 @@ impl VirtualDom {
         ElementId(self.elements.insert(None))
     }
 
-    pub(crate) fn next_element_ref(&mut self, element_ref: ElementRef) -> BubbleId {
-        let new_id = BubbleId(
-            self.element_refs
-                .insert(unsafe { std::mem::transmute(element_ref) }),
-        );
+    pub(crate) fn next_vnode_ref(&mut self, vnode: &VNode) -> VNodeId {
+        let new_id = VNodeId(self.element_refs.insert(Some(unsafe {
+            std::mem::transmute::<NonNull<VNode>, _>(vnode.into())
+        })));
+
+        println!("next_element_ref: {:?}", new_id);
 
         // Set this id to be dropped when the scope is rerun
         if let Some(scope) = self.runtime.current_scope_id() {
@@ -72,30 +74,12 @@ impl VirtualDom {
             );
         }
 
-        self.elements.try_remove(el.0).and_then(|id| match id {
-            Some(id) => self.element_refs.try_remove(id.0).map(|_| ()),
-            None => Some(()),
-        })
-    }
-
-    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] = unsafe { std::mem::transmute(element_ref) };
-            }
-            None => {
-                self.elements[el.0] = Some(self.next_element_ref(element_ref));
-            }
-        }
-    }
-
-    pub(crate) fn update_template(&mut self, el: ElementId, node: *const VNode) {
-        let bubble_id = self.elements[el.0].unwrap();
-        self.update_template_bubble(bubble_id, node)
+        self.elements.try_remove(el.0).map(|_| ())
     }
 
-    pub(crate) fn update_template_bubble(&mut self, bubble_id: BubbleId, node: *const VNode) {
-        self.element_refs[bubble_id.0].template = Some(unsafe { std::mem::transmute(node) });
+    pub(crate) fn set_template(&mut self, id: VNodeId, vnode: &VNode) {
+        self.element_refs[id.0] =
+            Some(unsafe { std::mem::transmute::<NonNull<VNode>, _>(vnode.into()) });
     }
 
     // Drop a scope and all its children
@@ -141,9 +125,6 @@ impl VirtualDom {
                     self.drop_scope(f, true);
                 }
                 c.props.take();
-                if let Some(bubble_id) = c.bubble_id.get() {
-                    self.element_refs.try_remove(bubble_id.0);
-                }
             }
             DynamicNode::Fragment(nodes) => {
                 nodes.iter().for_each(|node| self.drop_scope_inner(node))
@@ -162,9 +143,9 @@ impl VirtualDom {
             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);
+                println!("ensure_drop_safety: {:?}", element_ref);
                 if let Some(element_ref) = element_refs_slab.get_mut(element_ref.0) {
-                    element_ref.template = None;
+                    *element_ref = None;
                 }
             }
         }

+ 17 - 22
packages/core/src/create.rs

@@ -96,6 +96,9 @@ impl<'b> VirtualDom {
             nodes_mut.resize(len, ElementId::default());
         };
 
+        // Set this node id
+        node.stable_id.set(Some(self.next_vnode_ref(node)));
+
         // The best renderers will have templates prehydrated and registered
         // Just in case, let's create the template using instructions anyways
         self.register_template(node.template.get());
@@ -187,7 +190,7 @@ impl<'b> VirtualDom {
                     path: ElementPath {
                         path: &template.template.get().node_paths[idx],
                     },
-                    template: Some(template.into()),
+                    template: template.stable_id().unwrap(),
                     scope: self.runtime.current_scope_id().unwrap_or(ScopeId(0)),
                 };
                 self.create_dynamic_node(template_ref, node)
@@ -197,10 +200,10 @@ impl<'b> VirtualDom {
                     path: ElementPath {
                         path: &template.template.get().node_paths[idx],
                     },
-                    template: Some(template.into()),
+                    template: template.stable_id().unwrap(),
                     scope: self.runtime.current_scope_id().unwrap_or(ScopeId(0)),
                 };
-                parent.set(Some(self.next_element_ref(template_ref)));
+                parent.set(Some(template_ref));
                 let id = self.set_slot(id);
                 self.mutations.push(CreatePlaceholder { id });
                 1
@@ -286,7 +289,7 @@ impl<'b> VirtualDom {
                 path: ElementPath {
                     path: &template.template.get().node_paths[idx],
                 },
-                template: Some(template.into()),
+                template: template.stable_id().unwrap(),
                 scope: self.runtime.current_scope_id().unwrap_or(ScopeId(0)),
             };
             let m = self.create_dynamic_node(boundary_ref, &template.dynamic_nodes[idx]);
@@ -340,10 +343,10 @@ impl<'b> VirtualDom {
                 let path = &template.template.get().attr_paths[idx];
                 let element_ref = ElementRef {
                     path: ElementPath { path },
-                    template: Some(template.into()),
+                    template: template.stable_id().unwrap(),
                     scope: self.runtime.current_scope_id().unwrap_or(ScopeId(0)),
                 };
-                self.set_template(id, element_ref);
+                self.elements[id.0] = Some(element_ref);
                 self.mutations.push(NewEventListener {
                     // all listeners start with "on"
                     name: &unbounded_name[2..],
@@ -474,7 +477,7 @@ impl<'b> VirtualDom {
 
     pub(crate) fn create_dynamic_node(
         &mut self,
-        parent: ElementRef<'b>,
+        parent: ElementRef,
         node: &'b DynamicNode<'b>,
     ) -> usize {
         use DynamicNode::*;
@@ -486,7 +489,7 @@ impl<'b> VirtualDom {
         }
     }
 
-    fn create_dynamic_text(&mut self, parent: ElementRef<'b>, text: &'b VText<'b>) -> usize {
+    fn create_dynamic_text(&mut self, parent: ElementRef, text: &'b VText<'b>) -> usize {
         // Allocate a dynamic element reference for this text node
         let new_id = self.next_element();
 
@@ -510,7 +513,7 @@ impl<'b> VirtualDom {
     pub(crate) fn create_placeholder(
         &mut self,
         placeholder: &VPlaceholder,
-        parent: ElementRef<'b>,
+        parent: ElementRef,
     ) -> usize {
         // Allocate a dynamic element reference for this text node
         let id = self.next_element();
@@ -519,7 +522,7 @@ impl<'b> VirtualDom {
         placeholder.id.set(Some(id));
 
         // Assign the placeholder's parent
-        placeholder.parent.set(Some(self.next_element_ref(parent)));
+        placeholder.parent.set(Some(parent));
 
         // Assign the ID to the existing node in the template
         self.mutations.push(AssignId {
@@ -533,7 +536,7 @@ impl<'b> VirtualDom {
 
     pub(super) fn create_component_node(
         &mut self,
-        parent: Option<ElementRef<'b>>,
+        parent: Option<ElementRef>,
         component: &'b VComponent<'b>,
     ) -> usize {
         use RenderReturn::*;
@@ -546,10 +549,8 @@ 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());
-                created
+                self.create_scope(scope, t)
             }
             Aborted(t) => self.mount_aborted(t, parent),
         }
@@ -567,17 +568,11 @@ impl<'b> VirtualDom {
             .unwrap_or_else(|| component.scope.get().unwrap())
     }
 
-    fn mount_aborted(
-        &mut self,
-        placeholder: &VPlaceholder,
-        parent: Option<ElementRef<'b>>,
-    ) -> usize {
+    fn mount_aborted(&mut self, placeholder: &VPlaceholder, parent: Option<ElementRef>) -> usize {
         let id = self.next_element();
         self.mutations.push(Mutation::CreatePlaceholder { id });
         placeholder.id.set(Some(id));
-        placeholder
-            .parent
-            .set(parent.map(|parent| self.next_element_ref(parent)));
+        placeholder.parent.set(parent);
 
         1
     }

+ 29 - 63
packages/core/src/diff.rs

@@ -10,7 +10,7 @@ use crate::{
     nodes::{DynamicNode, VNode},
     scopes::ScopeId,
     virtual_dom::VirtualDom,
-    Attribute, AttributeValue, TemplateNode,
+    Attribute, TemplateNode,
 };
 
 use rustc_hash::{FxHashMap, FxHashSet};
@@ -52,7 +52,7 @@ impl<'b> VirtualDom {
                 (Aborted(l), Ready(r)) => self.replace_placeholder(
                     l,
                     [r],
-                    self.element_refs[l.parent.get().expect("root should not be none").0],
+                    l.parent.get().expect("root node should not be none"),
                 ),
             };
         }
@@ -92,9 +92,7 @@ impl<'b> VirtualDom {
                 if let Some(&template) = map.get(&byte_index) {
                     right_template.template.set(template);
                     if template != left_template.template.get() {
-                        let parent = left_template.parent.take().map(|parent_id| unsafe {
-                            std::mem::transmute(self.element_refs[parent_id.0])
-                        });
+                        let parent = left_template.parent.take();
                         return self.replace(left_template, [right_template], parent);
                     }
                 }
@@ -106,6 +104,12 @@ impl<'b> VirtualDom {
             right_template.parent.set(left_template.parent.get());
         }
 
+        // Update the bubble id pointer
+        right_template.stable_id.set(left_template.stable_id.get());
+        if let Some(bubble_id) = right_template.stable_id.get() {
+            self.set_template(bubble_id, right_template);
+        }
+
         // If the templates are the same, we don't need to do anything, nor do we want to
         if templates_are_the_same(left_template, right_template) {
             return;
@@ -128,10 +132,7 @@ impl<'b> VirtualDom {
                 right_attr.mounted_element.set(mounted_element);
 
                 // If the attributes are different (or volatile), we need to update them
-                if let AttributeValue::Listener(_) = left_attr.value {
-                    // Nodes with Listeners always need their pointers updated
-                    self.update_template(mounted_element, right_template);
-                } else if left_attr.value != right_attr.value || left_attr.volatile {
+                if left_attr.value != right_attr.value || left_attr.volatile {
                     self.update_attribute(right_attr, left_attr);
                 }
             });
@@ -144,7 +145,7 @@ impl<'b> VirtualDom {
             .enumerate()
             .for_each(|(dyn_node_idx, (left_node, right_node))| {
                 let current_ref = ElementRef {
-                    template: Some(right_template.into()),
+                    template: right_template.stable_id().unwrap(),
                     path: ElementPath {
                         path: left_template.template.get().node_paths[dyn_node_idx],
                     },
@@ -167,7 +168,7 @@ impl<'b> VirtualDom {
         &mut self,
         left_node: &'b DynamicNode<'b>,
         right_node: &'b DynamicNode<'b>,
-        parent: ElementRef<'b>,
+        parent: ElementRef,
     ) {
         match (left_node, right_node) {
             (Text(left), Text(right)) => self.diff_vtext(left, right),
@@ -175,8 +176,6 @@ impl<'b> VirtualDom {
             (Placeholder(left), Placeholder(right)) => {
                 right.id.set(left.id.get());
                 right.parent.set(left.parent.get());
-                // Update the template
-                self.update_template(left.id.get().unwrap(), parent.template.unwrap().as_ptr());
             },
             (Component(left), Component(right)) => self.diff_vcomponent(left, right, Some(parent)),
             (Placeholder(left), Fragment(right)) => self.replace_placeholder(left, *right, parent),
@@ -201,7 +200,7 @@ impl<'b> VirtualDom {
         &mut self,
         left: &'b VComponent<'b>,
         right: &'b VComponent<'b>,
-        parent: Option<ElementRef<'b>>,
+        parent: Option<ElementRef>,
     ) {
         if std::ptr::eq(left, right) {
             return;
@@ -216,7 +215,6 @@ impl<'b> VirtualDom {
         let scope_id = left.scope.get().unwrap();
 
         right.scope.set(Some(scope_id));
-        right.bubble_id.set(left.bubble_id.get());
 
         // copy out the box for both
         let old = self.scopes[scope_id.0].props.as_ref();
@@ -227,16 +225,6 @@ 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;
         }
 
@@ -245,16 +233,6 @@ impl<'b> VirtualDom {
 
         // Now run the component and diff it
         self.run_scope(scope_id);
-        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);
-            }
-        }
         self.diff_scope(scope_id);
 
         self.dirty_scopes.remove(&DirtyScope {
@@ -267,7 +245,7 @@ impl<'b> VirtualDom {
         &mut self,
         right: &'b VComponent<'b>,
         left: &'b VComponent<'b>,
-        parent: Option<ElementRef<'b>>,
+        parent: Option<ElementRef>,
     ) {
         let m = self.create_component_node(parent, right);
 
@@ -324,10 +302,7 @@ impl<'b> VirtualDom {
     /// }
     /// ```
     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]) });
+        let parent = left.parent.take();
         match matching_components(left, right) {
             None => self.replace(left, [right], parent),
             Some(components) => components
@@ -354,7 +329,7 @@ impl<'b> VirtualDom {
         &mut self,
         old: &'b [VNode<'b>],
         new: &'b [VNode<'b>],
-        parent: ElementRef<'b>,
+        parent: ElementRef,
     ) {
         let new_is_keyed = new[0].key.is_some();
         let old_is_keyed = old[0].key.is_some();
@@ -386,7 +361,7 @@ impl<'b> VirtualDom {
         &mut self,
         old: &'b [VNode<'b>],
         new: &'b [VNode<'b>],
-        parent: ElementRef<'b>,
+        parent: ElementRef,
     ) {
         use std::cmp::Ordering;
 
@@ -427,7 +402,7 @@ impl<'b> VirtualDom {
         &mut self,
         old: &'b [VNode<'b>],
         new: &'b [VNode<'b>],
-        parent: ElementRef<'b>,
+        parent: ElementRef,
     ) {
         if cfg!(debug_assertions) {
             let mut keys = rustc_hash::FxHashSet::default();
@@ -506,7 +481,7 @@ impl<'b> VirtualDom {
         &mut self,
         old: &'b [VNode<'b>],
         new: &'b [VNode<'b>],
-        parent: ElementRef<'b>,
+        parent: ElementRef,
     ) -> Option<(usize, usize)> {
         let mut left_offset = 0;
 
@@ -565,7 +540,7 @@ impl<'b> VirtualDom {
         &mut self,
         old: &'b [VNode<'b>],
         new: &'b [VNode<'b>],
-        parent: ElementRef<'b>,
+        parent: ElementRef,
     ) {
         /*
         1. Map the old keys into a numerical ordering based on indices.
@@ -804,7 +779,7 @@ impl<'b> VirtualDom {
     fn create_children(
         &mut self,
         nodes: impl IntoIterator<Item = &'b VNode<'b>>,
-        parent: Option<ElementRef<'b>>,
+        parent: Option<ElementRef>,
     ) -> usize {
         nodes.into_iter().fold(0, |acc, child| {
             self.assign_boundary_ref(parent, child);
@@ -816,7 +791,7 @@ impl<'b> VirtualDom {
         &mut self,
         new: &'b [VNode<'b>],
         before: &'b VNode<'b>,
-        parent: ElementRef<'b>,
+        parent: ElementRef,
     ) {
         let m = self.create_children(new, Some(parent));
         let id = self.find_first_element(before);
@@ -827,7 +802,7 @@ impl<'b> VirtualDom {
         &mut self,
         new: &'b [VNode<'b>],
         after: &'b VNode<'b>,
-        parent: ElementRef<'b>,
+        parent: ElementRef,
     ) {
         let m = self.create_children(new, Some(parent));
         let id = self.find_last_element(after);
@@ -839,7 +814,7 @@ impl<'b> VirtualDom {
         &mut self,
         l: &'b VPlaceholder,
         r: impl IntoIterator<Item = &'b VNode<'b>>,
-        parent: ElementRef<'b>,
+        parent: ElementRef,
     ) {
         let m = self.create_children(r, Some(parent));
         let id = l.id.get().unwrap();
@@ -851,7 +826,7 @@ impl<'b> VirtualDom {
         &mut self,
         left: &'b VNode<'b>,
         right: impl IntoIterator<Item = &'b VNode<'b>>,
-        parent: Option<ElementRef<'b>>,
+        parent: Option<ElementRef>,
     ) {
         let m = self.create_children(right, parent);
 
@@ -872,17 +847,12 @@ impl<'b> VirtualDom {
         };
     }
 
-    fn node_to_placeholder(
-        &mut self,
-        l: &'b [VNode<'b>],
-        r: &'b VPlaceholder,
-        parent: ElementRef<'b>,
-    ) {
+    fn node_to_placeholder(&mut self, l: &'b [VNode<'b>], r: &'b VPlaceholder, parent: ElementRef) {
         // Create the placeholder first, ensuring we get a dedicated ID for the placeholder
         let placeholder = self.next_element();
 
         r.id.set(Some(placeholder));
-        r.parent.set(Some(self.next_element_ref(parent)));
+        r.parent.set(Some(parent));
 
         self.mutations
             .push(Mutation::CreatePlaceholder { id: placeholder });
@@ -1079,14 +1049,10 @@ impl<'b> VirtualDom {
         }
     }
 
-    pub(crate) fn assign_boundary_ref(
-        &mut self,
-        parent: Option<ElementRef<'b>>,
-        child: &'b VNode<'b>,
-    ) {
+    pub(crate) fn assign_boundary_ref(&mut self, parent: Option<ElementRef>, child: &'b VNode<'b>) {
         if let Some(parent) = parent {
             // assign the parent of the child
-            child.parent.set(Some(self.next_element_ref(parent)));
+            child.parent.set(Some(parent));
         }
     }
 }

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

@@ -31,6 +31,7 @@ pub fn Fragment<'a>(cx: Scope<'a, FragmentProps<'a>>) -> Element {
     Some(VNode {
         key: children.key,
         parent: children.parent.clone(),
+        stable_id: children.stable_id.clone(),
         template: children.template.clone(),
         root_ids: children.root_ids.clone(),
         dynamic_nodes: children.dynamic_nodes,

+ 32 - 6
packages/core/src/nodes.rs

@@ -1,4 +1,4 @@
-use crate::innerlude::BubbleId;
+use crate::innerlude::{ElementRef, VNodeId};
 use crate::{
     any_props::AnyProps, arena::ElementId, Element, Event, LazyNodes, ScopeId, ScopeState,
 };
@@ -48,7 +48,10 @@ pub struct VNode<'a> {
     pub key: Option<&'a str>,
 
     /// When rendered, this template will be linked to its parent manually
-    pub parent: Cell<Option<BubbleId>>,
+    pub(crate) parent: Cell<Option<ElementRef>>,
+
+    /// The bubble id assigned to the child that we need to update and drop when diffing happens
+    pub(crate) stable_id: Cell<Option<VNodeId>>,
 
     /// The static nodes and static descriptor of the template
     pub template: Cell<Template<'static>>,
@@ -70,6 +73,7 @@ impl<'a> VNode<'a> {
         Some(VNode {
             key: None,
             parent: Default::default(),
+            stable_id: Default::default(),
             root_ids: RefCell::new(bumpalo::collections::Vec::new_in(cx.bump())),
             dynamic_nodes: &[],
             dynamic_attrs: &[],
@@ -82,6 +86,30 @@ impl<'a> VNode<'a> {
         })
     }
 
+    /// Create a new VNode
+    pub fn new(
+        key: Option<&'a str>,
+        template: Template<'static>,
+        root_ids: bumpalo::collections::Vec<'a, ElementId>,
+        dynamic_nodes: &'a [DynamicNode<'a>],
+        dynamic_attrs: &'a [Attribute<'a>],
+    ) -> Self {
+        Self {
+            key,
+            parent: Cell::new(None),
+            stable_id: Cell::new(None),
+            template: Cell::new(template),
+            root_ids: RefCell::new(root_ids),
+            dynamic_nodes,
+            dynamic_attrs,
+        }
+    }
+
+    /// Get the stable id of this node used for bubbling events
+    pub(crate) fn stable_id(&self) -> Option<VNodeId> {
+        self.stable_id.get()
+    }
+
     /// Load a dynamic root at the given index
     ///
     /// Returns [`None`] if the root is actually a static node (Element/Text)
@@ -318,9 +346,6 @@ pub struct VComponent<'a> {
     /// The assigned Scope for this component
     pub(crate) scope: Cell<Option<ScopeId>>,
 
-    /// The bubble id assigned to the child that we need to update and drop when diffing happens
-    pub(crate) bubble_id: Cell<Option<BubbleId>>,
-
     /// The function pointer of the component, known at compile time
     ///
     /// It is possible that components get folded at compile time, so these shouldn't be really used as a key
@@ -377,7 +402,7 @@ pub struct VPlaceholder {
     /// The ID of this node in the real DOM
     pub(crate) id: Cell<Option<ElementId>>,
     /// The parent of this node
-    pub(crate) parent: Cell<Option<BubbleId>>,
+    pub(crate) parent: Cell<Option<ElementRef>>,
 }
 
 impl VPlaceholder {
@@ -729,6 +754,7 @@ impl<'a> IntoDynNode<'a> for &'a VNode<'a> {
     fn into_vnode(self, _cx: &'a ScopeState) -> DynamicNode<'a> {
         DynamicNode::Fragment(_cx.bump().alloc([VNode {
             parent: self.parent.clone(),
+            stable_id: self.stable_id.clone(),
             template: self.template.clone(),
             root_ids: self.root_ids.clone(),
             key: self.key,

+ 3 - 4
packages/core/src/scopes.rs

@@ -2,8 +2,8 @@ use crate::{
     any_props::AnyProps,
     any_props::VProps,
     bump_frame::BumpFrame,
-    innerlude::{BubbleId, ErrorBoundary},
-    innerlude::{DynamicNode, EventHandler, VComponent, VText},
+    innerlude::ErrorBoundary,
+    innerlude::{DynamicNode, EventHandler, VComponent, VNodeId, VText},
     lazynodes::LazyNodes,
     nodes::{IntoAttributeValue, IntoDynNode, RenderReturn},
     runtime::Runtime,
@@ -95,7 +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) element_refs_to_drop: RefCell<Vec<VNodeId>>,
 
     pub(crate) props: Option<Box<dyn AnyProps<'static>>>,
 }
@@ -456,7 +456,6 @@ impl<'src> ScopeState {
             render_fn: component as *const (),
             static_props: P::IS_STATIC,
             props: RefCell::new(Some(extended)),
-            bubble_id: Default::default(),
             scope: Default::default(),
         })
     }

+ 28 - 25
packages/core/src/virtual_dom.rs

@@ -4,19 +4,19 @@
 
 use crate::{
     any_props::VProps,
-    arena::{ElementId, ElementRef},
-    innerlude::{BubbleId, DirtyScope, ErrorBoundary, Mutations, Scheduler, SchedulerMsg},
+    arena::ElementId,
+    innerlude::{DirtyScope, ElementRef, ErrorBoundary, Mutations, Scheduler, SchedulerMsg},
     mutations::Mutation,
     nodes::RenderReturn,
     nodes::{Template, TemplateId},
     runtime::{Runtime, RuntimeGuard},
     scopes::{ScopeId, ScopeState},
-    AttributeValue, Element, Event, Scope,
+    AttributeValue, Element, Event, Scope, VNode,
 };
 use futures_util::{pin_mut, StreamExt};
 use rustc_hash::{FxHashMap, FxHashSet};
 use slab::Slab;
-use std::{any::Any, cell::Cell, collections::BTreeSet, future::Future, rc::Rc};
+use std::{any::Any, cell::Cell, collections::BTreeSet, future::Future, ptr::NonNull, rc::Rc};
 
 /// A virtual node system that progresses user events and diffs UI trees.
 ///
@@ -183,10 +183,10 @@ 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<'static>>,
+    pub(crate) element_refs: Slab<Option<NonNull<VNode<'static>>>>,
 
     // The element ids that are used in the renderer
-    pub(crate) elements: Slab<Option<BubbleId>>,
+    pub(crate) elements: Slab<Option<ElementRef>>,
 
     pub(crate) mutations: Mutations<'static>,
 
@@ -353,11 +353,16 @@ impl VirtualDom {
         | | |       <-- no, broke early
         |           <-- no, broke early
         */
-        let element = match self.elements.get(element.0) {
+        let parent_path = match self.elements.get(element.0) {
             Some(Some(el)) => el,
             _ => return,
         };
-        let mut parent_path = self.element_refs.get(element.0).cloned();
+        println!("handle_event: {:?}", element);
+        let mut parent_node = self
+            .element_refs
+            .get(parent_path.template.0)
+            .cloned()
+            .map(|el| (*parent_path, el));
         let mut listeners = vec![];
 
         // We will clone this later. The data itself is wrapped in RC to be used in callbacks if required
@@ -369,16 +374,11 @@ impl VirtualDom {
         // If the event bubbles, we traverse through the tree until we find the target element.
         if bubbles {
             // 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 {
+            while let Some((path, el_ref)) = parent_node {
                 // safety: we maintain references of all vnodes in the element slab
-                let template = unsafe {
-                    el_ref
-                        .template
-                        .expect("template reference should be valid")
-                        .as_ref()
-                };
+                let template = unsafe { el_ref.unwrap().as_ref() };
                 let node_template = template.template.get();
-                let target_path = el_ref.path;
+                let target_path = path.path;
 
                 for (idx, attr) in template.dynamic_attrs.iter().enumerate() {
                     let this_path = node_template.attr_paths[idx];
@@ -402,7 +402,7 @@ impl VirtualDom {
                 // We check the bubble state between each call to see if the event has been stopped from bubbling
                 for listener in listeners.drain(..).rev() {
                     if let AttributeValue::Listener(listener) = listener {
-                        let origin = el_ref.scope;
+                        let origin = path.scope;
                         self.runtime.scope_stack.borrow_mut().push(origin);
                         self.runtime.rendering.set(false);
                         if let Some(cb) = listener.borrow_mut().as_deref_mut() {
@@ -417,18 +417,21 @@ impl VirtualDom {
                     }
                 }
 
-                parent_path = template
-                    .parent
-                    .get()
-                    .and_then(|id| self.element_refs.get(id.0).cloned());
+                parent_node = template.parent.get().and_then(|element_ref| {
+                    println!("handle_event: {:?}", element_ref);
+                    self.element_refs
+                        .get(element_ref.template.0)
+                        .cloned()
+                        .map(|el| (element_ref, el))
+                });
             }
         } else {
             // Otherwise, we just call the listener on the target element
-            if let Some(el_ref) = parent_path {
+            if let Some((path, el_ref)) = parent_node {
                 // safety: we maintain references of all vnodes in the element slab
-                let template = unsafe { el_ref.template.unwrap().as_ref() };
+                let template = unsafe { el_ref.unwrap().as_ref() };
                 let node_template = template.template.get();
-                let target_path = el_ref.path;
+                let target_path = path.path;
 
                 for (idx, attr) in template.dynamic_attrs.iter().enumerate() {
                     let this_path = node_template.attr_paths[idx];
@@ -437,7 +440,7 @@ impl VirtualDom {
                     // Only call the listener if this is the exact target element.
                     if attr.name.trim_start_matches("on") == name && target_path == this_path {
                         if let AttributeValue::Listener(listener) = &attr.value {
-                            let origin = el_ref.scope;
+                            let origin = path.scope;
                             self.runtime.scope_stack.borrow_mut().push(origin);
                             self.runtime.rendering.set(false);
                             if let Some(cb) = listener.borrow_mut().as_deref_mut() {

+ 25 - 25
packages/core/tests/fuzzing.rs

@@ -2,7 +2,7 @@
 
 use dioxus::prelude::Props;
 use dioxus_core::*;
-use std::{cell::Cell, collections::HashSet};
+use std::collections::HashSet;
 
 fn random_ns() -> Option<&'static str> {
     let namespace = rand::random::<u8>() % 2;
@@ -170,22 +170,23 @@ fn create_random_dynamic_node(cx: &ScopeState, depth: usize) -> DynamicNode {
     let range = if depth > 5 { 1 } else { 4 };
     match rand::random::<u8>() % range {
         0 => DynamicNode::Placeholder(Default::default()),
-        1 => cx.make_node((0..(rand::random::<u8>() % 5)).map(|_| VNode {
-            key: None,
-            parent: Default::default(),
-            template: Cell::new(Template {
-                name: concat!(file!(), ":", line!(), ":", column!(), ":0"),
-                roots: &[TemplateNode::Dynamic { id: 0 }],
-                node_paths: &[&[0]],
-                attr_paths: &[],
-            }),
-            root_ids: bumpalo::collections::Vec::new_in(cx.bump()).into(),
-            dynamic_nodes: cx.bump().alloc([cx.component(
-                create_random_element,
-                DepthProps { depth, root: false },
-                "create_random_element",
-            )]),
-            dynamic_attrs: &[],
+        1 => cx.make_node((0..(rand::random::<u8>() % 5)).map(|_| {
+            VNode::new(
+                None,
+                Template {
+                    name: concat!(file!(), ":", line!(), ":", column!(), ":0"),
+                    roots: &[TemplateNode::Dynamic { id: 0 }],
+                    node_paths: &[&[0]],
+                    attr_paths: &[],
+                },
+                bumpalo::collections::Vec::new_in(cx.bump()).into(),
+                cx.bump().alloc([cx.component(
+                    create_random_element,
+                    DepthProps { depth, root: false },
+                    "create_random_element",
+                )]),
+                &[],
+            )
         })),
         2 => cx.component(
             create_random_element,
@@ -271,12 +272,11 @@ fn create_random_element(cx: Scope<DepthProps>) -> Element {
                 )
                 .into_boxed_str(),
             ));
-            let node = VNode {
-                key: None,
-                parent: Default::default(),
-                template: Cell::new(template),
-                root_ids: bumpalo::collections::Vec::new_in(cx.bump()).into(),
-                dynamic_nodes: {
+            let node = VNode::new(
+                None,
+                template,
+                bumpalo::collections::Vec::new_in(cx.bump()).into(),
+                {
                     let dynamic_nodes: Vec<_> = dynamic_node_types
                         .iter()
                         .map(|ty| match ty {
@@ -290,12 +290,12 @@ fn create_random_element(cx: Scope<DepthProps>) -> Element {
                         .collect();
                     cx.bump().alloc(dynamic_nodes)
                 },
-                dynamic_attrs: cx.bump().alloc(
+                cx.bump().alloc(
                     (0..template.attr_paths.len())
                         .map(|_| create_random_dynamic_attr(cx))
                         .collect::<Vec<_>>(),
                 ),
-            };
+            );
             Some(node)
         }
         _ => None,

+ 7 - 8
packages/rsx/src/lib.rs

@@ -244,14 +244,13 @@ impl<'a> ToTokens for TemplateRenderer<'a> {
                 node_paths: &[ #(#node_paths),* ],
                 attr_paths: &[ #(#attr_paths),* ],
             };
-            ::dioxus::core::VNode {
-                parent: Default::default(),
-                key: #key_tokens,
-                template: std::cell::Cell::new(TEMPLATE),
-                root_ids: dioxus::core::exports::bumpalo::collections::Vec::with_capacity_in(#root_count, __cx.bump()).into(),
-                dynamic_nodes: __cx.bump().alloc([ #( #node_printer ),* ]),
-                dynamic_attrs: __cx.bump().alloc([ #( #dyn_attr_printer ),* ]),
-            }
+            ::dioxus::core::VNode::new(
+                #key_tokens,
+                TEMPLATE,
+                dioxus::core::exports::bumpalo::collections::Vec::with_capacity_in(#root_count, __cx.bump()),
+                __cx.bump().alloc([ #( #node_printer ),* ]),
+                __cx.bump().alloc([ #( #dyn_attr_printer ),* ]),
+            )
         });
     }
 }