diff --git a/packages/yew-macro/src/html_tree/html_element.rs b/packages/yew-macro/src/html_tree/html_element.rs index a501644941b..740d672a92d 100644 --- a/packages/yew-macro/src/html_tree/html_element.rs +++ b/packages/yew-macro/src/html_tree/html_element.rs @@ -247,27 +247,35 @@ impl ToTokens for HtmlElement { } }); - fn apply_as(directive: Option<&PropDirective>) -> TokenStream { - match directive { - Some(PropDirective::ApplyAsProperty(token)) => { - quote_spanned!(token.span()=> ::yew::virtual_dom::ApplyAttributeAs::Property) - } - None => quote!(::yew::virtual_dom::ApplyAttributeAs::Attribute), - } - } - /// Try to turn attribute list into a `::yew::virtual_dom::Attributes::Static` fn try_into_static( src: &[(LitStr, Value, Option)], ) -> Option { + if src + .iter() + .any(|(_, _, d)| matches!(d, Some(PropDirective::ApplyAsProperty(_)))) + { + // don't try to make a static attribute list if there are any properties to + // assign + return None; + } let mut kv = Vec::with_capacity(src.len()); for (k, v, directive) in src.iter() { let v = match v { Value::Static(v) => quote! { #v }, Value::Dynamic(_) => return None, }; - let apply_as = apply_as(directive.as_ref()); - kv.push(quote! { ( #k, #v, #apply_as ) }); + let v = match directive { + Some(PropDirective::ApplyAsProperty(token)) => { + quote_spanned!(token.span()=> ::yew::virtual_dom::AttributeOrProperty::Property( + ::std::convert::Into::into(#v) + )) + } + None => quote!(::yew::virtual_dom::AttributeOrProperty::Static( + #v + )), + }; + kv.push(quote! { ( #k, #v) }); } Some(quote! { ::yew::virtual_dom::Attributes::Static(&[#(#kv),*]) }) @@ -280,9 +288,22 @@ impl ToTokens for HtmlElement { try_into_static(&attrs).unwrap_or_else(|| { let keys = attrs.iter().map(|(k, ..)| quote! { #k }); let values = attrs.iter().map(|(_, v, directive)| { - let apply_as = apply_as(directive.as_ref()); - let value = wrap_attr_value(v); - quote! { ::std::option::Option::map(#value, |it| (it, #apply_as)) } + let value = match directive { + Some(PropDirective::ApplyAsProperty(token)) => { + quote_spanned!(token.span()=> ::std::option::Option::Some( + ::yew::virtual_dom::AttributeOrProperty::Property( + ::std::convert::Into::into(#v) + )) + ) + } + None => { + let value = wrap_attr_value(v); + quote! { + ::std::option::Option::map(#value, ::yew::virtual_dom::AttributeOrProperty::Attribute) + } + }, + }; + quote! { #value } }); quote! { ::yew::virtual_dom::Attributes::Dynamic{ diff --git a/packages/yew/src/dom_bundle/btag/attributes.rs b/packages/yew/src/dom_bundle/btag/attributes.rs index e2f41e11498..d56be61021d 100644 --- a/packages/yew/src/dom_bundle/btag/attributes.rs +++ b/packages/yew/src/dom_bundle/btag/attributes.rs @@ -9,7 +9,7 @@ use yew::AttrValue; use super::Apply; use crate::dom_bundle::BSubtree; use crate::virtual_dom::vtag::{InputFields, Value}; -use crate::virtual_dom::{ApplyAttributeAs, Attributes}; +use crate::virtual_dom::{AttributeOrProperty, Attributes}; impl Apply for Value { type Bundle = Self; @@ -92,23 +92,23 @@ impl Attributes { #[cold] fn apply_diff_index_maps( el: &Element, - new: &IndexMap, - old: &IndexMap, + new: &IndexMap, + old: &IndexMap, ) { for (key, value) in new.iter() { match old.get(key) { Some(old_value) => { if value != old_value { - Self::set(el, key, value.0.as_ref(), value.1); + Self::set(el, key, value); } } - None => Self::set(el, key, value.0.as_ref(), value.1), + None => Self::set(el, key, value), } } - for (key, (_, apply_as)) in old.iter() { + for (key, value) in old.iter() { if !new.contains_key(key) { - Self::remove(el, key, *apply_as); + Self::remove(el, key, value); } } } @@ -117,26 +117,17 @@ impl Attributes { /// Works with any [Attributes] variants. #[cold] fn apply_diff_as_maps<'a>(el: &Element, new: &'a Self, old: &'a Self) { - fn collect(src: &Attributes) -> HashMap<&str, (&str, ApplyAttributeAs)> { + fn collect(src: &Attributes) -> HashMap<&str, &AttributeOrProperty> { use Attributes::*; match src { - Static(arr) => (*arr) - .iter() - .map(|(k, v, apply_as)| (*k, (*v, *apply_as))) - .collect(), + Static(arr) => (*arr).iter().map(|(k, v)| (*k, v)).collect(), Dynamic { keys, values } => keys .iter() .zip(values.iter()) - .filter_map(|(k, v)| { - v.as_ref() - .map(|(v, apply_as)| (*k, (v.as_ref(), *apply_as))) - }) - .collect(), - IndexMap(m) => m - .iter() - .map(|(k, (v, apply_as))| (k.as_ref(), (v.as_ref(), *apply_as))) + .filter_map(|(k, v)| v.as_ref().map(|v| (*k, v))) .collect(), + IndexMap(m) => m.iter().map(|(k, v)| (k.as_ref(), v)).collect(), } } @@ -149,37 +140,39 @@ impl Attributes { Some(old) => old != new, None => true, } { - Self::set(el, k, new.0, new.1); + Self::set(el, k, new); } } // Remove missing - for (k, (_, apply_as)) in old.iter() { + for (k, old_value) in old.iter() { if !new.contains_key(k) { - Self::remove(el, k, *apply_as); + Self::remove(el, k, old_value); } } } - fn set(el: &Element, key: &str, value: &str, apply_as: ApplyAttributeAs) { - match apply_as { - ApplyAttributeAs::Attribute => el + fn set(el: &Element, key: &str, value: &AttributeOrProperty) { + match value { + AttributeOrProperty::Attribute(value) => el + .set_attribute(intern(key), value) + .expect("invalid attribute key"), + AttributeOrProperty::Static(value) => el .set_attribute(intern(key), value) .expect("invalid attribute key"), - ApplyAttributeAs::Property => { + AttributeOrProperty::Property(value) => { let key = JsValue::from_str(key); - let value = JsValue::from_str(value); - js_sys::Reflect::set(el.as_ref(), &key, &value).expect("could not set property"); + js_sys::Reflect::set(el.as_ref(), &key, value).expect("could not set property"); } } } - fn remove(el: &Element, key: &str, apply_as: ApplyAttributeAs) { - match apply_as { - ApplyAttributeAs::Attribute => el + fn remove(el: &Element, key: &str, old_value: &AttributeOrProperty) { + match old_value { + AttributeOrProperty::Attribute(_) | AttributeOrProperty::Static(_) => el .remove_attribute(intern(key)) .expect("could not remove attribute"), - ApplyAttributeAs::Property => { + AttributeOrProperty::Property(_) => { let key = JsValue::from_str(key); js_sys::Reflect::set(el.as_ref(), &key, &JsValue::UNDEFINED) .expect("could not remove property"); @@ -195,20 +188,20 @@ impl Apply for Attributes { fn apply(self, _root: &BSubtree, el: &Element) -> Self { match &self { Self::Static(arr) => { - for (k, v, apply_as) in arr.iter() { - Self::set(el, k, v, *apply_as); + for (k, v) in arr.iter() { + Self::set(el, k, v); } } Self::Dynamic { keys, values } => { for (k, v) in keys.iter().zip(values.iter()) { - if let Some((v, apply_as)) = v { - Self::set(el, k, v, *apply_as) + if let Some(v) = v { + Self::set(el, k, v) } } } Self::IndexMap(m) => { - for (k, (v, apply_as)) in m.iter() { - Self::set(el, k, v, *apply_as) + for (k, v) in m.iter() { + Self::set(el, k, v) } } } @@ -248,7 +241,7 @@ impl Apply for Attributes { } macro_rules! set { ($new:expr) => { - Self::set(el, key!(), $new.0.as_ref(), $new.1) + Self::set(el, key!(), $new) }; } @@ -260,7 +253,7 @@ impl Apply for Attributes { } (Some(new), None) => set!(new), (None, Some(old)) => { - Self::remove(el, key!(), old.1); + Self::remove(el, key!(), old); } (None, None) => (), } @@ -282,6 +275,7 @@ impl Apply for Attributes { #[cfg(target_arch = "wasm32")] #[cfg(test)] mod tests { + use std::rc::Rc; use std::time::Duration; use gloo::utils::document; @@ -303,10 +297,11 @@ mod tests { #[test] fn properties_are_set() { - let attrs = Attributes::Static(&[ - ("href", "https://example.com/", ApplyAttributeAs::Property), - ("alt", "somewhere", ApplyAttributeAs::Property), - ]); + let attrs = indexmap::indexmap! { + AttrValue::Static("href") => AttributeOrProperty::Property(JsValue::from_str("https://example.com/")), + AttrValue::Static("alt") => AttributeOrProperty::Property(JsValue::from_str("somewhere")), + }; + let attrs = Attributes::IndexMap(Rc::new(attrs)); let (element, btree) = create_element(); attrs.apply(&btree, &element); assert_eq!( @@ -329,10 +324,11 @@ mod tests { #[test] fn respects_apply_as() { - let attrs = Attributes::Static(&[ - ("href", "https://example.com/", ApplyAttributeAs::Attribute), - ("alt", "somewhere", ApplyAttributeAs::Property), - ]); + let attrs = indexmap::indexmap! { + AttrValue::Static("href") => AttributeOrProperty::Attribute(AttrValue::from("https://example.com/")), + AttrValue::Static("alt") => AttributeOrProperty::Property(JsValue::from_str("somewhere")), + }; + let attrs = Attributes::IndexMap(Rc::new(attrs)); let (element, btree) = create_element(); attrs.apply(&btree, &element); assert_eq!( @@ -352,7 +348,7 @@ mod tests { #[test] fn class_is_always_attrs() { - let attrs = Attributes::Static(&[("class", "thing", ApplyAttributeAs::Attribute)]); + let attrs = Attributes::Static(&[("class", AttributeOrProperty::Static("thing"))]); let (element, btree) = create_element(); attrs.apply(&btree, &element); @@ -363,10 +359,10 @@ mod tests { async fn macro_syntax_works() { #[function_component] fn Comp() -> Html { - html! { } + html! { } } - let output = gloo::utils::document().get_element_by_id("output").unwrap(); + let output = document().get_element_by_id("output").unwrap(); yew::Renderer::::with_root(output.clone()).render(); gloo::timers::future::sleep(Duration::from_secs(1)).await; @@ -384,5 +380,13 @@ mod tests { "abc", "property `alt` not set properly" ); + + assert!( + Reflect::get(element.as_ref(), &JsValue::from_str("data-bool")) + .expect("no alt") + .as_bool() + .expect("not a bool"), + "property `alt` not set properly" + ); } } diff --git a/packages/yew/src/virtual_dom/mod.rs b/packages/yew/src/virtual_dom/mod.rs index 6ede1e6e074..7b5aac47f5b 100644 --- a/packages/yew/src/virtual_dom/mod.rs +++ b/packages/yew/src/virtual_dom/mod.rs @@ -25,6 +25,7 @@ use std::hint::unreachable_unchecked; use std::rc::Rc; use indexmap::IndexMap; +use wasm_bindgen::JsValue; #[doc(inline)] pub use self::key::Key; @@ -172,22 +173,29 @@ mod feat_ssr { } } -/// Defines if the [`Attributes`] is set as element's attribute or property +/// Defines if the [`Attributes`] is set as element's attribute or property and its value. #[allow(missing_docs)] -#[derive(PartialEq, Eq, Copy, Clone, Debug)] -pub enum ApplyAttributeAs { - Attribute, - Property, +#[derive(PartialEq, Clone, Debug)] +pub enum AttributeOrProperty { + // This exists as a workaround to support Rust <1.72 + // Previous versions of Rust did not See + // `AttributeOrProperty::Attribute(AttrValue::Static(_))` as `'static` that html! macro + // used, and thus failed with "temporary value dropped while borrowed" + // + // See: https://github.com/yewstack/yew/pull/3458#discussion_r1350362215 + Static(&'static str), + Attribute(AttrValue), + Property(JsValue), } /// A collection of attributes for an element -#[derive(PartialEq, Eq, Clone, Debug)] +#[derive(PartialEq, Clone, Debug)] pub enum Attributes { /// 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, &'static str, ApplyAttributeAs)]), + Static(&'static [(&'static str, AttributeOrProperty)]), /// Static list of attribute keys with possibility to exclude attributes and dynamic attribute /// values. @@ -200,12 +208,12 @@ pub enum Attributes { /// Attribute values. Matches [keys](Attributes::Dynamic::keys). Optional attributes are /// designated by setting [None]. - values: Box<[Option<(AttrValue, ApplyAttributeAs)>]>, + values: Box<[Option]>, }, /// IndexMap is used to provide runtime attribute deduplication in cases where the html! macro /// was not used to guarantee it. - IndexMap(Rc>), + IndexMap(Rc>), } impl Attributes { @@ -216,21 +224,31 @@ impl Attributes { /// Return iterator over attribute key-value pairs. /// This function is suboptimal and does not inline well. Avoid on hot paths. + /// + /// This function only returns attributes pub fn iter<'a>(&'a self) -> Box + 'a> { match self { - Self::Static(arr) => Box::new(arr.iter().map(|(k, v, _)| (*k, *v 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.as_ref(), v.as_ref()))), + Self::Static(arr) => Box::new(arr.iter().filter_map(|(k, v)| match v { + AttributeOrProperty::Attribute(v) => Some((*k, v.as_ref())), + AttributeOrProperty::Property(_) => None, + AttributeOrProperty::Static(v) => Some((*k, v)), + })), + Self::Dynamic { keys, values } => { + Box::new(keys.iter().zip(values.iter()).filter_map(|(k, v)| match v { + Some(AttributeOrProperty::Attribute(v)) => Some((*k, v.as_ref())), + _ => None, + })) + } + Self::IndexMap(m) => Box::new(m.iter().filter_map(|(k, v)| match v { + AttributeOrProperty::Attribute(v) => Some((k.as_ref(), v.as_ref())), + _ => None, + })), } } /// 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 { + pub fn get_mut_index_map(&mut self) -> &mut IndexMap { macro_rules! unpack { () => { match self { @@ -245,9 +263,7 @@ impl Attributes { Self::IndexMap(m) => Rc::make_mut(m), Self::Static(arr) => { *self = Self::IndexMap(Rc::new( - arr.iter() - .map(|(k, v, ty)| ((*k).into(), ((*v).into(), *ty))) - .collect(), + arr.iter().map(|(k, v)| ((*k).into(), v.clone())).collect(), )); unpack!() } @@ -269,7 +285,7 @@ impl From> for Attributes { fn from(map: IndexMap) -> Self { let v = map .into_iter() - .map(|(k, v)| (k, (v, ApplyAttributeAs::Attribute))) + .map(|(k, v)| (k, AttributeOrProperty::Attribute(v))) .collect(); Self::IndexMap(Rc::new(v)) } @@ -279,7 +295,17 @@ impl From> for Attributes { fn from(v: IndexMap<&'static str, AttrValue>) -> Self { let v = v .into_iter() - .map(|(k, v)| (AttrValue::Static(k), (v, ApplyAttributeAs::Attribute))) + .map(|(k, v)| (AttrValue::Static(k), (AttributeOrProperty::Attribute(v)))) + .collect(); + Self::IndexMap(Rc::new(v)) + } +} + +impl From> for Attributes { + fn from(v: IndexMap<&'static str, JsValue>) -> Self { + let v = v + .into_iter() + .map(|(k, v)| (AttrValue::Static(k), (AttributeOrProperty::Property(v)))) .collect(); Self::IndexMap(Rc::new(v)) } diff --git a/packages/yew/src/virtual_dom/vtag.rs b/packages/yew/src/virtual_dom/vtag.rs index 60f70c53c14..0b0d9699049 100644 --- a/packages/yew/src/virtual_dom/vtag.rs +++ b/packages/yew/src/virtual_dom/vtag.rs @@ -6,9 +6,10 @@ use std::mem; use std::ops::{Deref, DerefMut}; use std::rc::Rc; +use wasm_bindgen::JsValue; use web_sys::{HtmlInputElement as InputElement, HtmlTextAreaElement as TextAreaElement}; -use super::{ApplyAttributeAs, AttrValue, Attributes, Key, Listener, Listeners, VNode}; +use super::{AttrValue, AttributeOrProperty, Attributes, Key, Listener, Listeners, VNode}; use crate::html::{ImplicitClone, IntoPropValue, NodeRef}; /// SVG namespace string used for creating svg elements @@ -386,17 +387,17 @@ impl VTag { pub fn add_attribute(&mut self, key: &'static str, value: impl Into) { self.attributes.get_mut_index_map().insert( AttrValue::Static(key), - (value.into(), ApplyAttributeAs::Attribute), + AttributeOrProperty::Attribute(value.into()), ); } /// Set the given key as property on the element /// /// [`js_sys::Reflect`] is used for setting properties. - pub fn add_property(&mut self, key: &'static str, value: impl Into) { + pub fn add_property(&mut self, key: &'static str, value: impl Into) { self.attributes.get_mut_index_map().insert( AttrValue::Static(key), - (value.into(), ApplyAttributeAs::Property), + AttributeOrProperty::Property(value.into()), ); } @@ -412,7 +413,7 @@ impl VTag { pub fn __macro_push_attr(&mut self, key: &'static str, value: impl IntoPropValue) { self.attributes.get_mut_index_map().insert( AttrValue::from(key), - (value.into_prop_value(), ApplyAttributeAs::Property), + AttributeOrProperty::Attribute(value.into_prop_value()), ); }