From aa88b0514c4cec14b264c452b9ada82215b4885d Mon Sep 17 00:00:00 2001 From: Muhammad Hamza Date: Mon, 9 Oct 2023 18:38:34 +0500 Subject: [PATCH 1/6] Allow setting JsValue as properties --- .../yew-macro/src/html_tree/html_element.rs | 44 ++++++--- .../yew/src/dom_bundle/btag/attributes.rs | 97 ++++++++++--------- packages/yew/src/virtual_dom/mod.rs | 56 ++++++++--- packages/yew/src/virtual_dom/vtag.rs | 12 ++- 4 files changed, 132 insertions(+), 77 deletions(-) diff --git a/packages/yew-macro/src/html_tree/html_element.rs b/packages/yew-macro/src/html_tree/html_element.rs index 0124dd0c0d7..66b4da439df 100644 --- a/packages/yew-macro/src/html_tree/html_element.rs +++ b/packages/yew-macro/src/html_tree/html_element.rs @@ -247,27 +247,32 @@ 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::Attribute( + ::yew::virtual_dom::AttrValue::Static(#v) + )), + }; + kv.push(quote! { ( #k, #v) }); } Some(quote! { ::yew::virtual_dom::Attributes::Static(&[#(#kv),*]) }) @@ -280,9 +285,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..6c48a69352c 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,25 +117,25 @@ 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))) + .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))) + .map(|v| (*k, v)) }) .collect(), IndexMap(m) => m .iter() - .map(|(k, (v, apply_as))| (k.as_ref(), (v.as_ref(), *apply_as))) + .map(|(k, v)| (k.as_ref(), v)) .collect(), } } @@ -149,37 +149,36 @@ 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 - .set_attribute(intern(key), value) + fn set(el: &Element, key: &str, value: &AttributeOrProperty) { + match value { + AttributeOrProperty::Attribute(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(_) => 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 +194,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 +247,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 +259,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) => (), } @@ -303,10 +302,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(attrs); let (element, btree) = create_element(); attrs.apply(&btree, &element); assert_eq!( @@ -329,10 +329,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(attrs); let (element, btree) = create_element(); attrs.apply(&btree, &element); assert_eq!( @@ -352,7 +353,7 @@ mod tests { #[test] fn class_is_always_attrs() { - let attrs = Attributes::Static(&[("class", "thing", ApplyAttributeAs::Attribute)]); + let attrs = Attributes::Static(&[("class", AttributeOrProperty::Attribute(AttrValue::Static("thing")))]); let (element, btree) = create_element(); attrs.apply(&btree, &element); @@ -363,10 +364,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 +385,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 940c46021f9..e106e00d2dc 100644 --- a/packages/yew/src/virtual_dom/mod.rs +++ b/packages/yew/src/virtual_dom/mod.rs @@ -24,6 +24,7 @@ pub mod vtext; use std::hint::unreachable_unchecked; use indexmap::IndexMap; +use wasm_bindgen::JsValue; #[doc(inline)] pub use self::key::Key; @@ -173,20 +174,20 @@ mod feat_ssr { /// Defines if the [`Attributes`] is set as element's attribute or property #[allow(missing_docs)] -#[derive(PartialEq, Eq, Copy, Clone, Debug)] -pub enum ApplyAttributeAs { - Attribute, - Property, +#[derive(PartialEq, Clone, Debug)] +pub enum AttributeOrProperty { + 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. @@ -199,12 +200,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(IndexMap), + IndexMap(IndexMap), } impl Attributes { @@ -215,21 +216,36 @@ 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::Static(arr) => Box::new(arr.iter().filter_map(|(k, v)| match v { + AttributeOrProperty::Attribute(v) => Some((*k, v.as_ref())), + AttributeOrProperty::Property(_) => None, + })), Self::Dynamic { keys, values } => Box::new( keys.iter() .zip(values.iter()) - .filter_map(|(k, v)| v.as_ref().map(|(v, _)| (*k, v.as_ref()))), + .filter_map(|(k, v)| { + match v { + Some(AttributeOrProperty::Attribute(v)) => Some((*k, v.as_ref())), + _ => None + } + }), ), - Self::IndexMap(m) => Box::new(m.iter().map(|(k, (v, _))| (k.as_ref(), v.as_ref()))), + 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,7 +261,7 @@ impl Attributes { Self::Static(arr) => { *self = Self::IndexMap( arr.iter() - .map(|(k, v, ty)| ((*k).into(), ((*v).into(), *ty))) + .map(|(k, v)| ((*k).into(), v.clone())) .collect(), ); unpack!() @@ -268,7 +284,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(v) } @@ -278,7 +294,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(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(v) } diff --git a/packages/yew/src/virtual_dom/vtag.rs b/packages/yew/src/virtual_dom/vtag.rs index 266c1e8f339..f26bc811f70 100644 --- a/packages/yew/src/virtual_dom/vtag.rs +++ b/packages/yew/src/virtual_dom/vtag.rs @@ -6,10 +6,11 @@ use std::marker::PhantomData; 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::{AttributeOrProperty, AttrValue, Attributes, Key, Listener, Listeners, VNode}; use crate::html::{IntoPropValue, NodeRef}; /// SVG namespace string used for creating svg elements @@ -373,17 +374,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()), ); } @@ -397,9 +398,10 @@ impl VTag { #[doc(hidden)] pub fn __macro_push_attr(&mut self, key: &'static str, value: impl IntoPropValue) { + // #[cfg(target_arch = "wasm32")] self.attributes.get_mut_index_map().insert( AttrValue::from(key), - (value.into_prop_value(), ApplyAttributeAs::Property), + AttributeOrProperty::Property(JsValue::from_str(&value.into_prop_value())), ); } From 2875be249e9c542baa99f2f4e8faae7fb240f848 Mon Sep 17 00:00:00 2001 From: Muhammad Hamza Date: Mon, 9 Oct 2023 18:55:33 +0500 Subject: [PATCH 2/6] fix tests & CI --- .../yew-macro/src/html_tree/html_element.rs | 11 ++++--- .../yew/src/dom_bundle/btag/attributes.rs | 22 +++++-------- packages/yew/src/virtual_dom/mod.rs | 32 +++++++------------ packages/yew/src/virtual_dom/vtag.rs | 7 ++-- 4 files changed, 29 insertions(+), 43 deletions(-) diff --git a/packages/yew-macro/src/html_tree/html_element.rs b/packages/yew-macro/src/html_tree/html_element.rs index 66b4da439df..d9cecc84b07 100644 --- a/packages/yew-macro/src/html_tree/html_element.rs +++ b/packages/yew-macro/src/html_tree/html_element.rs @@ -247,14 +247,17 @@ impl ToTokens for HtmlElement { } }); - /// 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 + 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() { diff --git a/packages/yew/src/dom_bundle/btag/attributes.rs b/packages/yew/src/dom_bundle/btag/attributes.rs index 6c48a69352c..84ea499c04c 100644 --- a/packages/yew/src/dom_bundle/btag/attributes.rs +++ b/packages/yew/src/dom_bundle/btag/attributes.rs @@ -121,22 +121,13 @@ impl Attributes { use Attributes::*; match src { - Static(arr) => (*arr) - .iter() - .map(|(k, v)| (*k, v)) - .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| (*k, v)) - }) - .collect(), - IndexMap(m) => m - .iter() - .map(|(k, v)| (k.as_ref(), v)) + .filter_map(|(k, v)| v.as_ref().map(|v| (*k, v))) .collect(), + IndexMap(m) => m.iter().map(|(k, v)| (k.as_ref(), v)).collect(), } } @@ -164,7 +155,7 @@ impl Attributes { fn set(el: &Element, key: &str, value: &AttributeOrProperty) { match value { AttributeOrProperty::Attribute(value) => el - .set_attribute(intern(key), &value) + .set_attribute(intern(key), value) .expect("invalid attribute key"), AttributeOrProperty::Property(value) => { let key = JsValue::from_str(key); @@ -353,7 +344,10 @@ mod tests { #[test] fn class_is_always_attrs() { - let attrs = Attributes::Static(&[("class", AttributeOrProperty::Attribute(AttrValue::Static("thing")))]); + let attrs = Attributes::Static(&[( + "class", + AttributeOrProperty::Attribute(AttrValue::Static("thing")), + )]); let (element, btree) = create_element(); attrs.apply(&btree, &element); diff --git a/packages/yew/src/virtual_dom/mod.rs b/packages/yew/src/virtual_dom/mod.rs index e106e00d2dc..a8fe6b78a90 100644 --- a/packages/yew/src/virtual_dom/mod.rs +++ b/packages/yew/src/virtual_dom/mod.rs @@ -172,7 +172,7 @@ 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, Clone, Debug)] pub enum AttributeOrProperty { @@ -224,21 +224,15 @@ impl Attributes { AttributeOrProperty::Attribute(v) => Some((*k, v.as_ref())), AttributeOrProperty::Property(_) => None, })), - 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 - } + 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, })), } } @@ -259,11 +253,7 @@ impl Attributes { match self { Self::IndexMap(m) => m, Self::Static(arr) => { - *self = Self::IndexMap( - arr.iter() - .map(|(k, v)| ((*k).into(), v.clone())) - .collect(), - ); + *self = Self::IndexMap(arr.iter().map(|(k, v)| ((*k).into(), v.clone())).collect()); unpack!() } Self::Dynamic { keys, values } => { diff --git a/packages/yew/src/virtual_dom/vtag.rs b/packages/yew/src/virtual_dom/vtag.rs index f26bc811f70..921d5aaf05f 100644 --- a/packages/yew/src/virtual_dom/vtag.rs +++ b/packages/yew/src/virtual_dom/vtag.rs @@ -6,11 +6,11 @@ use std::marker::PhantomData; use std::mem; use std::ops::{Deref, DerefMut}; use std::rc::Rc; -use wasm_bindgen::JsValue; +use wasm_bindgen::JsValue; use web_sys::{HtmlInputElement as InputElement, HtmlTextAreaElement as TextAreaElement}; -use super::{AttributeOrProperty, AttrValue, Attributes, Key, Listener, Listeners, VNode}; +use super::{AttrValue, AttributeOrProperty, Attributes, Key, Listener, Listeners, VNode}; use crate::html::{IntoPropValue, NodeRef}; /// SVG namespace string used for creating svg elements @@ -398,10 +398,9 @@ impl VTag { #[doc(hidden)] pub fn __macro_push_attr(&mut self, key: &'static str, value: impl IntoPropValue) { - // #[cfg(target_arch = "wasm32")] self.attributes.get_mut_index_map().insert( AttrValue::from(key), - AttributeOrProperty::Property(JsValue::from_str(&value.into_prop_value())), + AttributeOrProperty::Attribute(value.into_prop_value()), ); } From e9e1bb714de91cc84d5b98fabad8d5ed548bbfaf Mon Sep 17 00:00:00 2001 From: Muhammad Hamza Date: Sat, 28 Oct 2023 19:51:56 +0500 Subject: [PATCH 3/6] Rc::new --- packages/yew/src/virtual_dom/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/yew/src/virtual_dom/mod.rs b/packages/yew/src/virtual_dom/mod.rs index c2285c0db59..864a98c5d24 100644 --- a/packages/yew/src/virtual_dom/mod.rs +++ b/packages/yew/src/virtual_dom/mod.rs @@ -287,7 +287,7 @@ impl From> for Attributes { .into_iter() .map(|(k, v)| (AttrValue::Static(k), (AttributeOrProperty::Attribute(v)))) .collect(); - Self::IndexMap(v) + Self::IndexMap(Rc::new(v)) } } From ba95cd0662e55c06c9ac895f0f22c200454254ad Mon Sep 17 00:00:00 2001 From: Muhammad Hamza Date: Sat, 28 Oct 2023 20:14:26 +0500 Subject: [PATCH 4/6] Workaround for Rust <1.72 --- packages/yew-macro/src/html_tree/html_element.rs | 4 ++-- packages/yew/src/dom_bundle/btag/attributes.rs | 5 ++++- packages/yew/src/virtual_dom/mod.rs | 12 +++++++++++- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/packages/yew-macro/src/html_tree/html_element.rs b/packages/yew-macro/src/html_tree/html_element.rs index 5981bf03b06..740d672a92d 100644 --- a/packages/yew-macro/src/html_tree/html_element.rs +++ b/packages/yew-macro/src/html_tree/html_element.rs @@ -271,8 +271,8 @@ impl ToTokens for HtmlElement { ::std::convert::Into::into(#v) )) } - None => quote!(::yew::virtual_dom::AttributeOrProperty::Attribute( - ::yew::virtual_dom::AttrValue::Static(#v) + None => quote!(::yew::virtual_dom::AttributeOrProperty::Static( + #v )), }; kv.push(quote! { ( #k, #v) }); diff --git a/packages/yew/src/dom_bundle/btag/attributes.rs b/packages/yew/src/dom_bundle/btag/attributes.rs index 84ea499c04c..9dee1fc543e 100644 --- a/packages/yew/src/dom_bundle/btag/attributes.rs +++ b/packages/yew/src/dom_bundle/btag/attributes.rs @@ -157,6 +157,9 @@ impl Attributes { 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"), AttributeOrProperty::Property(value) => { let key = JsValue::from_str(key); js_sys::Reflect::set(el.as_ref(), &key, value).expect("could not set property"); @@ -166,7 +169,7 @@ impl Attributes { fn remove(el: &Element, key: &str, old_value: &AttributeOrProperty) { match old_value { - AttributeOrProperty::Attribute(_) => el + AttributeOrProperty::Attribute(_) | AttributeOrProperty::Static(_) => el .remove_attribute(intern(key)) .expect("could not remove attribute"), AttributeOrProperty::Property(_) => { diff --git a/packages/yew/src/virtual_dom/mod.rs b/packages/yew/src/virtual_dom/mod.rs index 864a98c5d24..7b5aac47f5b 100644 --- a/packages/yew/src/virtual_dom/mod.rs +++ b/packages/yew/src/virtual_dom/mod.rs @@ -177,6 +177,13 @@ mod feat_ssr { #[allow(missing_docs)] #[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), } @@ -224,6 +231,7 @@ impl Attributes { 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 { @@ -254,7 +262,9 @@ impl Attributes { match self { Self::IndexMap(m) => Rc::make_mut(m), Self::Static(arr) => { - *self = Self::IndexMap(Rc::new(arr.iter().map(|(k, v)| ((*k).into(), v.clone())).collect())); + *self = Self::IndexMap(Rc::new( + arr.iter().map(|(k, v)| ((*k).into(), v.clone())).collect(), + )); unpack!() } Self::Dynamic { keys, values } => { From 2a675df7e2cac6fd4132b51839b1af7496723a30 Mon Sep 17 00:00:00 2001 From: Muhammad Hamza Date: Sat, 28 Oct 2023 20:18:38 +0500 Subject: [PATCH 5/6] more Rc::new --- packages/yew/src/dom_bundle/btag/attributes.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/yew/src/dom_bundle/btag/attributes.rs b/packages/yew/src/dom_bundle/btag/attributes.rs index 9dee1fc543e..94fd078c1fa 100644 --- a/packages/yew/src/dom_bundle/btag/attributes.rs +++ b/packages/yew/src/dom_bundle/btag/attributes.rs @@ -276,6 +276,7 @@ impl Apply for Attributes { #[cfg(test)] mod tests { use std::time::Duration; + use std::rc::Rc; use gloo::utils::document; use js_sys::Reflect; @@ -300,7 +301,7 @@ mod tests { AttrValue::Static("href") => AttributeOrProperty::Property(JsValue::from_str("https://example.com/")), AttrValue::Static("alt") => AttributeOrProperty::Property(JsValue::from_str("somewhere")), }; - let attrs = Attributes::IndexMap(attrs); + let attrs = Attributes::IndexMap(Rc::new(attrs)); let (element, btree) = create_element(); attrs.apply(&btree, &element); assert_eq!( @@ -327,7 +328,7 @@ mod tests { AttrValue::Static("href") => AttributeOrProperty::Attribute(AttrValue::from("https://example.com/")), AttrValue::Static("alt") => AttributeOrProperty::Property(JsValue::from_str("somewhere")), }; - let attrs = Attributes::IndexMap(attrs); + let attrs = Attributes::IndexMap(Rc::new(attrs)); let (element, btree) = create_element(); attrs.apply(&btree, &element); assert_eq!( From 1cdc5b981f3b828984680bc18a36270ca8a349b6 Mon Sep 17 00:00:00 2001 From: Muhammad Hamza Date: Sat, 28 Oct 2023 20:29:17 +0500 Subject: [PATCH 6/6] ci green? --- packages/yew/src/dom_bundle/btag/attributes.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/yew/src/dom_bundle/btag/attributes.rs b/packages/yew/src/dom_bundle/btag/attributes.rs index 94fd078c1fa..d56be61021d 100644 --- a/packages/yew/src/dom_bundle/btag/attributes.rs +++ b/packages/yew/src/dom_bundle/btag/attributes.rs @@ -275,8 +275,8 @@ impl Apply for Attributes { #[cfg(target_arch = "wasm32")] #[cfg(test)] mod tests { - use std::time::Duration; use std::rc::Rc; + use std::time::Duration; use gloo::utils::document; use js_sys::Reflect; @@ -348,10 +348,7 @@ mod tests { #[test] fn class_is_always_attrs() { - let attrs = Attributes::Static(&[( - "class", - AttributeOrProperty::Attribute(AttrValue::Static("thing")), - )]); + let attrs = Attributes::Static(&[("class", AttributeOrProperty::Static("thing"))]); let (element, btree) = create_element(); attrs.apply(&btree, &element);