diff --git a/packages/core/src/create.rs b/packages/core/src/create.rs index 7fbc8a9c8..226a446f9 100644 --- a/packages/core/src/create.rs +++ b/packages/core/src/create.rs @@ -1,5 +1,7 @@ use crate::any_props::AnyProps; -use crate::innerlude::{BorrowedAttributeValue, VComponent, VPlaceholder, VText}; +use crate::innerlude::{ + AttributeType, BorrowedAttributeValue, MountedAttribute, VComponent, VPlaceholder, VText, +}; use crate::mutations::Mutation; use crate::mutations::Mutation::*; use crate::nodes::VNode; @@ -283,7 +285,7 @@ impl<'b> VirtualDom { let id = self.assign_static_node_as_dynamic(path, root, node, attr_id); loop { - self.write_attribute(&node.dynamic_attrs[attr_id], id); + self.write_attribute_type(&node.dynamic_attrs[attr_id], id); // Only push the dynamic attributes forward if they match the current path (same element) match attrs.next_if(|(_, p)| *p == path) { @@ -294,10 +296,20 @@ impl<'b> VirtualDom { } } - fn write_attribute(&mut self, attribute: &'b crate::Attribute<'b>, id: ElementId) { + fn write_attribute_type(&mut self, attribute: &'b MountedAttribute<'b>, id: ElementId) { // Make sure we set the attribute's associated id attribute.mounted_element.set(id); + match &attribute.ty { + AttributeType::Single(attribute) => self.write_attribute(attribute, id), + AttributeType::Many(attribute) => { + for attribute in *attribute { + self.write_attribute(attribute, id); + } + } + } + } + pub(crate) fn write_attribute(&mut self, attribute: &'b crate::Attribute<'b>, id: ElementId) { // Safety: we promise not to re-alias this text later on after committing it to the mutation let unbounded_name: &str = unsafe { std::mem::transmute(attribute.name) }; diff --git a/packages/core/src/diff.rs b/packages/core/src/diff.rs index 270c81730..12fb31b1b 100644 --- a/packages/core/src/diff.rs +++ b/packages/core/src/diff.rs @@ -1,7 +1,9 @@ use crate::{ any_props::AnyProps, arena::ElementId, - innerlude::{BorrowedAttributeValue, DirtyScope, VComponent, VPlaceholder, VText}, + innerlude::{ + AttributeType, BorrowedAttributeValue, DirtyScope, VComponent, VPlaceholder, VText, + }, mutations::Mutation, nodes::RenderReturn, nodes::{DynamicNode, VNode}, @@ -103,16 +105,51 @@ impl<'b> VirtualDom { .zip(right_template.dynamic_attrs.iter()) .for_each(|(left_attr, right_attr)| { // Move over the ID from the old to the new - right_attr - .mounted_element - .set(left_attr.mounted_element.get()); + let mounted_id = left_attr.mounted_element.get(); + right_attr.mounted_element.set(mounted_id); // We want to make sure anything that gets pulled is valid self.update_template(left_attr.mounted_element.get(), right_template); - // If the attributes are different (or volatile), we need to update them - if left_attr.value != right_attr.value || left_attr.volatile { - self.update_attribute(right_attr, left_attr); + match (&left_attr.ty, &right_attr.ty) { + (AttributeType::Single(left), AttributeType::Single(right)) => { + self.diff_attribute(left, right, mounted_id) + } + (AttributeType::Many(left), AttributeType::Many(right)) => { + let mut left_iter = left.iter().peekable(); + let mut right_iter = right.iter().peekable(); + + loop { + match (left_iter.peek(), right_iter.peek()) { + (Some(left), Some(right)) => { + // check which name is greater + match left.name.cmp(right.name) { + std::cmp::Ordering::Less => self.remove_attribute( + left.name, + left.namespace, + mounted_id, + ), + std::cmp::Ordering::Greater => { + self.write_attribute(right, mounted_id) + } + std::cmp::Ordering::Equal => { + self.diff_attribute(left, right, mounted_id) + } + } + } + (Some(_), None) => { + let left = left_iter.next().unwrap(); + self.remove_attribute(left.name, left.namespace, mounted_id) + } + (None, Some(_)) => { + let right = right_iter.next().unwrap(); + self.write_attribute(right, mounted_id) + } + (None, None) => break, + } + } + } + _ => unreachable!("The macro should never generate this case"), } }); @@ -138,6 +175,18 @@ impl<'b> VirtualDom { } } + fn diff_attribute( + &mut self, + left_attr: &'b Attribute<'b>, + right_attr: &'b Attribute<'b>, + id: ElementId, + ) { + // If the attributes are different (or volatile), we need to update them + if left_attr.value != right_attr.value || left_attr.volatile { + self.update_attribute(right_attr, left_attr, id); + } + } + fn diff_dynamic_node( &mut self, left_node: &'b DynamicNode<'b>, @@ -155,12 +204,29 @@ impl<'b> VirtualDom { }; } - fn update_attribute(&mut self, right_attr: &'b Attribute<'b>, left_attr: &'b Attribute) { + fn remove_attribute(&mut self, name: &'b str, ns: Option<&'static str>, id: ElementId) { + let name = unsafe { std::mem::transmute(name) }; + let value: BorrowedAttributeValue<'b> = BorrowedAttributeValue::None; + let value = unsafe { std::mem::transmute(value) }; + self.mutations.push(Mutation::SetAttribute { + id, + ns, + name, + value, + }); + } + + fn update_attribute( + &mut self, + right_attr: &'b Attribute<'b>, + left_attr: &'b Attribute<'b>, + id: ElementId, + ) { let name = unsafe { std::mem::transmute(left_attr.name) }; let value: BorrowedAttributeValue<'b> = (&right_attr.value).into(); let value = unsafe { std::mem::transmute(value) }; self.mutations.push(Mutation::SetAttribute { - id: left_attr.mounted_element.get(), + id, ns: right_attr.namespace, name, value, diff --git a/packages/core/src/lib.rs b/packages/core/src/lib.rs index 230ae4c34..45fc43451 100644 --- a/packages/core/src/lib.rs +++ b/packages/core/src/lib.rs @@ -73,11 +73,11 @@ pub(crate) mod innerlude { } pub use crate::innerlude::{ - fc_to_builder, vdom_is_rendering, AnyValue, Attribute, AttributeValue, BorrowedAttributeValue, - CapturedError, Component, DynamicNode, Element, ElementId, Event, Fragment, HasAttributesBox, - IntoDynNode, LazyNodes, Mutation, Mutations, Properties, RenderReturn, Scope, ScopeId, - ScopeState, Scoped, TaskId, Template, TemplateAttribute, TemplateNode, VComponent, VNode, - VPlaceholder, VText, VirtualDom, + fc_to_builder, vdom_is_rendering, AnyValue, Attribute, AttributeType, AttributeValue, + BorrowedAttributeValue, CapturedError, Component, DynamicNode, Element, ElementId, Event, + Fragment, HasAttributesBox, IntoDynNode, LazyNodes, MountedAttribute, Mutation, Mutations, + Properties, RenderReturn, Scope, ScopeId, ScopeState, Scoped, TaskId, Template, + TemplateAttribute, TemplateNode, VComponent, VNode, VPlaceholder, VText, VirtualDom, }; /// The purpose of this module is to alleviate imports of many common types @@ -88,9 +88,9 @@ pub mod prelude { consume_context, consume_context_from_scope, current_scope_id, fc_to_builder, has_context, provide_context, provide_context_to_scope, provide_root_context, push_future, remove_future, schedule_update_any, spawn, spawn_forever, suspend, throw, AnyValue, - Component, Element, Event, EventHandler, Fragment, IntoAttributeValue, LazyNodes, - Properties, Scope, ScopeId, ScopeState, Scoped, TaskId, Template, TemplateAttribute, - TemplateNode, Throw, VNode, VirtualDom, + AttributeType, Component, Element, Event, EventHandler, Fragment, IntoAttributeValue, + LazyNodes, MountedAttribute, Properties, Scope, ScopeId, ScopeState, Scoped, TaskId, + Template, TemplateAttribute, TemplateNode, Throw, VNode, VirtualDom, }; } diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index f3d0ea750..e34fb7d42 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -60,7 +60,7 @@ pub struct VNode<'a> { pub dynamic_nodes: &'a [DynamicNode<'a>], /// The dynamic parts of the template - pub dynamic_attrs: &'a [Attribute<'a>], + pub dynamic_attrs: &'a [MountedAttribute<'a>], } impl<'a> VNode<'a> { @@ -414,6 +414,44 @@ pub enum TemplateAttribute<'a> { }, } +#[derive(Debug)] +pub struct MountedAttribute<'a> { + pub(crate) ty: AttributeType<'a>, + + /// The element in the DOM that this attribute belongs to + pub(crate) mounted_element: Cell, +} + +impl<'a> From> for MountedAttribute<'a> { + fn from(attr: Attribute<'a>) -> Self { + Self { + ty: AttributeType::Single(attr), + mounted_element: Default::default(), + } + } +} + +impl<'a> From<&'a [Attribute<'a>]> for MountedAttribute<'a> { + fn from(attr: &'a [Attribute<'a>]) -> Self { + Self { + ty: AttributeType::Many(attr), + mounted_element: Default::default(), + } + } +} + +impl<'a> MountedAttribute<'a> { + /// Get the type of this attribute + pub fn attribute_type(&self) -> &AttributeType<'a> { + &self.ty + } + + /// Get the element that this attribute is mounted to + pub fn mounted_element(&self) -> ElementId { + self.mounted_element.get() + } +} + /// An attribute on a DOM node, such as `id="my-thing"` or `href="https://example.com"` #[derive(Debug)] pub struct Attribute<'a> { @@ -430,9 +468,6 @@ pub struct Attribute<'a> { /// An indication of we should always try and set the attribute. Used in controlled components to ensure changes are propagated pub volatile: bool, - - /// The element in the DOM that this attribute belongs to - pub(crate) mounted_element: Cell, } impl<'a> Attribute<'a> { @@ -448,13 +483,38 @@ impl<'a> Attribute<'a> { value, namespace, volatile, - mounted_element: Cell::new(ElementId::default()), + } + } +} + +#[derive(Debug)] +pub enum AttributeType<'a> { + Single(Attribute<'a>), + /// Many different attributes sorted by name + Many(&'a [Attribute<'a>]), +} + +impl<'a> AttributeType<'a> { + /// Call the given function on each attribute + pub fn for_each<'b, F>(&'b self, mut f: F) + where + F: FnMut(&'b Attribute<'a>), + { + match self { + Self::Single(attr) => f(attr), + Self::Many(attrs) => attrs.iter().for_each(f), } } - /// Get the element that this attribute is mounted to - pub fn mounted_element(&self) -> ElementId { - self.mounted_element.get() + /// Try to call the given function on each attribute + pub fn try_for_each<'b, F, E>(&'b self, mut f: F) -> Result<(), E> + where + F: FnMut(&'b Attribute<'a>) -> Result<(), E>, + { + match self { + Self::Single(attr) => f(attr), + Self::Many(attrs) => attrs.iter().try_for_each(f), + } } } diff --git a/packages/core/src/scopes.rs b/packages/core/src/scopes.rs index 5ada89069..5d279f86a 100644 --- a/packages/core/src/scopes.rs +++ b/packages/core/src/scopes.rs @@ -8,7 +8,8 @@ use crate::{ nodes::{IntoAttributeValue, IntoDynNode, RenderReturn}, runtime::Runtime, scope_context::ScopeContext, - AnyValue, Attribute, AttributeValue, Element, Event, Properties, TaskId, + AnyValue, Attribute, AttributeType, AttributeValue, Element, Event, MountedAttribute, + Properties, TaskId, }; use bumpalo::{boxed::Box as BumpBox, Bump}; use std::{ @@ -335,14 +336,10 @@ impl<'src> ScopeState { let mut listeners = self.attributes_to_drop.borrow_mut(); for attr in element.dynamic_attrs { - match attr.value { - AttributeValue::Any(_) | AttributeValue::Listener(_) => { - let unbounded = unsafe { std::mem::transmute(attr as *const Attribute) }; - listeners.push(unbounded); - } - - _ => (), - } + attr.ty.for_each(|attr| { + let unbounded = unsafe { std::mem::transmute(attr as *const Attribute) }; + listeners.push(unbounded); + }) } let mut props = self.borrowed_props.borrow_mut(); @@ -393,13 +390,15 @@ impl<'src> ScopeState { value: impl IntoAttributeValue<'src>, namespace: Option<&'static str>, volatile: bool, - ) -> Attribute<'src> { - Attribute { - name, - namespace, - volatile, + ) -> MountedAttribute<'src> { + MountedAttribute { + ty: AttributeType::Single(Attribute { + name, + namespace, + volatile, + value: value.into_value(self.bump()), + }), mounted_element: Default::default(), - value: value.into_value(self.bump()), } } diff --git a/packages/core/src/virtual_dom.rs b/packages/core/src/virtual_dom.rs index a1def71d2..bee13b3a2 100644 --- a/packages/core/src/virtual_dom.rs +++ b/packages/core/src/virtual_dom.rs @@ -5,13 +5,13 @@ use crate::{ any_props::VProps, arena::{ElementId, ElementRef}, - innerlude::{DirtyScope, ErrorBoundary, Mutations, Scheduler, SchedulerMsg}, + innerlude::{AttributeType, DirtyScope, ErrorBoundary, Mutations, Scheduler, SchedulerMsg}, mutations::Mutation, nodes::RenderReturn, nodes::{Template, TemplateId}, runtime::{Runtime, RuntimeGuard}, scopes::{ScopeId, ScopeState}, - AttributeValue, Element, Event, Scope, + Attribute, AttributeValue, Element, Event, Scope, }; use futures_util::{pin_mut, StreamExt}; use rustc_hash::{FxHashMap, FxHashSet}; @@ -371,12 +371,30 @@ impl VirtualDom { for (idx, attr) in template.dynamic_attrs.iter().enumerate() { let this_path = node_template.attr_paths[idx]; - // 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) - { - listeners.push(&attr.value); + fn add_listener<'a>( + attribute: &'a Attribute<'a>, + event_name: &str, + listeners: &mut Vec<&'a AttributeValue<'a>>, + ) { + if attribute.name.trim_start_matches("on") == event_name { + listeners.push(&attribute.value); + } + listeners.push(&attribute.value); + } + + // Remove the "on" prefix if it exists, TODO, we should remove this and settle on one + if target_path.is_decendant(&this_path) { + match &attr.ty { + AttributeType::Single(attribute) => { + add_listener(attribute, name, &mut listeners); + } + AttributeType::Many(attributes) => { + for attribute in *attributes { + add_listener(attribute, name, &mut listeners); + } + } + } // Break if this is the exact target element. // This means we won't call two listeners with the same name on the same element. This should be // documented, or be rejected from the rsx! macro outright @@ -422,20 +440,57 @@ impl VirtualDom { for (idx, attr) in template.dynamic_attrs.iter().enumerate() { let this_path = node_template.attr_paths[idx]; - // 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 { - if let AttributeValue::Listener(listener) = &attr.value { - let origin = el_ref.scope; - self.runtime.scope_stack.borrow_mut().push(origin); - self.runtime.rendering.set(false); - if let Some(cb) = listener.borrow_mut().as_deref_mut() { - cb(uievent.clone()); - } - self.runtime.scope_stack.borrow_mut().pop(); - self.runtime.rendering.set(true); + fn call_listener( + attribute: &Attribute, + event_name: &str, + uievent: &Event, + runtime: &Runtime, + origin: ScopeId, + ) -> bool { + // 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 attribute.name.trim_start_matches("on") == event_name { + if let AttributeValue::Listener(listener) = &attribute.value { + runtime.scope_stack.borrow_mut().push(origin); + runtime.rendering.set(false); + if let Some(cb) = listener.borrow_mut().as_deref_mut() { + cb(uievent.clone()); + } + runtime.scope_stack.borrow_mut().pop(); + runtime.rendering.set(true); - break; + return true; + } + } + false + } + + if target_path == this_path { + match &attr.ty { + AttributeType::Single(attribute) => { + if call_listener( + attribute, + name, + &uievent, + &self.runtime, + el_ref.scope, + ) { + return; + } + } + AttributeType::Many(attributes) => { + for attribute in *attributes { + if call_listener( + attribute, + name, + &uievent, + &self.runtime, + el_ref.scope, + ) { + return; + } + } + } } } } diff --git a/packages/core/tests/fuzzing.rs b/packages/core/tests/fuzzing.rs index 40bb5dcbf..0de10ec56 100644 --- a/packages/core/tests/fuzzing.rs +++ b/packages/core/tests/fuzzing.rs @@ -1,7 +1,7 @@ #![cfg(not(miri))] use dioxus::prelude::Props; -use dioxus_core::*; +use dioxus_core::{MountedAttribute, *}; use std::{cell::Cell, collections::HashSet}; fn random_ns() -> Option<&'static str> { @@ -205,7 +205,7 @@ fn create_random_dynamic_node(cx: &ScopeState, depth: usize) -> DynamicNode { } } -fn create_random_dynamic_attr(cx: &ScopeState) -> Attribute { +fn create_random_dynamic_attr(cx: &ScopeState) -> MountedAttribute { let value = match rand::random::() % 7 { 0 => AttributeValue::Text(Box::leak( format!("{}", rand::random::()).into_boxed_str(), @@ -217,7 +217,7 @@ fn create_random_dynamic_attr(cx: &ScopeState) -> Attribute { 5 => AttributeValue::None, 6 => { let value = cx.listener(|e: Event| println!("{:?}", e)); - return Attribute::new("ondata", value, None, false); + return Attribute::new("ondata", value, None, false).into(); } _ => unreachable!(), }; @@ -227,6 +227,7 @@ fn create_random_dynamic_attr(cx: &ScopeState) -> Attribute { random_ns(), rand::random(), ) + .into() } static mut TEMPLATE_COUNT: usize = 0; diff --git a/packages/dioxus/tests/props_spread.rs b/packages/dioxus/tests/props_spread.rs index 8839e86e8..01423ea80 100644 --- a/packages/dioxus/tests/props_spread.rs +++ b/packages/dioxus/tests/props_spread.rs @@ -164,7 +164,7 @@ fn props_spread() { static TEMPLATE: ::dioxus::core::Template = ::dioxus::core::Template { name: concat!(file!(), ":", line!(), ":", column!(), ":", "" ), roots: &[::dioxus::core::TemplateNode::Element { tag: dioxus_elements::audio::TAG_NAME, namespace: dioxus_elements::audio::NAME_SPACE, attrs: &[::dioxus::core::TemplateAttribute::Dynamic { id: 0usize }], children: &[] }], node_paths: &[], attr_paths: &[&[0u8]] }; let mut attrs = vec![__cx.attr(dioxus_elements::audio::muted.0, muted, dioxus_elements::audio::muted.1, dioxus_elements::audio::muted.2)]; for attr in attributes { - attrs.push(__cx.attr(attr.name, attr.value.into_value(__cx.bump()), attr.namespace, attr.volatile)); + attrs.push(attr); } ::dioxus::core::VNode { parent: None, diff --git a/packages/html-internal-macro/tests/01-simple.rs b/packages/html-internal-macro/tests/01-simple.rs index e71fdf554..f328e4d9d 100644 --- a/packages/html-internal-macro/tests/01-simple.rs +++ b/packages/html-internal-macro/tests/01-simple.rs @@ -1 +1 @@ -fn main() {} \ No newline at end of file +fn main() {} diff --git a/packages/html/src/events.rs b/packages/html/src/events.rs index c59149493..dc4e2b815 100644 --- a/packages/html/src/events.rs +++ b/packages/html/src/events.rs @@ -9,7 +9,7 @@ macro_rules! impl_event { $( $( #[$attr] )* #[inline] - pub fn $name<'a, E: crate::EventReturn, T>(_cx: &'a ::dioxus_core::ScopeState, mut _f: impl FnMut(::dioxus_core::Event<$data>) -> E + 'a) -> ::dioxus_core::Attribute<'a> { + pub fn $name<'a, E: crate::EventReturn, T>(_cx: &'a ::dioxus_core::ScopeState, mut _f: impl FnMut(::dioxus_core::Event<$data>) -> E + 'a) -> ::dioxus_core::MountedAttribute<'a> { ::dioxus_core::Attribute::new( stringify!($name), _cx.listener(move |e: ::dioxus_core::Event<$data>| { @@ -17,7 +17,7 @@ macro_rules! impl_event { }), None, false, - ) + ).into() } )* }; diff --git a/packages/rsx/src/element.rs b/packages/rsx/src/element.rs index dd33c825f..f64ab9809 100644 --- a/packages/rsx/src/element.rs +++ b/packages/rsx/src/element.rs @@ -167,7 +167,7 @@ impl Parse for Element { attributes, children, brace, - extra_attributes + extra_attributes, }) } } @@ -397,11 +397,3 @@ impl ToTokens for ElementAttrNamed { tokens.append_all(attribute); } } - -// ::dioxus::core::Attribute { -// name: stringify!(#name), -// namespace: None, -// volatile: false, -// mounted_node: Default::default(), -// value: ::dioxus::core::AttributeValue::Text(#value), -// } diff --git a/packages/ssr/src/renderer.rs b/packages/ssr/src/renderer.rs index f23e654fb..74066ad44 100644 --- a/packages/ssr/src/renderer.rs +++ b/packages/ssr/src/renderer.rs @@ -79,23 +79,31 @@ impl Renderer { match segment { Segment::Attr(idx) => { let attr = &template.dynamic_attrs[*idx]; - if attr.name == "dangerous_inner_html" { - inner_html = Some(attr); - } else if attr.namespace == Some("style") { - accumulated_dynamic_styles.push(attr); - } else { - match attr.value { - AttributeValue::Text(value) => { - write!(buf, " {}=\"{}\"", attr.name, value)? - } - AttributeValue::Bool(value) => write!(buf, " {}={}", attr.name, value)?, - AttributeValue::Int(value) => write!(buf, " {}={}", attr.name, value)?, - AttributeValue::Float(value) => { - write!(buf, " {}={}", attr.name, value)? - } - _ => {} - }; - } + attr.attribute_type().try_for_each(|attr| { + if attr.name == "dangerous_inner_html" { + inner_html = Some(attr); + } else if attr.namespace == Some("style") { + accumulated_dynamic_styles.push(attr); + } else { + match attr.value { + AttributeValue::Text(value) => { + write!(buf, " {}=\"{}\"", attr.name, value)? + } + AttributeValue::Bool(value) => { + write!(buf, " {}={}", attr.name, value)? + } + AttributeValue::Int(value) => { + write!(buf, " {}={}", attr.name, value)? + } + AttributeValue::Float(value) => { + write!(buf, " {}={}", attr.name, value)? + } + _ => {} + }; + } + + Ok(()) + })?; } Segment::Node(idx) => match &template.dynamic_nodes[*idx] { DynamicNode::Component(node) => {