diff --git a/packages/yew-macro/src/html_tree/html_element.rs b/packages/yew-macro/src/html_tree/html_element.rs index 24d0b7ac894..663f72797ce 100644 --- a/packages/yew-macro/src/html_tree/html_element.rs +++ b/packages/yew-macro/src/html_tree/html_element.rs @@ -1,6 +1,6 @@ use super::{HtmlChildrenTree, HtmlDashedName, TagTokens}; use crate::props::{ClassesForm, ElementProps, Prop}; -use crate::stringify::Stringify; +use crate::stringify::{Stringify, Value}; use crate::{non_capitalized_ascii, Peek, PeekValue}; use boolinator::Boolinator; use proc_macro2::{Delimiter, TokenStream}; @@ -8,7 +8,7 @@ use quote::{quote, quote_spanned, ToTokens}; use syn::buffer::Cursor; use syn::parse::{Parse, ParseStream}; use syn::spanned::Spanned; -use syn::{Block, Ident, Token}; +use syn::{Block, Expr, Ident, Lit, LitStr, Token}; pub struct HtmlElement { name: TagName, @@ -147,17 +147,35 @@ impl ToTokens for HtmlElement { let attributes = { let normal_attrs = attributes.iter().map(|Prop { label, value, .. }| { - let key = label.to_lit_str(); - let value = value.optimize_literals(); - quote! { - ::yew::virtual_dom::PositionalAttr::new(#key, #value) - } + (label.to_lit_str(), value.optimize_literals_tagged()) }); - let boolean_attrs = booleans.iter().map(|Prop { label, value, .. }| { + let boolean_attrs = booleans.iter().filter_map(|Prop { label, value, .. }| { let key = label.to_lit_str(); - quote! { - ::yew::virtual_dom::PositionalAttr::new_boolean(#key, #value) - } + Some(( + key.clone(), + match value { + Expr::Lit(e) => match &e.lit { + Lit::Bool(b) => Value::Static(if b.value { + quote! { #key } + } else { + return None; + }), + _ => Value::Dynamic(quote_spanned! {value.span()=> { + ::yew::utils::__ensure_type::(#value); + #key + }}), + }, + expr => Value::Dynamic(quote_spanned! {expr.span()=> + if #expr { + ::std::option::Option::Some( + ::std::borrow::Cow::<'static, str>::Borrowed(#key) + ) + } else { + None + } + }), + }, + )) }); let class_attr = classes.as_ref().and_then(|classes| match classes { ClassesForm::Tuple(classes) => { @@ -176,36 +194,78 @@ impl ToTokens for HtmlElement { }; }; - Some(quote! { - { - let mut __yew_classes = ::yew::html::Classes::with_capacity(#n); - #(__yew_classes.push(#classes);)* + Some(( + LitStr::new("class", span), + Value::Dynamic(quote! { + { + #deprecation_warning - #deprecation_warning - - ::yew::virtual_dom::PositionalAttr::new("class", __yew_classes) - } - }) + let mut __yew_classes = ::yew::html::Classes::with_capacity(#n); + #(__yew_classes.push(#classes);)* + __yew_classes + } + }), + )) } - ClassesForm::Single(classes) => match classes.try_into_lit() { - Some(lit) => { - if lit.value().is_empty() { - None - } else { - let sr = lit.stringify(); - Some(quote! { ::yew::virtual_dom::PositionalAttr::new("class", #sr) }) + ClassesForm::Single(classes) => { + match classes.try_into_lit() { + Some(lit) => { + if lit.value().is_empty() { + None + } else { + Some(( + LitStr::new("class", lit.span()), + Value::Static(quote! { #lit }), + )) + } + } + None => { + Some(( + LitStr::new("class", classes.span()), + Value::Dynamic(quote! { + ::std::convert::Into::<::yew::html::Classes>::into(#classes) + }), + )) } - } - None => { - Some(quote! { ::yew::virtual_dom::PositionalAttr::new("class", ::std::convert::Into::<::yew::html::Classes>::into(#classes)) }) } } }); - let attrs = normal_attrs.chain(boolean_attrs).chain(class_attr); - quote! { - ::yew::virtual_dom::Attributes::Vec(::std::vec![#(#attrs),*]) + /// Try to turn attribute list into a `::yew::virtual_dom::Attributes::Static` + fn try_into_static(src: &[(LitStr, Value)]) -> Option { + let mut kv = Vec::with_capacity(src.len()); + for (k, v) in src.iter() { + let v = match v { + Value::Static(v) => quote! { #v }, + Value::Dynamic(_) => return None, + }; + kv.push(quote! { [ #k, #v ] }); + } + + Some(quote! { ::yew::virtual_dom::Attributes::Static(&[#(#kv),*]) }) } + + let attrs = normal_attrs + .chain(boolean_attrs) + .chain(class_attr) + .collect::>(); + try_into_static(&attrs).unwrap_or_else(|| { + let keys = attrs.iter().map(|(k, _)| quote! { #k }); + let values = attrs.iter().map(|(_, v)| { + quote_spanned! {v.span()=> + ::yew::html::IntoPropValue::< + ::std::option::Option::<::yew::virtual_dom::AttrValue> + > + ::into_prop_value(#v) + } + }); + quote! { + ::yew::virtual_dom::Attributes::Dynamic{ + keys: &[#(#keys),*], + values: ::std::boxed::Box::new([#(#values),*]), + } + } + }) }; let listeners = if listeners.is_empty() { @@ -291,9 +351,7 @@ impl ToTokens for HtmlElement { let handle_value_attr = props.value.as_ref().map(|prop| { let v = prop.value.optimize_literals(); quote_spanned! {v.span()=> { - __yew_vtag.__macro_push_attr( - ::yew::virtual_dom::PositionalAttr::new("value", #v), - ); + __yew_vtag.__macro_push_attr("value", #v); }} }); diff --git a/packages/yew-macro/src/stringify.rs b/packages/yew-macro/src/stringify.rs index 291d9147b9b..fa6a7b6b4ec 100644 --- a/packages/yew-macro/src/stringify.rs +++ b/packages/yew-macro/src/stringify.rs @@ -21,13 +21,21 @@ pub trait Stringify { /// Optimize literals to `&'static str`, otherwise keep the value as is. fn optimize_literals(&self) -> TokenStream + where + Self: ToTokens, + { + self.optimize_literals_tagged().to_token_stream() + } + + /// Like `optimize_literals` but tags static or dynamic strings with [Value] + fn optimize_literals_tagged(&self) -> Value where Self: ToTokens, { if let Some(lit) = self.try_into_lit() { - lit.to_token_stream() + Value::Static(lit.to_token_stream()) } else { - self.to_token_stream() + Value::Dynamic(self.to_token_stream()) } } } @@ -41,6 +49,21 @@ impl Stringify for &T { } } +/// A stringified value that can be either static (known at compile time) or dynamic (known only at +/// runtime) +pub enum Value { + Static(TokenStream), + Dynamic(TokenStream), +} + +impl ToTokens for Value { + fn to_tokens(&self, tokens: &mut TokenStream) { + tokens.extend(match self { + Value::Static(tt) | Value::Dynamic(tt) => tt.clone(), + }); + } +} + impl Stringify for LitStr { fn try_into_lit(&self) -> Option { Some(self.clone()) diff --git a/packages/yew-macro/tests/classes_macro/classes-fail.stderr b/packages/yew-macro/tests/classes_macro/classes-fail.stderr index df4ac0aac3a..4ae13819a9a 100644 --- a/packages/yew-macro/tests/classes_macro/classes-fail.stderr +++ b/packages/yew-macro/tests/classes_macro/classes-fail.stderr @@ -51,9 +51,9 @@ error[E0277]: the trait bound `Classes: From<{integer}>` is not satisfied > and 4 others = note: required because of the requirements on the impl of `Into` for `{integer}` - = note: required because of the requirements on the impl of `From>` for `Classes` + = note: required because of the requirements on the impl of `From>` for `Classes` = note: 1 redundant requirements hidden - = note: required because of the requirements on the impl of `Into` for `std::vec::Vec<{integer}>` + = note: required because of the requirements on the impl of `Into` for `Vec<{integer}>` error[E0277]: the trait bound `Classes: From<{integer}>` is not satisfied --> $DIR/classes-fail.rs:13:14 diff --git a/packages/yew-macro/tests/html_macro/element-fail.stderr b/packages/yew-macro/tests/html_macro/element-fail.stderr index f6f00fcaaa1..9a5e031565f 100644 --- a/packages/yew-macro/tests/html_macro/element-fail.stderr +++ b/packages/yew-macro/tests/html_macro/element-fail.stderr @@ -177,10 +177,7 @@ error[E0277]: the trait bound `(): IntoPropValue>>` is 43 | html! { }; | ^^ the trait `IntoPropValue>>` is not implemented for `()` | - ::: $WORKSPACE/packages/yew/src/virtual_dom/mod.rs - | - | pub fn new(key: &'static str, value: impl IntoPropValue>) -> Self { - | -------------------------------- required by this bound in `PositionalAttr::new` + = note: required by `into_prop_value` error[E0277]: the trait bound `(): IntoPropValue>>` is not satisfied --> $DIR/element-fail.rs:44:27 @@ -196,10 +193,7 @@ error[E0277]: the trait bound `(): IntoPropValue>>` is 45 | html! { }; | ^^ the trait `IntoPropValue>>` is not implemented for `()` | - ::: $WORKSPACE/packages/yew/src/virtual_dom/mod.rs - | - | pub fn new(key: &'static str, value: impl IntoPropValue>) -> Self { - | -------------------------------- required by this bound in `PositionalAttr::new` + = note: required by `into_prop_value` error[E0277]: the trait bound `NotToString: IntoPropValue>>` is not satisfied --> $DIR/element-fail.rs:46:28 @@ -207,10 +201,7 @@ error[E0277]: the trait bound `NotToString: IntoPropValue }; | ^^^^^^^^^^^ the trait `IntoPropValue>>` is not implemented for `NotToString` | - ::: $WORKSPACE/packages/yew/src/virtual_dom/mod.rs - | - | pub fn new(key: &'static str, value: impl IntoPropValue>) -> Self { - | -------------------------------- required by this bound in `PositionalAttr::new` + = note: required by `into_prop_value` error[E0277]: the trait bound `Option: IntoPropValue>>` is not satisfied --> $DIR/element-fail.rs:47:23 @@ -218,15 +209,11 @@ error[E0277]: the trait bound `Option: IntoPropValue }; | ^^^^^^^^^^^^^^^^^ the trait `IntoPropValue>>` is not implemented for `Option` | - ::: $WORKSPACE/packages/yew/src/virtual_dom/mod.rs - | - | pub fn new(key: &'static str, value: impl IntoPropValue>) -> Self { - | -------------------------------- required by this bound in `PositionalAttr::new` - | = help: the following implementations were found: as IntoPropValue>>> as IntoPropValue>> as IntoPropValue>>> + = note: required by `into_prop_value` error[E0277]: the trait bound `Option<{integer}>: IntoPropValue>>` is not satisfied --> $DIR/element-fail.rs:48:22 @@ -234,15 +221,11 @@ error[E0277]: the trait bound `Option<{integer}>: IntoPropValue }; | ^^^^^^^ the trait `IntoPropValue>>` is not implemented for `Option<{integer}>` | - ::: $WORKSPACE/packages/yew/src/virtual_dom/mod.rs - | - | pub fn new(key: &'static str, value: impl IntoPropValue>) -> Self { - | -------------------------------- required by this bound in `PositionalAttr::new` - | = help: the following implementations were found: as IntoPropValue>>> as IntoPropValue>> as IntoPropValue>>> + = note: required by `into_prop_value` error[E0277]: the trait bound `{integer}: IntoPropValue>>` is not satisfied --> $DIR/element-fail.rs:51:28 diff --git a/packages/yew-macro/tests/props_macro/resolve-prop-fail.stderr b/packages/yew-macro/tests/props_macro/resolve-prop-fail.stderr index 45d2f67d396..142448008b5 100644 --- a/packages/yew-macro/tests/props_macro/resolve-prop-fail.stderr +++ b/packages/yew-macro/tests/props_macro/resolve-prop-fail.stderr @@ -1,8 +1,8 @@ -error[E0277]: the trait bound `std::vec::Vec<_>: yew::Properties` is not satisfied +error[E0277]: the trait bound `Vec<_>: yew::Properties` is not satisfied --> $DIR/resolve-prop-fail.rs:38:17 | 38 | yew::props!(Vec<_> {}); - | ^^^ the trait `yew::Properties` is not implemented for `std::vec::Vec<_>` + | ^^^ the trait `yew::Properties` is not implemented for `Vec<_>` | = note: required by `builder` diff --git a/packages/yew/src/html/classes.rs b/packages/yew/src/html/classes.rs index ac8fcec4390..fd77ca382cb 100644 --- a/packages/yew/src/html/classes.rs +++ b/packages/yew/src/html/classes.rs @@ -3,6 +3,7 @@ use crate::virtual_dom::AttrValue; use indexmap::IndexSet; use std::{ borrow::{Borrow, Cow}, + hint::unreachable_unchecked, iter::FromIterator, }; @@ -65,9 +66,14 @@ impl Classes { } impl IntoPropValue for Classes { + #[inline] fn into_prop_value(mut self) -> AttrValue { if self.set.len() == 1 { - self.set.pop().unwrap() + match self.set.pop() { + Some(attr) => attr, + // SAFETY: the collection is checked to be non-empty above + None => unsafe { unreachable_unchecked() }, + } } else { Cow::Owned(self.to_string()) } @@ -75,6 +81,7 @@ impl IntoPropValue for Classes { } impl IntoPropValue> for Classes { + #[inline] fn into_prop_value(self) -> Option { if self.is_empty() { None diff --git a/packages/yew/src/utils/mod.rs b/packages/yew/src/utils/mod.rs index c9c0c52a8d8..ecf700c1100 100644 --- a/packages/yew/src/utils/mod.rs +++ b/packages/yew/src/utils/mod.rs @@ -95,6 +95,12 @@ impl IntoIterator for NodeSeq { } } +/// Hack to force type mismatch compile errors in yew-macro. +// +// TODO: replace with `compile_error!`, when `type_name_of_val` is stabilised (https://github.com/rust-lang/rust/issues/66359). +#[doc(hidden)] +pub fn __ensure_type(_: T) {} + /// Print the [web_sys::Node]'s contents as a string for debugging purposes pub fn print_node(n: &web_sys::Node) -> String { use wasm_bindgen::JsCast; diff --git a/packages/yew/src/virtual_dom/mod.rs b/packages/yew/src/virtual_dom/mod.rs index b6b118208ea..7a2d1502304 100644 --- a/packages/yew/src/virtual_dom/mod.rs +++ b/packages/yew/src/virtual_dom/mod.rs @@ -13,10 +13,10 @@ pub mod vtag; #[doc(hidden)] pub mod vtext; -use crate::html::{AnyScope, IntoPropValue, NodeRef}; +use crate::html::{AnyScope, NodeRef}; use gloo::events::EventListener; use indexmap::IndexMap; -use std::{borrow::Cow, collections::HashMap, fmt, hint::unreachable_unchecked, iter, mem}; +use std::{borrow::Cow, collections::HashMap, fmt, hint::unreachable_unchecked, iter}; use web_sys::{Element, Node}; #[doc(inline)] @@ -62,71 +62,48 @@ trait Apply { fn apply_diff(&mut self, el: &Self::Element, ancestor: Self); } -/// Key-value tuple which makes up an item of the [`Attributes::Vec`] variant. -#[derive(Clone, Debug, Eq, PartialEq)] -pub struct PositionalAttr(pub &'static str, pub Option); -impl PositionalAttr { - /// Create a positional attribute - #[inline] - pub fn new(key: &'static str, value: impl IntoPropValue>) -> Self { - Self(key, value.into_prop_value()) - } - - /// Create a boolean attribute. - /// `present` controls whether the attribute is added - #[inline] - pub fn new_boolean(key: &'static str, present: bool) -> Self { - let value = if present { - Some(Cow::Borrowed(key)) - } else { - None - }; - Self::new(key, value) - } - - /// Create a placeholder for removed attributes - #[inline] - pub fn new_placeholder(key: &'static str) -> Self { - Self(key, None) - } - - #[inline] - fn transpose(self) -> Option<(&'static str, AttrValue)> { - let Self(key, value) = self; - value.map(|v| (key, v)) - } - - #[inline] - fn transposed<'a>(&'a self) -> Option<(&'static str, &'a AttrValue)> { - let Self(key, value) = self; - value.as_ref().map(|v| (*key, v)) - } -} - /// A collection of attributes for an element #[derive(PartialEq, Eq, Clone, Debug)] pub enum Attributes { - /// A vector is ideal because most of the time the list will neither change - /// length nor key order. - Vec(Vec), + /// Static list of attributes. + /// + /// Allows optimizing comparison to a simple pointer equality check and reducing allocations, + /// if the attributes do not change on a node. + Static(&'static [[&'static str; 2]]), + + /// Static list of attribute keys with possibility to exclude attributes and dynamic attribute + /// values. + /// + /// Allows optimizing comparison to a simple pointer equality check and reducing allocations, + /// if the attributes keys do not change on a node. + Dynamic { + /// Attribute keys. Includes both always set and optional attribute keys. + keys: &'static [&'static str], + + /// Attribute values. Matches [keys]. Optional attributes are designated by setting [None]. + values: Box<[Option]>, + }, /// IndexMap is used to provide runtime attribute deduplication in cases where the html! macro /// was not used to guarantee it. IndexMap(IndexMap<&'static str, AttrValue>), } + impl Attributes { /// Construct a default Attributes instance pub fn new() -> Self { Self::default() } - /// Return iterator over attribute key-value pairs + /// Return iterator over attribute key-value pairs. + /// This function is suboptimal and does not inline well. Avoid on hot paths. pub fn iter<'a>(&'a self) -> Box + 'a> { match self { - Self::Vec(v) => Box::new( - v.iter() - .filter_map(PositionalAttr::transposed) - .map(|(k, v)| (k, v.as_ref())), + Self::Static(arr) => Box::new(arr.iter().map(|kv| (kv[0], kv[1] as &'a str))), + Self::Dynamic { keys, values } => Box::new( + keys.iter() + .zip(values.iter()) + .filter_map(|(k, v)| v.as_ref().map(|v| (*k, v.as_ref()))), ), Self::IndexMap(m) => Box::new(m.iter().map(|(k, v)| (*k, v.as_ref()))), } @@ -135,114 +112,46 @@ impl Attributes { /// Get a mutable reference to the underlying `IndexMap`. /// If the attributes are stored in the `Vec` variant, it will be converted. pub fn get_mut_index_map(&mut self) -> &mut IndexMap<&'static str, AttrValue> { - match self { - Self::IndexMap(m) => m, - Self::Vec(v) => { - *self = Self::IndexMap( - mem::take(v) - .into_iter() - .filter_map(PositionalAttr::transpose) - .collect(), - ); + macro_rules! unpack { + () => { match self { Self::IndexMap(m) => m, - // SAFETY: unreachable because we set the value to the `IndexMap` variant above. + // SAFETY: unreachable because we set self to the `IndexMap` variant above. _ => unsafe { unreachable_unchecked() }, } - } + }; } - } - - fn diff_vec<'a>( - new: &'a [PositionalAttr], - old: &[PositionalAttr], - ) -> Vec> { - let mut out = Vec::new(); - let mut new_iter = new.iter(); - let mut old_iter = old.iter(); - loop { - match (new_iter.next(), old_iter.next()) { - ( - Some(PositionalAttr(key, new_value)), - Some(PositionalAttr(old_key, old_value)), - ) if key == old_key => match (new_value, old_value) { - (Some(new), Some(old)) => { - if new != old { - out.push(Patch::Replace(*key, new.as_ref())); - } - } - (Some(value), None) => out.push(Patch::Add(*key, value.as_ref())), - (None, Some(_)) => out.push(Patch::Remove(*key)), - (None, None) => {} - }, - // keys don't match, we can no longer compare linearly from here on out - (Some(new_attr), Some(old_attr)) => { - // assume that every attribute is new. - let mut added = iter::once(new_attr) - .chain(new_iter) - .filter_map(PositionalAttr::transposed) - .map(|(key, value)| (key, value.as_ref())) - .collect::>(); - - // now filter out all the attributes that aren't new - for (key, old_value) in iter::once(old_attr) - .chain(old_iter) - .filter_map(PositionalAttr::transposed) - { - if let Some(new_value) = added.remove(key) { - // attribute still exists but changed value - if new_value != old_value.as_ref() { - out.push(Patch::Replace(key, new_value)); - } - } else { - // attribute no longer exists - out.push(Patch::Remove(key)); - } - } - - // finally, we're left with the attributes that are actually new. - out.extend(added.into_iter().map(|(k, v)| Patch::Add(k, v))); - break; - } - // added attributes - (Some(attr), None) => { - for PositionalAttr(key, value) in iter::once(attr).chain(new_iter) { - // only add value if it has a value - if let Some(value) = value { - out.push(Patch::Add(*key, value)); - } - } - break; - } - // removed attributes - (None, Some(attr)) => { - for PositionalAttr(key, value) in iter::once(attr).chain(old_iter) { - // only remove the attribute if it had a value before - if value.is_some() { - out.push(Patch::Remove(*key)); - } - } - break; - } - (None, None) => break, + match self { + Self::IndexMap(m) => m, + Self::Static(arr) => { + *self = Self::IndexMap(arr.iter().map(|kv| (kv[0], kv[1].into())).collect()); + unpack!() + } + Self::Dynamic { keys, values } => { + *self = Self::IndexMap( + std::mem::take(values) + .iter_mut() + .zip(keys.iter()) + .filter_map(|(v, k)| v.take().map(|v| (*k, v))) + .collect(), + ); + unpack!() } } - - out } - fn diff_index_map<'a, A, B>( + #[cold] + fn apply_diff_index_maps<'a, A, B>( + el: &Element, // this makes it possible to diff `&'a IndexMap<_, A>` and `IndexMap<_, &'a A>`. mut new_iter: impl Iterator, new: &IndexMap<&'static str, A>, old: &IndexMap<&'static str, B>, - ) -> Vec> - where + ) where A: AsRef, B: AsRef, { - let mut out = Vec::new(); let mut old_iter = old.iter(); loop { match (new_iter.next(), old_iter.next()) { @@ -251,7 +160,7 @@ impl Attributes { break; } if new_value != old_value.as_ref() { - out.push(Patch::Replace(new_key, new_value)); + Self::set_attribute(el, new_key, new_value); } } // new attributes @@ -260,10 +169,12 @@ impl Attributes { match old.get(key) { Some(old_value) => { if value != old_value.as_ref() { - out.push(Patch::Replace(key, value)); + Self::set_attribute(el, key, value); } } - None => out.push(Patch::Add(key, value)), + None => { + Self::set_attribute(el, key, value); + } } } break; @@ -272,7 +183,7 @@ impl Attributes { (None, Some(attr)) => { for (key, _) in iter::once(attr).chain(old_iter) { if !new.contains_key(key) { - out.push(Patch::Remove(*key)); + Self::remove_attribute(el, key); } } break; @@ -280,35 +191,43 @@ impl Attributes { (None, None) => break, } } - - out } - fn diff<'a>(new: &'a Self, old: &'a Self) -> Vec> { - match (new, old) { - (Self::Vec(new), Self::Vec(old)) => Self::diff_vec(new, old), - (Self::Vec(new), Self::IndexMap(old)) => { - // this case is somewhat tricky because we need to return references to the values in `new` - // but we also want to turn `new` into a hash map for performance reasons - let new_iter = new + /// Convert [Attributes] pair to [HashMap]s and patch changes to `el`. + /// Works with any [Attributes] variants. + #[cold] + fn apply_diff_as_maps<'a>(el: &Element, new: &'a Self, old: &'a Self) { + fn collect<'a>(src: &'a Attributes) -> HashMap<&'static str, &'a str> { + use Attributes::*; + + match src { + Static(arr) => (*arr).iter().map(|[k, v]| (*k, *v)).collect(), + Dynamic { keys, values } => keys .iter() - .filter_map(PositionalAttr::transposed) - .map(|(k, v)| (k, v.as_ref())); - // create a "view" over references to the actual data in `new`. - let new = new.iter().filter_map(PositionalAttr::transposed).collect(); - Self::diff_index_map(new_iter, &new, old) + .zip(values.iter()) + .filter_map(|(k, v)| v.as_ref().map(|v| (*k, v.as_ref()))) + .collect(), + IndexMap(m) => m.iter().map(|(k, v)| (*k, v.as_ref())).collect(), } - (Self::IndexMap(new), Self::Vec(old)) => { - let new_iter = new.iter().map(|(k, v)| (*k, v.as_ref())); - Self::diff_index_map( - new_iter, - new, - &old.iter().filter_map(PositionalAttr::transposed).collect(), - ) + } + + let new = collect(new); + let old = collect(old); + + // Update existing or set new + for (k, new) in new.iter() { + if match old.get(k) { + Some(old) => old != new, + None => true, + } { + el.set_attribute(k, new).unwrap(); } - (Self::IndexMap(new), Self::IndexMap(old)) => { - let new_iter = new.iter().map(|(k, v)| (*k, v.as_ref())); - Self::diff_index_map(new_iter, new, old) + } + + // Remove missing + for k in old.keys() { + if !new.contains_key(k) { + Self::remove_attribute(el, k); } } } @@ -316,6 +235,11 @@ impl Attributes { fn set_attribute(el: &Element, key: &str, value: &str) { el.set_attribute(key, value).expect("invalid attribute key") } + + fn remove_attribute(el: &Element, key: &str) { + el.remove_attribute(key) + .expect("could not remove attribute") + } } impl Apply for Attributes { @@ -323,10 +247,15 @@ impl Apply for Attributes { fn apply(&mut self, el: &Element) { match self { - Self::Vec(v) => { - for attr in v.iter() { - if let Some(v) = &attr.1 { - Self::set_attribute(el, attr.0, v) + Self::Static(arr) => { + for kv in arr.iter() { + Self::set_attribute(el, kv[0], kv[1]); + } + } + Self::Dynamic { keys, values } => { + for (k, v) in keys.iter().zip(values.iter()) { + if let Some(v) = v { + Self::set_attribute(el, k, v) } } } @@ -339,25 +268,68 @@ impl Apply for Attributes { } fn apply_diff(&mut self, el: &Element, ancestor: Self) { - for change in Self::diff(self, &ancestor) { - match change { - Patch::Add(key, value) | Patch::Replace(key, value) => { - Self::set_attribute(el, key, value); - } - Patch::Remove(key) => { - el.remove_attribute(key) - .expect("could not remove attribute"); + #[inline] + fn ptr_eq(a: &[T], b: &[T]) -> bool { + a.as_ptr() == b.as_ptr() + } + + match (self, ancestor) { + // Hot path + (Self::Static(new), Self::Static(old)) if ptr_eq(new, old) => (), + // Hot path + ( + Self::Dynamic { + keys: new_k, + values: new_v, + }, + Self::Dynamic { + keys: old_k, + values: old_v, + }, + ) if ptr_eq(new_k, old_k) => { + // Double zipping does not optimize well, so use asserts and unsafe instead + assert!(new_k.len() == new_v.len()); + assert!(new_k.len() == old_v.len()); + for i in 0..new_k.len() { + macro_rules! key { + () => { + unsafe { new_k.get_unchecked(i) } + }; + } + macro_rules! set { + ($new:expr) => { + Self::set_attribute(el, key!(), $new); + }; + } + + match unsafe { (new_v.get_unchecked(i), old_v.get_unchecked(i)) } { + (Some(new), Some(old)) => { + if new != old { + set!(new); + } + } + (Some(new), None) => set!(new), + (None, Some(_)) => { + Self::remove_attribute(el, key!()); + } + (None, None) => (), + } } } + // For VTag's constructed outside the html! macro + (Self::IndexMap(new), Self::IndexMap(old)) => { + let new_iter = new.iter().map(|(k, v)| (*k, v.as_ref())); + Self::apply_diff_index_maps(el, new_iter, new, &old); + } + // Cold path. Happens only with conditional swapping and reordering of `VTag`s with the + // same tag and no keys. + (new, ancestor) => { + Self::apply_diff_as_maps(el, new, &ancestor); + } } } } -impl From> for Attributes { - fn from(v: Vec) -> Self { - Self::Vec(v) - } -} impl From> for Attributes { fn from(v: IndexMap<&'static str, AttrValue>) -> Self { Self::IndexMap(v) @@ -366,18 +338,10 @@ impl From> for Attributes { impl Default for Attributes { fn default() -> Self { - Self::Vec(Default::default()) + Self::Static(&[]) } } -/// Patch for DOM node modification. -#[derive(Debug, PartialEq)] -enum Patch { - Add(ID, T), - Replace(ID, T), - Remove(ID), -} - // TODO(#938): What about implementing `VDiff` for `Element`? // It would make it possible to include ANY element into the tree. // `Ace` editor embedding for example? @@ -573,120 +537,187 @@ mod layout_tests { #[cfg(all(test, feature = "wasm_bench"))] mod benchmarks { - use super::{Attributes, PositionalAttr}; - use std::borrow::Cow; + use super::*; use wasm_bindgen_test::{wasm_bindgen_test, wasm_bindgen_test_configure}; wasm_bindgen_test_configure!(run_in_browser); - fn create_pos_attrs() -> Vec { - vec![ - PositionalAttr::new("oh", Cow::Borrowed("danny")), - PositionalAttr::new("boy", Cow::Borrowed("the")), - PositionalAttr::new("pipes", Cow::Borrowed("the")), - PositionalAttr::new("are", Cow::Borrowed("calling")), - PositionalAttr::new("from", Cow::Borrowed("glen")), - PositionalAttr::new("to", Cow::Borrowed("glen")), - PositionalAttr::new("and", Cow::Borrowed("down")), - PositionalAttr::new("the", Cow::Borrowed("mountain")), - PositionalAttr::new("side", Cow::Borrowed("")), - ] - } + macro_rules! run { + ($name:ident => { + $( $old:expr => $new:expr )+ + }) => { + // NB: these benchmarks only compare diffing. They do not take into account aspects like + // allocation impact, which is lower for both `Static` and `Dynamic`. - fn run_benchmarks(name: &str, new: Vec, old: Vec) { - let new_vec = Attributes::from(new); - let old_vec = Attributes::from(old); - - let mut new_map = new_vec.clone(); - let _ = new_map.get_mut_index_map(); - let mut old_map = old_vec.clone(); - let _ = old_map.get_mut_index_map(); - - const TIME_LIMIT: f64 = 2.0; - - let vv = easybench_wasm::bench_env_limit(TIME_LIMIT, (&new_vec, &old_vec), |(new, old)| { - format!("{:?}", Attributes::diff(new, old)) - }); - let mm = easybench_wasm::bench_env_limit(TIME_LIMIT, (&new_map, &old_map), |(new, old)| { - format!("{:?}", Attributes::diff(new, old)) - }); - - let vm = easybench_wasm::bench_env_limit(TIME_LIMIT, (&new_vec, &old_map), |(new, old)| { - format!("{:?}", Attributes::diff(new, old)) - }); - let mv = easybench_wasm::bench_env_limit(TIME_LIMIT, (&new_map, &old_vec), |(new, old)| { - format!("{:?}", Attributes::diff(new, old)) - }); - - wasm_bindgen_test::console_log!( - "{}:\n\tvec-vec: {}\n\tmap-map: {}\n\tvec-map: {}\n\tmap-vec: {}", - name, - vv, - mm, - vm, - mv - ); + let results = vec![ + $( + { + let mut old = $old.clone(); + let new = $new.clone(); + let el = crate::utils::document().create_element("div").unwrap(); + old.apply(&el); + ( + format!("{} -> {}", attr_variant(&old), attr_variant(&new)), + easybench_wasm::bench_env_limit( + 2.0, + (NodeCloner(el), new, old), + |(el, mut new, old)| new.apply_diff(&el.0, old), + ), + ) + }, + )+ + ]; + + let max_name_len = results.iter().map(|(name, _)| name.len()).max().unwrap_or_default(); + wasm_bindgen_test::console_log!( + "{}:{}", + stringify!($name), + results.into_iter().fold(String::new(), |mut acc, (name, res)| { + use std::fmt::Write; + + write!(&mut acc, "\n\t\t{:7.4} ns (R²={:.3}, {:>7} iterations in {:>3} samples)", + res.ns_per_iter, + res.goodness_of_fit, + res.iterations, + res.samples, + ) + .unwrap(); + } + + acc + }) + ); + }; } #[wasm_bindgen_test] - fn bench_diff_attributes_equal() { - let old = create_pos_attrs(); - let new = old.clone(); + fn bench_diff_empty() { + let static_ = Attributes::Static(&[]); + let dynamic = Attributes::Dynamic { + keys: &[], + values: vec![], + }; + let map = Attributes::IndexMap(Default::default()); + + run! { + empty => { + static_ => static_ + dynamic => dynamic + map => map + static_ => dynamic + static_ => map + dynamic => map + } + } + } - run_benchmarks("equal", new, old); + #[wasm_bindgen_test] + fn bench_diff_equal() { + let static_ = Attributes::Static(sample_attrs()); + let dynamic = make_dynamic(sample_values()); + let map = make_indexed_map(sample_values()); + + run! { + equal => { + static_ => static_ + dynamic => dynamic + map => map + static_ => dynamic + static_ => map + dynamic => map + } + } } #[wasm_bindgen_test] - fn bench_diff_attributes_length_end() { - let old = create_pos_attrs(); + fn bench_diff_change_first() { + let old = sample_values(); let mut new = old.clone(); - new.push(PositionalAttr::new("hidden", Cow::Borrowed("hidden"))); + new[0] = AttrValue::Borrowed("changed"); + + let dynamic = (make_dynamic(old.clone()), make_dynamic(new.clone())); + let map = (make_indexed_map(old), make_indexed_map(new)); - run_benchmarks("added to end", new.clone(), old.clone()); - run_benchmarks("removed from end", old, new); + run! { + changed_first => { + dynamic.0 => dynamic.1 + map.0 => map.1 + dynamic.0 => map.1 + } + } } - #[wasm_bindgen_test] - fn bench_diff_attributes_length_start() { - let old = create_pos_attrs(); - let mut new = old.clone(); - new.insert(0, PositionalAttr::new("hidden", Cow::Borrowed("hidden"))); - run_benchmarks("added to start", new.clone(), old.clone()); - run_benchmarks("removed from start", old, new); + fn make_dynamic(values: Vec) -> Attributes { + Attributes::Dynamic { + keys: sample_keys(), + values: values.into_iter().map(|v| Some(v)).collect(), + } } - #[wasm_bindgen_test] - fn bench_diff_attributes_reorder() { - let old = create_pos_attrs(); - let new = old.clone().into_iter().rev().collect(); + fn make_indexed_map(values: Vec) -> Attributes { + Attributes::IndexMap( + sample_keys() + .iter() + .copied() + .zip(values.into_iter()) + .collect(), + ) + } - run_benchmarks("reordered", new, old); + fn sample_keys() -> &'static [&'static str] { + &[ + "oh", "boy", "pipes", "are", "from", "to", "and", "the", "side", + ] } - #[wasm_bindgen_test] - fn bench_diff_attributes_change_first() { - let old = create_pos_attrs(); - let mut new = old.clone(); - new[0].1 = Some(Cow::Borrowed("changed")); + fn sample_values() -> Vec { + [ + "danny", "the", "the", "calling", "glen", "glen", "down", "mountain", "", + ] + .iter() + .map(|v| AttrValue::Borrowed(*v)) + .collect() + } - run_benchmarks("changed first", new, old); + fn sample_attrs() -> &'static [[&'static str; 2]] { + &[ + ["oh", "danny"], + ["boy", "the"], + ["pipes", "the"], + ["are", "calling"], + ["from", "glen"], + ["to", "glen"], + ["and", "down"], + ["the", "mountain"], + ["side", ""], + ] } - #[wasm_bindgen_test] - fn bench_diff_attributes_change_middle() { - let old = create_pos_attrs(); - let mut new = old.clone(); - new[old.len() / 2].1 = Some(Cow::Borrowed("changed")); + fn attr_variant(attrs: &Attributes) -> &'static str { + use Attributes::*; - run_benchmarks("changed middle", new, old); + match attrs { + Static(_) => "static", + Dynamic { .. } => "dynamic", + IndexMap(_) => "indexed_map", + } } - #[wasm_bindgen_test] - fn bench_diff_attributes_change_last() { - let old = create_pos_attrs(); - let mut new = old.clone(); - new[old.len() - 1].1 = Some(Cow::Borrowed("changed")); + /// Clones the node on [Clone] call + struct NodeCloner(Element); - run_benchmarks("changed last", new, old); + impl Clone for NodeCloner { + fn clone(&self) -> Self { + use wasm_bindgen::JsCast; + + Self(self.0.clone_node().unwrap().dyn_into().unwrap()) + } } } diff --git a/packages/yew/src/virtual_dom/vtag.rs b/packages/yew/src/virtual_dom/vtag.rs index f45448328d9..e474a3e69a4 100644 --- a/packages/yew/src/virtual_dom/vtag.rs +++ b/packages/yew/src/virtual_dom/vtag.rs @@ -1,6 +1,6 @@ //! This module contains the implementation of a virtual element node [VTag]. -use super::{Apply, AttrValue, Attributes, Key, Listener, PositionalAttr, VDiff, VList, VNode}; +use super::{Apply, AttrValue, Attributes, Key, Listener, VDiff, VList, VNode}; use crate::html::{AnyScope, IntoPropValue, NodeRef}; use crate::utils::document; use gloo::events::EventListener; @@ -217,7 +217,7 @@ pub struct VTag { /// List of attached listeners. listeners: Listeners, - /// A reference to the DOM `Element`. + /// A reference to the DOM [`Element`]. reference: Option, /// A node reference used for DOM access in Component lifecycle methods @@ -491,11 +491,10 @@ impl VTag { } #[doc(hidden)] - pub fn __macro_push_attr(&mut self, attr: PositionalAttr) { - match &mut self.attributes { - Attributes::Vec(attrs) => attrs.push(attr), - _ => unreachable!("the macro always uses positional attributes"), - } + pub fn __macro_push_attr(&mut self, key: &'static str, value: impl IntoPropValue) { + self.attributes + .get_mut_index_map() + .insert(key, value.into_prop_value()); } /// Adds new listener to the node. @@ -685,7 +684,7 @@ impl PartialEq for VTag { #[cfg(test)] mod tests { use super::*; - use crate::html; + use crate::{html, Html}; #[cfg(feature = "wasm_test")] use wasm_bindgen_test::{wasm_bindgen_test as test, wasm_bindgen_test_configure}; @@ -734,7 +733,7 @@ mod tests { } #[test] - fn it_compares_attributes() { + fn it_compares_attributes_static() { let a = html! {
}; @@ -751,6 +750,24 @@ mod tests { assert_ne!(a, c); } + #[test] + fn it_compares_attributes_dynamic() { + let a = html! { +
+ }; + + let b = html! { +
+ }; + + let c = html! { +
+ }; + + assert_eq!(a, b); + assert_ne!(a, c); + } + #[test] fn it_compares_children() { let a = html! { @@ -776,7 +793,7 @@ mod tests { } #[test] - fn it_compares_classes() { + fn it_compares_classes_static() { let a = html! {
}; @@ -795,7 +812,30 @@ mod tests { assert_eq!(a, b); assert_ne!(a, c); - assert_eq!(c, d); + assert_ne!(a, d); + } + + #[test] + fn it_compares_classes_dynamic() { + let a = html! { +
+ }; + + let b = html! { +
+ }; + + let c = html! { +
+ }; + + let d = html! { +
+ }; + + assert_eq!(a, b); + assert_ne!(a, c); + assert_ne!(a, d); } fn assert_vtag(node: &VNode) -> &VTag { @@ -946,26 +986,35 @@ mod tests { document().body().unwrap().append_child(&parent).unwrap(); let mut elem = html! {
}; - elem.apply(&scope, &parent, NodeRef::default(), None); + VDiff::apply(&mut elem, &scope, &parent, NodeRef::default(), None); let vtag = assert_vtag_mut(&mut elem); // test if the className has not been set assert!(!vtag.reference.as_ref().unwrap().has_attribute("class")); } - #[test] - fn it_sets_class_name() { + fn test_set_class_name(gen_html: impl FnOnce() -> Html) { let scope = test_scope(); let parent = document().create_element("div").unwrap(); document().body().unwrap().append_child(&parent).unwrap(); - let mut elem = html! {
}; - elem.apply(&scope, &parent, NodeRef::default(), None); + let mut elem = gen_html(); + VDiff::apply(&mut elem, &scope, &parent, NodeRef::default(), None); let vtag = assert_vtag_mut(&mut elem); // test if the className has been set assert!(vtag.reference.as_ref().unwrap().has_attribute("class")); } + #[test] + fn it_sets_class_name_static() { + test_set_class_name(|| html! {
}); + } + + #[test] + fn it_sets_class_name_dynamic() { + test_set_class_name(|| html! {
}); + } + #[test] fn controlled_input_synced() { let scope = test_scope(); @@ -977,7 +1026,7 @@ mod tests { // Initial state let mut elem = html! { }; - elem.apply(&scope, &parent, NodeRef::default(), None); + VDiff::apply(&mut elem, &scope, &parent, NodeRef::default(), None); let vtag = if let VNode::VTag(vtag) = elem { vtag } else { @@ -1020,7 +1069,7 @@ mod tests { // Initial state let mut elem = html! { }; - elem.apply(&scope, &parent, NodeRef::default(), None); + VDiff::apply(&mut elem, &scope, &parent, NodeRef::default(), None); let vtag = if let VNode::VTag(vtag) = elem { vtag } else { @@ -1067,7 +1116,7 @@ mod tests { builder }/> }; - elem.apply(&scope, &parent, NodeRef::default(), None); + VDiff::apply(&mut elem, &scope, &parent, NodeRef::default(), None); let vtag = assert_vtag_mut(&mut elem); // make sure the new tag name is used internally assert_eq!(vtag.tag(), "a");