From 36e9eb11600e6ef08bc205dbf27a402dde93eaf6 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Sun, 14 Jan 2024 15:21:19 -0600 Subject: [PATCH] partially restore spreading attributes --- examples/spread.rs | 16 +-- packages/core-macro/src/props/mod.rs | 83 +++--------- packages/core/src/diff/node.rs | 157 +++++++++++++++-------- packages/core/src/error_boundary.rs | 2 +- packages/core/src/lib.rs | 14 +- packages/core/src/nodes.rs | 51 +++++++- packages/core/src/virtual_dom.rs | 42 +++--- packages/core/tests/create_dom.rs | 8 +- packages/core/tests/create_fragments.rs | 9 +- packages/core/tests/diff_unkeyed_list.rs | 2 +- packages/core/tests/fuzzing.rs | 6 +- packages/core/tests/miri_full_app.rs | 2 +- packages/core/tests/task.rs | 2 +- packages/desktop/src/launch.rs | 4 +- packages/html-internal-macro/src/lib.rs | 4 +- packages/html/src/elements.rs | 7 +- packages/html/src/global_attributes.rs | 3 +- packages/html/src/lib.rs | 4 +- packages/rsx/src/lib.rs | 30 +++-- 19 files changed, 252 insertions(+), 194 deletions(-) diff --git a/examples/spread.rs b/examples/spread.rs index beb8ee2d3..ce32dae12 100644 --- a/examples/spread.rs +++ b/examples/spread.rs @@ -1,8 +1,8 @@ -use dioxus::prelude::*; +use dioxus::{core::NoOpMutations, prelude::*}; fn main() { let mut dom = VirtualDom::new(app); - let _ = dom.rebuild(); + let _ = dom.rebuild(&mut NoOpMutations); let html = dioxus_ssr::render(&dom); println!("{}", html); @@ -23,14 +23,14 @@ fn app() -> Element { #[component] fn Component(props: Props) -> Element { render! { - audio { ..cx.props.attributes, "1: {cx.props.extra_data}\n2: {cx.props.extra_data2}" } + audio { ..props.attributes, "1: {props.extra_data}\n2: {props.extra_data2}" } } } -#[derive(Props)] -struct Props<'a> { +#[derive(Props, PartialEq, Clone)] +struct Props { #[props(extends = GlobalAttributes)] - attributes: Vec>, - extra_data: &'a str, - extra_data2: &'a str, + attributes: Vec, + extra_data: String, + extra_data2: String, } diff --git a/packages/core-macro/src/props/mod.rs b/packages/core-macro/src/props/mod.rs index 59358fb9c..07ebbd8be 100644 --- a/packages/core-macro/src/props/mod.rs +++ b/packages/core-macro/src/props/mod.rs @@ -40,7 +40,7 @@ pub fn impl_my_derive(ast: &syn::DeriveInput) -> Result { #builder_creation #conversion_helper #( #fields )* - // #( #extends )* + #( #extends )* #( #required_fields )* #build_method } @@ -497,8 +497,7 @@ mod struct_info { use syn::parse::Error; use syn::punctuated::Punctuated; use syn::spanned::Spanned; - use syn::visit::Visit; - use syn::{parse_quote, Expr, Ident}; + use syn::{Expr, Ident}; use super::field_info::{FieldBuilderAttr, FieldInfo}; use super::util::{ @@ -532,37 +531,6 @@ mod struct_info { .filter(|f| !f.builder_attr.extends.is_empty()) } - fn extend_lifetime(&self) -> syn::Result> { - let first_extend = self.extend_fields().next(); - - match first_extend { - Some(f) => { - struct VisitFirstLifetime(Option); - - impl Visit<'_> for VisitFirstLifetime { - fn visit_lifetime(&mut self, lifetime: &'_ syn::Lifetime) { - if self.0.is_none() { - self.0 = Some(lifetime.clone()); - } - } - } - - let name = f.name; - let mut visitor = VisitFirstLifetime(None); - - visitor.visit_type(f.ty); - - visitor.0.ok_or_else(|| { - syn::Error::new_spanned( - name, - "Unable to find lifetime for extended field. Please specify it manually", - ) - }).map(Some) - } - None => Ok(None), - } - } - pub fn new( ast: &'a syn::DeriveInput, fields: impl Iterator, @@ -607,16 +575,8 @@ mod struct_info { // Therefore, we will generate code that shortcircuits the "comparison" in memoization let are_there_generics = !self.generics.params.is_empty(); - let extend_lifetime = self.extend_lifetime()?; - let generics = self.generics.clone(); - let (_, ty_generics, where_clause) = generics.split_for_impl(); - let impl_generics = self.modify_generics(|g| { - if extend_lifetime.is_none() { - g.params.insert(0, parse_quote!('__bump)); - } - }); - let (impl_generics, _, _) = impl_generics.split_for_impl(); + let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); let (_, b_initial_generics, _) = self.generics.split_for_impl(); let all_fields_param = syn::GenericParam::Type( syn::Ident::new("TypedBuilderFields", proc_macro2::Span::call_site()).into(), @@ -699,12 +659,23 @@ Finally, call `.build()` to create the instance of `{name}`. false => quote! { self == other }, }; + let extend_fields = self.extend_fields().map(|f| { + let name = f.name; + let ty = f.ty; + quote!(#name: #ty) + }); + let extend_fields_value = self.extend_fields().map(|f| { + let name = f.name; + quote!(#name: Vec::new()) + }); + Ok(quote! { impl #impl_generics #name #ty_generics #where_clause { #[doc = #builder_method_doc] #[allow(dead_code, clippy::type_complexity)] #vis fn builder() -> #builder_name #generics_with_empty { #builder_name { + #(#extend_fields_value,)* fields: #empties_tuple, _phantom: ::core::default::Default::default(), } @@ -715,11 +686,12 @@ Finally, call `.build()` to create the instance of `{name}`. #builder_type_doc #[allow(dead_code, non_camel_case_types, non_snake_case)] #vis struct #builder_name #b_generics { + #(#extend_fields,)* fields: #all_fields_param, _phantom: (#( #phantom_generics ),*), } - impl #impl_generics ::dioxus::prelude::Properties<#extend_lifetime> for #name #ty_generics + impl #impl_generics ::dioxus::prelude::Properties for #name #ty_generics #b_generics_where_extras_predicates { type Builder = #builder_name #generics_with_empty; @@ -776,7 +748,6 @@ Finally, call `.build()` to create the instance of `{name}`. }); let reconstructing = self.included_fields().map(|f| f.name); - // Add the bump lifetime to the generics let mut ty_generics: Vec = self .generics .params @@ -840,11 +811,6 @@ Finally, call `.build()` to create the instance of `{name}`. quote!(#name: self.#name) }); - let extend_lifetime = self.extend_lifetime()?.ok_or(Error::new_spanned( - field_name, - "Unable to find lifetime for extended field. Please specify it manually", - ))?; - let extends_impl = field.builder_attr.extends.iter().map(|path| { let name_str = path_to_single_string(path).unwrap(); let camel_name = name_str.to_case(Case::UpperCamel); @@ -854,18 +820,18 @@ Finally, call `.build()` to create the instance of `{name}`. ); quote! { #[allow(dead_code, non_camel_case_types, missing_docs)] - impl #impl_generics dioxus_elements::extensions::#marker_name < #extend_lifetime > for #builder_name < #( #ty_generics ),* > #where_clause {} + impl #impl_generics dioxus_elements::extensions::#marker_name for #builder_name < #( #ty_generics ),* > #where_clause {} } }); Ok(quote! { #[allow(dead_code, non_camel_case_types, missing_docs)] - impl #impl_generics ::dioxus::prelude::HasAttributes<#extend_lifetime> for #builder_name < #( #ty_generics ),* > #where_clause { + impl #impl_generics ::dioxus::prelude::HasAttributes for #builder_name < #( #ty_generics ),* > #where_clause { fn push_attribute( mut self, - name: &#extend_lifetime str, + name: &'static str, ns: Option<&'static str>, - attr: impl ::dioxus::prelude::IntoAttributeValue<#extend_lifetime>, + attr: impl ::dioxus::prelude::IntoAttributeValue, volatile: bool ) -> Self { let ( #(#descructuring,)* ) = self.fields; @@ -874,7 +840,7 @@ Finally, call `.build()` to create the instance of `{name}`. name, { use ::dioxus::prelude::IntoAttributeValue; - attr.into_value(self.bump) + attr.into_value() }, ns, volatile, @@ -882,7 +848,6 @@ Finally, call `.build()` to create the instance of `{name}`. ); #builder_name { #(#forward_extended_fields,)* - bump: self.bump, fields: ( #(#reconstructing,)* ), _phantom: self._phantom, } @@ -1007,11 +972,6 @@ Finally, call `.build()` to create the instance of `{name}`. let name = f.name; quote!(#name: self.#name) }); - let forward_bump = if self.extend_fields().next().is_some() { - quote!(bump: self.bump,) - } else { - quote!() - }; Ok(quote! { #[allow(dead_code, non_camel_case_types, missing_docs)] @@ -1023,7 +983,6 @@ Finally, call `.build()` to create the instance of `{name}`. let ( #(#descructuring,)* ) = self.fields; #builder_name { #(#forward_extended_fields,)* - #forward_bump fields: ( #(#reconstructing,)* ), _phantom: self._phantom, } diff --git a/packages/core/src/diff/node.rs b/packages/core/src/diff/node.rs index eadc1d2b1..b8777f2eb 100644 --- a/packages/core/src/diff/node.rs +++ b/packages/core/src/diff/node.rs @@ -7,7 +7,6 @@ use crate::{ arena::ElementId, innerlude::{ElementPath, ElementRef, VComponent, VNodeMount, VText}, nodes::DynamicNode, - nodes::RenderReturn, scopes::ScopeId, TemplateNode, TemplateNode::*, @@ -61,16 +60,7 @@ impl VNode { // If the templates are the same, we can diff the attributes and children // Start with the attributes - self.dynamic_attrs - .iter() - .zip(new.dynamic_attrs.iter()) - .enumerate() - .for_each(|(idx, (old_attr, new_attr))| { - // If the attributes are different (or volatile), we need to update them - if old_attr.value != new_attr.value || new_attr.volatile { - self.update_attribute(mount, idx, new_attr, to); - } - }); + self.diff_attributes(new, dom, to); // Now diff the dynamic nodes self.dynamic_nodes @@ -317,17 +307,108 @@ impl VNode { } } - pub(super) fn update_attribute( + pub(super) fn diff_attributes( &self, - mount: &VNodeMount, - idx: usize, - new_attr: &Attribute, + new: &VNode, + dom: &mut VirtualDom, to: &mut impl WriteMutations, ) { - let name = &new_attr.name; - let value = &new_attr.value; - let id = mount.mounted_attributes[idx]; - to.set_attribute(name, new_attr.namespace, value, id); + let mount_id = self.mount.get(); + for (idx, (old_attrs, new_attrs)) in self + .dynamic_attrs + .iter() + .zip(new.dynamic_attrs.iter()) + .enumerate() + { + let mut old_attributes_iter = old_attrs.iter().peekable(); + let mut new_attributes_iter = new_attrs.iter().peekable(); + let attribute_id = dom.mounts[mount_id.0].mounted_attributes[idx]; + let path = self.template.get().attr_paths[idx]; + + loop { + match (old_attributes_iter.peek(), new_attributes_iter.peek()) { + (Some(old_attribute), Some(new_attribute)) => { + // check which name is greater + match old_attribute.name.cmp(new_attribute.name) { + // The two attributes are the same, so diff them + std::cmp::Ordering::Equal => { + let old = old_attributes_iter.next().unwrap(); + let new = new_attributes_iter.next().unwrap(); + if old.value != new.value { + self.write_attribute( + path, + new, + attribute_id, + mount_id, + dom, + to, + ); + } + } + // In a sorted list, if the old attribute name is first, then the new attribute is missing + std::cmp::Ordering::Less => { + let old = old_attributes_iter.next().unwrap(); + self.remove_attribute(old, attribute_id, to) + } + // In a sorted list, if the new attribute name is first, then the old attribute is missing + std::cmp::Ordering::Greater => { + let new = new_attributes_iter.next().unwrap(); + self.write_attribute(path, new, attribute_id, mount_id, dom, to); + } + } + } + (Some(_), None) => { + let left = old_attributes_iter.next().unwrap(); + self.remove_attribute(left, attribute_id, to) + } + (None, Some(_)) => { + let right = new_attributes_iter.next().unwrap(); + self.write_attribute(path, right, attribute_id, mount_id, dom, to) + } + (None, None) => break, + } + } + } + } + + fn remove_attribute(&self, attribute: &Attribute, id: ElementId, to: &mut impl WriteMutations) { + match &attribute.value { + AttributeValue::Listener(_) => { + to.remove_event_listener(&attribute.name[2..], id); + } + _ => { + to.set_attribute( + attribute.name, + attribute.namespace, + &AttributeValue::None, + id, + ); + } + } + } + + fn write_attribute( + &self, + path: &'static [u8], + attribute: &Attribute, + id: ElementId, + mount: MountId, + dom: &mut VirtualDom, + to: &mut impl WriteMutations, + ) { + match &attribute.value { + AttributeValue::Listener(_) => { + let element_ref = ElementRef { + path: ElementPath { path }, + mount, + }; + dom.elements[id.0] = Some(element_ref); + to.create_event_listener(&attribute.name[2..], id); + } + _ => { + to.set_attribute(attribute.name, attribute.namespace, &attribute.value, id); + } + } } /// Lightly diff the two templates, checking only their roots. @@ -650,7 +731,9 @@ impl VNode { let id = self.assign_static_node_as_dynamic(path, root, dom, to); loop { - self.write_attribute(mount, attr_id, id, dom, to); + for attr in &*self.dynamic_attrs[attr_id] { + self.write_attribute(path, attr, id, mount, dom, to); + } // Only push the dynamic attributes forward if they match the current path (same element) match attrs.next_if(|(_, p)| *p == path) { @@ -661,40 +744,6 @@ impl VNode { } } - fn write_attribute( - &self, - mount: MountId, - idx: usize, - id: ElementId, - dom: &mut VirtualDom, - to: &mut impl WriteMutations, - ) { - // Make sure we set the attribute's associated id - dom.mounts[mount.0].mounted_attributes[idx] = id; - - let attribute = &self.dynamic_attrs[idx]; - - match &attribute.value { - AttributeValue::Listener(_) => { - // If this is a listener, we need to create an element reference for it so that when we receive an event, we can find the element - let path = &self.template.get().attr_paths[idx]; - - // The mount information should always be in the VDOM at this point - debug_assert!(dom.mounts.get(mount.0).is_some()); - - let element_ref = ElementRef { - path: ElementPath { path }, - mount, - }; - dom.elements[id.0] = Some(element_ref); - to.create_event_listener(&attribute.name[2..], id); - } - _ => { - to.set_attribute(attribute.name, attribute.namespace, &attribute.value, id); - } - } - } - fn load_template_root( &self, mount: MountId, diff --git a/packages/core/src/error_boundary.rs b/packages/core/src/error_boundary.rs index 51544377c..bbdd72054 100644 --- a/packages/core/src/error_boundary.rs +++ b/packages/core/src/error_boundary.rs @@ -7,7 +7,7 @@ use crate::{ use std::{ any::{Any, TypeId}, backtrace::Backtrace, - cell::{Cell, RefCell}, + cell::RefCell, error::Error, fmt::{Debug, Display}, rc::Rc, diff --git a/packages/core/src/lib.rs b/packages/core/src/lib.rs index b5db41e4e..56fbd2f74 100644 --- a/packages/core/src/lib.rs +++ b/packages/core/src/lib.rs @@ -74,9 +74,9 @@ pub(crate) mod innerlude { pub use crate::innerlude::{ fc_to_builder, generation, once, schedule_update, schedule_update_any, vdom_is_rendering, AnyValue, Attribute, AttributeValue, CapturedError, Component, DynamicNode, Element, ElementId, - Event, Fragment, IntoDynNode, Mutation, Mutations, NoOpMutations, Properties, RenderReturn, - ScopeId, Task, Template, TemplateAttribute, TemplateNode, VComponent, VNode, VNodeInner, - VPlaceholder, VText, VirtualDom, WriteMutations, + Event, Fragment, HasAttributes, IntoDynNode, Mutation, Mutations, NoOpMutations, Properties, + RenderReturn, ScopeId, Task, Template, TemplateAttribute, TemplateNode, VComponent, VNode, + VNodeInner, VPlaceholder, VText, VirtualDom, WriteMutations, }; /// The purpose of this module is to alleviate imports of many common types @@ -87,9 +87,9 @@ pub mod prelude { consume_context, consume_context_from_scope, current_scope_id, fc_to_builder, generation, has_context, needs_update, once, parent_scope, provide_context, provide_root_context, push_future, remove_future, schedule_update, schedule_update_any, spawn, spawn_forever, - suspend, use_error_boundary, use_hook, AnyValue, Component, Element, ErrorBoundary, Event, - EventHandler, Fragment, IntoAttributeValue, IntoDynNode, Properties, Runtime, RuntimeGuard, - ScopeId, Task, Template, TemplateAttribute, TemplateNode, Throw, VNode, VNodeInner, - VirtualDom, + suspend, use_error_boundary, use_hook, AnyValue, Attribute, Component, Element, + ErrorBoundary, Event, EventHandler, Fragment, HasAttributes, IntoAttributeValue, + IntoDynNode, Properties, Runtime, RuntimeGuard, ScopeId, Task, Template, TemplateAttribute, + TemplateNode, Throw, VNode, VNodeInner, VirtualDom, }; } diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index 5f24cc97d..bb3bcc98c 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -6,6 +6,7 @@ use crate::{ }; use crate::{arena::ElementId, Element, Event}; use crate::{Properties, VirtualDom}; +use core::panic; use std::ops::Deref; use std::rc::Rc; use std::vec; @@ -94,11 +95,34 @@ pub struct VNodeInner { /// The static nodes and static descriptor of the template pub template: Cell