From 35b643e23fcf6a67faaef4ec3d8deaead3525100 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Mon, 11 Sep 2023 15:55:58 -0500 Subject: [PATCH] switch to bubble ids on vnodes to fix nodes under fragments pointing to an invalid parent --- packages/core/src/arena.rs | 53 ++++++------------ packages/core/src/create.rs | 39 ++++++-------- packages/core/src/diff.rs | 92 ++++++++++---------------------- packages/core/src/fragment.rs | 1 + packages/core/src/nodes.rs | 38 ++++++++++--- packages/core/src/scopes.rs | 7 ++- packages/core/src/virtual_dom.rs | 53 +++++++++--------- packages/core/tests/fuzzing.rs | 50 ++++++++--------- packages/rsx/src/lib.rs | 15 +++--- 9 files changed, 159 insertions(+), 189 deletions(-) diff --git a/packages/core/src/arena.rs b/packages/core/src/arena.rs index 6430934f5..5dd8dbf67 100644 --- a/packages/core/src/arena.rs +++ b/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>>, + 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::, _>(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(()), - }) + self.elements.try_remove(el.0).map(|_| ()) } - 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) - } - - 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::, _>(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; } } } diff --git a/packages/core/src/create.rs b/packages/core/src/create.rs index 44b344380..7db744cf5 100644 --- a/packages/core/src/create.rs +++ b/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>, + parent: Option, 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>, - ) -> usize { + fn mount_aborted(&mut self, placeholder: &VPlaceholder, parent: Option) -> 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 } diff --git a/packages/core/src/diff.rs b/packages/core/src/diff.rs index cbff0621e..20358cfce 100644 --- a/packages/core/src/diff.rs +++ b/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>, + parent: Option, ) { 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>, + parent: Option, ) { 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>, - parent: Option>, + parent: Option, ) -> 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>, - 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>, - parent: Option>, + parent: Option, ) { 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>, - child: &'b VNode<'b>, - ) { + pub(crate) fn assign_boundary_ref(&mut self, parent: Option, 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)); } } } diff --git a/packages/core/src/fragment.rs b/packages/core/src/fragment.rs index 0e4bf09e3..dbc8e8131 100644 --- a/packages/core/src/fragment.rs +++ b/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, diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index f3a27eecf..c0b1a382a 100644 --- a/packages/core/src/nodes.rs +++ b/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>, + pub(crate) parent: Cell>, + + /// The bubble id assigned to the child that we need to update and drop when diffing happens + pub(crate) stable_id: Cell>, /// The static nodes and static descriptor of the template pub template: Cell>, @@ -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 { + 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>, - /// The bubble id assigned to the child that we need to update and drop when diffing happens - pub(crate) bubble_id: Cell>, - /// 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>, /// The parent of this node - pub(crate) parent: Cell>, + pub(crate) parent: Cell>, } 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, diff --git a/packages/core/src/scopes.rs b/packages/core/src/scopes.rs index 5cb066f41..95c06f6b3 100644 --- a/packages/core/src/scopes.rs +++ b/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>>, pub(crate) attributes_to_drop: RefCell>>, - pub(crate) element_refs_to_drop: RefCell>, + pub(crate) element_refs_to_drop: RefCell>, pub(crate) props: Option>>, } @@ -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(), }) } diff --git a/packages/core/src/virtual_dom.rs b/packages/core/src/virtual_dom.rs index 6601f2793..60727cb42 100644 --- a/packages/core/src/virtual_dom.rs +++ b/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>>, // 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>, + pub(crate) element_refs: Slab>>>, // The element ids that are used in the renderer - pub(crate) elements: Slab>, + pub(crate) elements: Slab>, 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() { diff --git a/packages/core/tests/fuzzing.rs b/packages/core/tests/fuzzing.rs index 95c3ed9bf..39b5b152c 100644 --- a/packages/core/tests/fuzzing.rs +++ b/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::() % 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::() % range { 0 => DynamicNode::Placeholder(Default::default()), - 1 => cx.make_node((0..(rand::random::() % 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::() % 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) -> 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) -> 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::>(), ), - }; + ); Some(node) } _ => None, diff --git a/packages/rsx/src/lib.rs b/packages/rsx/src/lib.rs index d5e5b2009..e49f4c52c 100644 --- a/packages/rsx/src/lib.rs +++ b/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 ),* ]), + ) }); } }