From 4364a4b511098f3db81be62937b5f8478e5eddcb Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Mon, 19 Dec 2022 19:28:44 -0600 Subject: [PATCH] handle updating templates better --- packages/core/src/create.rs | 48 +++++++++++++++++--------------- packages/core/src/diff.rs | 38 ++++++++++++++++--------- packages/core/src/fragment.rs | 2 +- packages/core/src/nodes.rs | 10 +++---- packages/core/src/virtual_dom.rs | 8 +++--- packages/html/src/elements.rs | 4 +-- packages/html/src/lib.rs | 1 + packages/rsx/src/lib.rs | 8 +----- packages/ssr/src/cache.rs | 4 +-- packages/ssr/src/renderer.rs | 2 +- 10 files changed, 67 insertions(+), 58 deletions(-) diff --git a/packages/core/src/create.rs b/packages/core/src/create.rs index fa102bd52..c81841652 100644 --- a/packages/core/src/create.rs +++ b/packages/core/src/create.rs @@ -4,7 +4,7 @@ use crate::mutations::Mutation::*; use crate::nodes::VNode; use crate::nodes::{DynamicNode, TemplateNode}; use crate::virtual_dom::VirtualDom; -use crate::{AttributeValue, ElementId, RenderReturn, ScopeId, SuspenseContext}; +use crate::{AttributeValue, ElementId, RenderReturn, ScopeId, SuspenseContext, Template}; use std::cell::Cell; use std::iter::{Enumerate, Peekable}; use std::rc::Rc; @@ -26,19 +26,22 @@ impl<'b> VirtualDom { pub(crate) fn create(&mut self, node: &'b VNode<'b>) -> usize { // The best renderers will have templates prehydrated and registered // Just in case, let's create the template using instructions anyways - if !self.templates.contains_key(&node.template.name) { - self.register_template(node); + if !self.templates.contains_key(&node.template.get().name) { + self.register_template(node.template.get()); } // we know that this will generate at least one mutation per node - self.mutations.edits.reserve(node.template.roots.len()); + self.mutations + .edits + .reserve(node.template.get().roots.len()); // Walk the roots, creating nodes and assigning IDs // todo: adjust dynamic nodes to be in the order of roots and then leaves (ie BFS) - let mut attrs = node.template.attr_paths.iter().enumerate().peekable(); - let mut nodes = node.template.node_paths.iter().enumerate().peekable(); + let mut attrs = node.template.get().attr_paths.iter().enumerate().peekable(); + let mut nodes = node.template.get().node_paths.iter().enumerate().peekable(); node.template + .get() .roots .iter() .enumerate() @@ -137,7 +140,7 @@ impl<'b> VirtualDom { let m = self.create_dynamic_node(template, &template.dynamic_nodes[idx], idx); if m > 0 { // The path is one shorter because the top node is the root - let path = &template.template.node_paths[idx][1..]; + let path = &template.template.get().node_paths[idx][1..]; self.mutations.push(ReplacePlaceholder { m, path }); } } @@ -209,7 +212,7 @@ impl<'b> VirtualDom { template.root_ids[root_idx].set(Some(this_id)); self.mutations.push(LoadTemplate { - name: template.template.name, + name: template.template.get().name, index: root_idx, id: this_id, }); @@ -237,7 +240,7 @@ impl<'b> VirtualDom { // if attribute is on a root node, then we've already created the element // Else, it's deep in the template and we should create a new id for it - let id = self.next_element(template, template.template.attr_paths[attr_id]); + let id = self.next_element(template, template.template.get().attr_paths[attr_id]); self.mutations.push(Mutation::AssignId { path: &path[1..], @@ -248,14 +251,13 @@ impl<'b> VirtualDom { } /// Insert a new template into the VirtualDom's template registry - fn register_template(&mut self, template: &'b VNode<'b>) { + pub(crate) fn register_template(&mut self, template: Template<'static>) { // First, make sure we mark the template as seen, regardless if we process it - self.templates - .insert(template.template.name, template.template); + self.templates.insert(template.name, template); // If it's all dynamic nodes, then we don't need to register it - if !template.template.is_completely_dynamic() { - self.mutations.templates.push(template.template); + if !template.is_completely_dynamic() { + self.mutations.templates.push(template); } } @@ -281,7 +283,7 @@ impl<'b> VirtualDom { idx: usize, ) -> usize { // Allocate a dynamic element reference for this text node - let new_id = self.next_element(template, template.template.node_paths[idx]); + let new_id = self.next_element(template, template.template.get().node_paths[idx]); // Make sure the text node is assigned to the correct element text.id.set(Some(new_id)); @@ -292,7 +294,7 @@ impl<'b> VirtualDom { // Add the mutation to the list self.mutations.push(HydrateText { id: new_id, - path: &template.template.node_paths[idx][1..], + path: &template.template.get().node_paths[idx][1..], value, }); @@ -307,14 +309,14 @@ impl<'b> VirtualDom { idx: usize, ) -> usize { // Allocate a dynamic element reference for this text node - let id = self.next_element(template, template.template.node_paths[idx]); + let id = self.next_element(template, template.template.get().node_paths[idx]); // Make sure the text node is assigned to the correct element placeholder.id.set(Some(id)); // Assign the ID to the existing node in the template self.mutations.push(AssignId { - path: &template.template.node_paths[idx][1..], + path: &template.template.get().node_paths[idx][1..], id, }); @@ -382,7 +384,7 @@ impl<'b> VirtualDom { }; // Since this is a boundary, use its placeholder within the template as the placeholder for the suspense tree - let new_id = self.next_element(new, parent.template.node_paths[idx]); + let new_id = self.next_element(new, parent.template.get().node_paths[idx]); // Now connect everything to the boundary self.scopes[scope.0].placeholder.set(Some(new_id)); @@ -404,7 +406,7 @@ impl<'b> VirtualDom { // Now assign the placeholder in the DOM self.mutations.push(AssignId { id: new_id, - path: &parent.template.node_paths[idx][1..], + path: &parent.template.get().node_paths[idx][1..], }); 0 @@ -419,7 +421,7 @@ impl<'b> VirtualDom { idx: usize, scope: ScopeId, ) -> usize { - let new_id = self.next_element(template, template.template.node_paths[idx]); + let new_id = self.next_element(template, template.template.get().node_paths[idx]); // Set the placeholder of the scope self.scopes[scope.0].placeholder.set(Some(new_id)); @@ -427,7 +429,7 @@ impl<'b> VirtualDom { // Since the placeholder is already in the DOM, we don't create any new nodes self.mutations.push(AssignId { id: new_id, - path: &template.template.node_paths[idx][1..], + path: &template.template.get().node_paths[idx][1..], }); 0 @@ -439,7 +441,7 @@ impl<'b> VirtualDom { slot: &'b Cell>, id: usize, ) -> ElementId { - let id = self.next_element(template, template.template.node_paths[id]); + let id = self.next_element(template, template.template.get().node_paths[id]); slot.set(Some(id)); id } diff --git a/packages/core/src/diff.rs b/packages/core/src/diff.rs index 8b283f7f5..eb48a09bf 100644 --- a/packages/core/src/diff.rs +++ b/packages/core/src/diff.rs @@ -54,6 +54,12 @@ impl<'b> VirtualDom { fn diff_err_to_ok(&mut self, _e: &anyhow::Error, _l: &'b VNode<'b>) {} fn diff_node(&mut self, left_template: &'b VNode<'b>, right_template: &'b VNode<'b>) { + // If hot reloading is enabled, we need to make sure we're using the latest template + #[cfg(debug_assertions)] + if let Some(template) = self.templates.get(right_template.template.get().name) { + right_template.template.set(*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; @@ -321,7 +327,7 @@ impl<'b> VirtualDom { }; // Just remove the rest from the dom - for (idx, _) in node.template.roots.iter().enumerate().skip(1) { + for (idx, _) in node.template.get().roots.iter().enumerate().skip(1) { self.remove_root_node(node, idx); } @@ -337,7 +343,7 @@ impl<'b> VirtualDom { fn clean_up_node(&mut self, node: &'b VNode<'b>) { for (idx, dyn_node) in node.dynamic_nodes.iter().enumerate() { // Roots are cleaned up automatically? - if node.template.node_paths[idx].len() == 1 { + if node.template.get().node_paths[idx].len() == 1 { continue; } @@ -368,7 +374,7 @@ impl<'b> VirtualDom { let mut id = None; for (idx, attr) in node.dynamic_attrs.iter().enumerate() { // We'll clean up the root nodes either way, so don't worry - if node.template.attr_paths[idx].len() == 1 { + if node.template.get().attr_paths[idx].len() == 1 { continue; } @@ -789,7 +795,7 @@ impl<'b> VirtualDom { } fn remove_node(&mut self, node: &'b VNode<'b>) { - for (idx, _) in node.template.roots.iter().enumerate() { + for (idx, _) in node.template.get().roots.iter().enumerate() { let id = match node.dynamic_root(idx) { Some(Text(t)) => t.id.take(), Some(Placeholder(t)) => t.id.take(), @@ -832,6 +838,7 @@ impl<'b> VirtualDom { /// Push all the real nodes on the stack fn push_all_real_nodes(&mut self, node: &'b VNode<'b>) -> usize { node.template + .get() .roots .iter() .enumerate() @@ -905,7 +912,7 @@ impl<'b> VirtualDom { } fn find_last_element(&self, node: &'b VNode<'b>) -> ElementId { - match node.dynamic_root(node.template.roots.len() - 1) { + match node.dynamic_root(node.template.get().roots.len() - 1) { None => node.root_ids.last().unwrap().get().unwrap(), Some(Text(t)) => t.id.get().unwrap(), Some(Fragment(t)) => self.find_last_element(t.last().unwrap()), @@ -950,27 +957,31 @@ impl<'b> VirtualDom { /// We use the pointer of the dynamic_node list in this case fn templates_are_the_same<'b>(left_template: &'b VNode<'b>, right_template: &'b VNode<'b>) -> bool { std::ptr::eq(left_template, right_template) - || std::ptr::eq(left_template.dynamic_nodes, right_template.dynamic_nodes) } fn templates_are_different(left_template: &VNode, right_template: &VNode) -> bool { - !std::ptr::eq(left_template.template.name, right_template.template.name) - && left_template.template.name != right_template.template.name + let left_template_name = left_template.template.get().name; + let right_template_name = right_template.template.get().name; + // we want to re-create the node if the template name is different by pointer even if the value is the same so that we can detect when hot reloading changes the template + !std::ptr::eq(left_template_name, right_template_name) } fn matching_components<'a>( left: &'a VNode<'a>, right: &'a VNode<'a>, ) -> Option, &'a VComponent<'a>)>> { - if left.template.roots.len() != right.template.roots.len() { + return None; + let left_template = left.template.get(); + let right_template = right.template.get(); + if left_template.roots.len() != right_template.roots.len() { return None; } // run through the components, ensuring they're the same - left.template + left_template .roots .iter() - .zip(right.template.roots.iter()) + .zip(right_template.roots.iter()) .map(|(l, r)| { let (l, r) = match (l, r) { (TemplateNode::Dynamic { id: l }, TemplateNode::Dynamic { id: r }) => (l, r), @@ -995,11 +1006,12 @@ fn matching_components<'a>( /// - for appending children we can use AppendChildren #[allow(dead_code)] fn is_dyn_node_only_child(node: &VNode, idx: usize) -> bool { - let path = node.template.node_paths[idx]; + let template = node.template.get(); + let path = template.node_paths[idx]; // use a loop to index every static node's children until the path has run out // only break if the last path index is a dynamic node - let mut static_node = &node.template.roots[path[0] as usize]; + let mut static_node = &template.roots[path[0] as usize]; for i in 1..path.len() - 1 { match static_node { diff --git a/packages/core/src/fragment.rs b/packages/core/src/fragment.rs index 776d3e37a..1e3fa01c0 100644 --- a/packages/core/src/fragment.rs +++ b/packages/core/src/fragment.rs @@ -31,7 +31,7 @@ pub fn Fragment<'a>(cx: Scope<'a, FragmentProps<'a>>) -> Element { Ok(VNode { key: children.key, parent: children.parent, - template: children.template, + template: children.template.clone(), root_ids: children.root_ids, dynamic_nodes: children.dynamic_nodes, dynamic_attrs: children.dynamic_attrs, diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index c05731ee2..81351ae95 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -42,7 +42,7 @@ pub struct VNode<'a> { pub parent: Option, /// The static nodes and static descriptor of the template - pub template: Template<'static>, + pub template: Cell>, /// The IDs for the roots of this template - to be used when moving the template around and removing it from /// the actual Dom @@ -64,12 +64,12 @@ impl<'a> VNode<'a> { root_ids: &[], dynamic_nodes: &[], dynamic_attrs: &[], - template: Template { + template: Cell::new(Template { name: "dioxus-empty", roots: &[], node_paths: &[], attr_paths: &[], - }, + }), }) } @@ -77,7 +77,7 @@ impl<'a> VNode<'a> { /// /// Returns [`None`] if the root is actually a static node (Element/Text) pub fn dynamic_root(&self, idx: usize) -> Option<&'a DynamicNode<'a>> { - match &self.template.roots[idx] { + match &self.template.get().roots[idx] { TemplateNode::Element { .. } | TemplateNode::Text { text: _ } => None, TemplateNode::Dynamic { id } | TemplateNode::DynamicText { id } => { Some(&self.dynamic_nodes[*id]) @@ -581,7 +581,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, - template: self.template, + template: self.template.clone(), root_ids: self.root_ids, key: self.key, dynamic_nodes: self.dynamic_nodes, diff --git a/packages/core/src/virtual_dom.rs b/packages/core/src/virtual_dom.rs index 2c6021ef1..3484e0d66 100644 --- a/packages/core/src/virtual_dom.rs +++ b/packages/core/src/virtual_dom.rs @@ -352,10 +352,11 @@ impl VirtualDom { while let Some(el_ref) = parent_path { // safety: we maintain references of all vnodes in the element slab let template = unsafe { &*el_ref.template }; + let node_template = template.template.get(); let target_path = el_ref.path; for (idx, attr) in template.dynamic_attrs.iter().enumerate() { - let this_path = template.template.attr_paths[idx]; + let this_path = node_template.attr_paths[idx]; // listeners are required to be prefixed with "on", but they come back to the virtualdom with that missing // we should fix this so that we look for "onclick" instead of "click" @@ -457,12 +458,11 @@ impl VirtualDom { /// /// The caller must ensure that the template refrences the same dynamic attributes and nodes as the original template. pub fn replace_template(&mut self, template: Template<'static>) { - self.templates.insert(template.name, template); - panic!("{:?}", self.templates); + self.register_template(template); // iterating a slab is very inefficient, but this is a rare operation that will only happen during development so it's fine for (_, scope) in &self.scopes { if let Some(RenderReturn::Sync(Ok(sync))) = scope.try_root_node() { - if sync.template.name == template.name { + if sync.template.get().name == template.name { let height = scope.height; self.dirty_scopes.insert(DirtyScope { height, diff --git a/packages/html/src/elements.rs b/packages/html/src/elements.rs index 651715662..5f5ff59c5 100644 --- a/packages/html/src/elements.rs +++ b/packages/html/src/elements.rs @@ -205,9 +205,9 @@ macro_rules! builder_constructors { }; )* ) => { - pub struct Html; + pub struct HtmlCtx; - impl HotReloadingContext for Html { + impl HotReloadingContext for HtmlCtx { fn map_attribute(element: &str, attribute: &str) -> Option<(&'static str, Option<&'static str>)> { $( impl_element_match_attributes!( diff --git a/packages/html/src/lib.rs b/packages/html/src/lib.rs index 577899fe1..7de26becc 100644 --- a/packages/html/src/lib.rs +++ b/packages/html/src/lib.rs @@ -14,6 +14,7 @@ //! Currently, we don't validate for structures, but do validate attributes. mod elements; +pub use elements::HtmlCtx; pub mod events; pub mod geometry; mod global_attributes; diff --git a/packages/rsx/src/lib.rs b/packages/rsx/src/lib.rs index a46e7b67a..eb9792079 100644 --- a/packages/rsx/src/lib.rs +++ b/packages/rsx/src/lib.rs @@ -180,10 +180,6 @@ impl<'a, Ctx: HotReloadingContext> ToTokens for TemplateRenderer<'a, Ctx> { None => quote! { None }, }; - let spndbg = format!("{:?}", self.roots[0].span()); - println!("spndbg: {}", spndbg); - let root_col = spndbg[9..].split("..").next().unwrap(); - let root_printer = self.roots.iter().enumerate().map(|(idx, root)| { context.current_path.push(idx as u8); let out = context.render_static_node(root); @@ -207,8 +203,6 @@ impl<'a, Ctx: HotReloadingContext> ToTokens for TemplateRenderer<'a, Ctx> { line!(), ":", column!(), - ":", - #root_col ), roots: &[ #roots ], node_paths: &[ #(#node_paths),* ], @@ -217,7 +211,7 @@ impl<'a, Ctx: HotReloadingContext> ToTokens for TemplateRenderer<'a, Ctx> { ::dioxus::core::VNode { parent: None, key: #key_tokens, - template: TEMPLATE, + template: std::cell::Cell::new(TEMPLATE), root_ids: std::cell::Cell::from_mut( __cx.bump().alloc([None; #num_roots]) as &mut _).as_slice_of_cells(), // root_ids: std::cell::Cell::from_mut( __cx.bump().alloc([None; #num_roots]) as &mut [::dioxus::core::ElementId]).as_slice_of_cells(), dynamic_nodes: __cx.bump().alloc([ #( #node_printer ),* ]), diff --git a/packages/ssr/src/cache.rs b/packages/ssr/src/cache.rs index 4b6f52178..54ff65b26 100644 --- a/packages/ssr/src/cache.rs +++ b/packages/ssr/src/cache.rs @@ -36,13 +36,13 @@ impl StringCache { let mut cur_path = vec![]; - for (root_idx, root) in template.template.roots.iter().enumerate() { + for (root_idx, root) in template.template.get().roots.iter().enumerate() { Self::recurse(root, &mut cur_path, root_idx, &mut chain)?; } Ok(Self { segments: chain.segments, - template: template.template, + template: template.template.get(), }) } diff --git a/packages/ssr/src/renderer.rs b/packages/ssr/src/renderer.rs index cc015d570..ee98df987 100644 --- a/packages/ssr/src/renderer.rs +++ b/packages/ssr/src/renderer.rs @@ -66,7 +66,7 @@ impl Renderer { ) -> std::fmt::Result { let entry = self .template_cache - .entry(template.template.name) + .entry(template.template.get().name) .or_insert_with(|| Rc::new(StringCache::from_template(template).unwrap())) .clone();