Updated to support keyed attributes (#1076)

* Add keys to components

* Switched to HashMap for added performance.  Fixed issue if their isn't a key on the nodes to not process the list.

Issue #479

* Ran cargo fmt.

* Deleted the node since we don't need it.

Switched to using remove instead of get_mut() to make it so we don't
need to do a full scan.

Issue #479

* Changed the key data type to Option<String>.

Issue #479.

* Removed rls config file.

* Fixed macro test to add the additional VChild parameter.

* Updated the code to handle cases where the root has a key.

Nodes with keys are in a hashmap and nodes without are in vector.
Added an error if duplicate keys are in the hashmap and outputs the duplicate key.

Issue #479

* Updated vlist to have key() attribute.

Issue #479

* Updated vlist without a key.

* Added a user defined key to vlist.

* Removed logging statement.

* Made the changes requested.  Still need to add macro tests.

* Updated the macro tests when the syntax changes.

* Ran cargo fmt.

* Switched to returning a reference for the key.

Removed the key comparision for vtag.

* Fixed clippy warning.

* Switch to use == to do the comparision to fix clippy error.

* Add clippy ignore

Co-authored-by: Deep Gaurav <deepgauravraj@gmail.com>
Co-authored-by: Justin Starry <justin.starry@icloud.com>

crates/macro/src/html_tree/html_component.rs

@@ -154,14 +154,20 @@ impl ToTokens for HtmlComponent {
             quote! { ::yew::html::NodeRef::default() }
         };
 
+        let key = if let Some(key) = props.key() {
+            quote_spanned! { key.span() => Some(#key) }
+        } else {
+            quote! {None }
+        };
+
         tokens.extend(quote! {{
             // These validation checks show a nice error message to the user.
             // They do not execute at runtime
             if false {
                 #validate_props
             }
 
-            ::yew::virtual_dom::VChild::<#ty>::new(#init_props, #node_ref)
+            ::yew::virtual_dom::VChild::<#ty>::new(#init_props, #node_ref, #key)
         }});
     }
 }
@@ -343,11 +349,13 @@ enum Props {
 struct ListProps {
     props: Vec<HtmlProp>,
     node_ref: Option<Expr>,
+    key: Option<Expr>,
 }
 
 struct WithProps {
     props: Ident,
     node_ref: Option<Expr>,
+    key: Option<Expr>,
 }
 
 impl Props {
@@ -359,6 +367,14 @@ impl Props {
         }
     }
 
+    fn key(&self) -> Option<&Expr> {
+        match self {
+            Props::List(list_props) => list_props.key.as_ref(),
+            Props::With(with_props) => with_props.key.as_ref(),
+            Props::None => None,
+        }
+    }
+
     fn collision_message() -> &'static str {
         "Using special syntax `with props` along with named prop is not allowed. This rule does not apply to special `ref` prop"
     }
@@ -368,6 +384,7 @@ impl Parse for Props {
     fn parse(input: ParseStream) -> ParseResult<Self> {
         let mut props = Props::None;
         let mut node_ref: Option<Expr> = None;
+        let mut key: Option<Expr> = None;
 
         while let Some((token, _)) = input.cursor().ident() {
             if token == "with" {
@@ -383,6 +400,7 @@ impl Parse for Props {
                 props = Props::With(Box::new(WithProps {
                     props: input.parse::<Ident>()?,
                     node_ref: None,
+                    key: None,
                 }));
 
                 // Handle optional comma
@@ -404,6 +422,15 @@ impl Parse for Props {
                 node_ref = Some(prop.value);
                 continue;
             }
+            if prop.label.to_string() == "key" {
+                match key {
+                    None => Ok(()),
+                    Some(_) => Err(syn::Error::new_spanned(&prop.label, "too many keys set")),
+                }?;
+
+                key = Some(prop.value);
+                continue;
+            }
 
             if prop.label.to_string() == "type" {
                 return Err(syn::Error::new_spanned(&prop.label, "expected identifier"));
@@ -418,6 +445,7 @@ impl Parse for Props {
                     *props = Props::List(Box::new(ListProps {
                         props: vec![prop],
                         node_ref: None,
+                        key: None,
                     }));
                 }
                 Props::With(_) => {
@@ -431,9 +459,13 @@ impl Parse for Props {
 
         match props {
             Props::None => {}
-            Props::With(ref mut p) => p.node_ref = node_ref,
+            Props::With(ref mut p) => {
+                p.node_ref = node_ref;
+                p.key = key
+            }
             Props::List(ref mut p) => {
                 p.node_ref = node_ref;
+                p.key = key;
 
                 // alphabetize
                 p.props.sort_by(|a, b| {

crates/macro/src/html_tree/html_list.rs

@@ -1,12 +1,18 @@
 use super::HtmlTree;
+use crate::html_tree::{HtmlProp, HtmlPropSuffix};
 use crate::PeekValue;
 use boolinator::Boolinator;
-use quote::{quote, ToTokens};
+use quote::{quote, quote_spanned, ToTokens};
 use syn::buffer::Cursor;
+use syn::parse;
 use syn::parse::{Parse, ParseStream, Result as ParseResult};
-use syn::Token;
+use syn::spanned::Spanned;
+use syn::{Expr, Token};
 
-pub struct HtmlList(pub Vec<HtmlTree>);
+pub struct HtmlList {
+    pub children: Vec<HtmlTree>,
+    pub key: Option<Expr>,
+}
 
 impl PeekValue<()> for HtmlList {
     fn peek(cursor: Cursor) -> Option<()> {
@@ -43,20 +49,29 @@ impl Parse for HtmlList {
 
         input.parse::<HtmlListClose>()?;
 
-        Ok(HtmlList(children))
+        Ok(HtmlList {
+            children,
+            key: open.key,
+        })
     }
 }
 
 impl ToTokens for HtmlList {
     fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) {
-        let children = &self.0;
+        let children = &self.children;
+        let key = if let Some(key) = &self.key {
+            quote_spanned! {key.span() => Some(#key)}
+        } else {
+            quote! {None }
+        };
         tokens.extend(quote! {
             ::yew::virtual_dom::VNode::VList(
                 ::yew::virtual_dom::VList::new_with_children({
                     let mut v = ::std::vec::Vec::new();
                     #(v.extend(::yew::utils::NodeSeq::from(#children));)*
                     v
-                })
+                },
+                #key)
             )
         });
     }
@@ -87,31 +102,58 @@ impl HtmlList {
 
 struct HtmlListOpen {
     lt: Token![<],
+    key: Option<Expr>,
     gt: Token![>],
 }
 
 impl PeekValue<()> for HtmlListOpen {
     fn peek(cursor: Cursor) -> Option<()> {
         let (punct, cursor) = cursor.punct()?;
         (punct.as_char() == '<').as_option()?;
-
-        let (punct, _) = cursor.punct()?;
-        (punct.as_char() == '>').as_option()
+        if let Some((ident, _)) = cursor.ident() {
+            (ident == "key").as_option()
+        } else {
+            let (punct, _) = cursor.punct()?;
+            (punct.as_char() == '>').as_option()
+        }
     }
 }
 
 impl Parse for HtmlListOpen {
     fn parse(input: ParseStream) -> ParseResult<Self> {
-        Ok(HtmlListOpen {
-            lt: input.parse()?,
-            gt: input.parse()?,
-        })
+        let lt = input.parse()?;
+        if input.cursor().ident().is_some() {
+            let HtmlPropSuffix { stream, gt, .. } = input.parse()?;
+            let props = parse::<ParseKey>(stream)?;
+            Ok(HtmlListOpen {
+                lt,
+                key: Some(props.key.value),
+                gt,
+            })
+        } else {
+            let gt = input.parse()?;
+            Ok(HtmlListOpen { lt, key: None, gt })
+        }
+    }
+}
+
+struct ParseKey {
+    key: HtmlProp,
+}
+
+impl Parse for ParseKey {
+    fn parse(input: ParseStream) -> ParseResult<Self> {
+        let key = input.parse::<HtmlProp>()?;
+        if !input.is_empty() {
+            input.error("Only a single key element is allowed on a <></>");
+        }
+        Ok(ParseKey { key })
     }
 }
 
 impl ToTokens for HtmlListOpen {
     fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) {
-        let HtmlListOpen { lt, gt } = self;
+        let HtmlListOpen { lt, gt, .. } = self;
         tokens.extend(quote! {#lt#gt});
     }
 }
@@ -126,7 +168,6 @@ impl PeekValue<()> for HtmlListClose {
     fn peek(cursor: Cursor) -> Option<()> {
         let (punct, cursor) = cursor.punct()?;
         (punct.as_char() == '<').as_option()?;
-
         let (punct, cursor) = cursor.punct()?;
         (punct.as_char() == '/').as_option()?;
 

crates/macro/src/html_tree/html_tag/mod.rs

@@ -79,6 +79,7 @@ impl Parse for HtmlTag {
 }
 
 impl ToTokens for HtmlTag {
+    #[allow(clippy::cognitive_complexity)]
     fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) {
         let HtmlTag {
             tag_name,
@@ -96,6 +97,7 @@ impl ToTokens for HtmlTag {
             value,
             checked,
             node_ref,
+            key,
             href,
             listeners,
         } = &attributes;
@@ -141,6 +143,11 @@ impl ToTokens for HtmlTag {
                 #vtag.node_ref = #node_ref;
             }
         });
+        let set_key = key.iter().map(|key| {
+            quote! {
+                #vtag.key = Some(#key);
+            }
+        });
         let listeners = listeners.iter().map(|listener| {
             let name = &listener.label.name;
             let callback = &listener.value;
@@ -163,6 +170,7 @@ impl ToTokens for HtmlTag {
             #(#set_booleans)*
             #(#set_classes)*
             #(#set_node_ref)*
+            #(#set_key)*
             #vtag.add_attributes(vec![#(#attr_pairs),*]);
             #vtag.add_listeners(vec![#(::std::rc::Rc::new(#listeners)),*]);
             #vtag.add_children(vec![#(#children),*]);
@@ -184,10 +192,18 @@ impl PeekValue<TagName> for HtmlTagOpen {
         let (punct, cursor) = cursor.punct()?;
         (punct.as_char() == '<').as_option()?;
 
-        let (name, _) = TagName::peek(cursor)?;
-        non_capitalized_ascii(&name.to_string()).as_option()?;
-
-        Some(name)
+        let (name, cursor) = TagName::peek(cursor)?;
+        if name.to_string() == "key" {
+            let (punct, _) = cursor.punct()?;
+            if punct.as_char() == '=' {
+                None
+            } else {
+                Some(name)
+            }
+        } else {
+            non_capitalized_ascii(&name.to_string()).as_option()?;
+            Some(name)
+        }
     }
 }
 

crates/macro/src/html_tree/html_tag/tag_attributes.rs

@@ -15,6 +15,7 @@ pub struct TagAttributes {
     pub kind: Option<Expr>,
     pub checked: Option<Expr>,
     pub node_ref: Option<Expr>,
+    pub key: Option<Expr>,
     pub href: Option<Expr>,
 }
 
@@ -189,6 +190,8 @@ impl Parse for TagAttributes {
         let kind = TagAttributes::remove_attr(&mut attributes, "type");
         let checked = TagAttributes::remove_attr(&mut attributes, "checked");
         let node_ref = TagAttributes::remove_attr(&mut attributes, "ref");
+        let key = TagAttributes::remove_attr(&mut attributes, "key");
+
         let href = TagAttributes::remove_attr(&mut attributes, "href");
 
         Ok(TagAttributes {
@@ -201,6 +204,7 @@ impl Parse for TagAttributes {
             kind,
             node_ref,
             href,
+            key,
         })
     }
 }

crates/macro/src/html_tree/mod.rs

@@ -115,7 +115,11 @@ impl ToTokens for HtmlTree {
 impl HtmlTree {
     fn token_stream(&self) -> proc_macro2::TokenStream {
         match self {
-            HtmlTree::Empty => HtmlList(Vec::new()).into_token_stream(),
+            HtmlTree::Empty => HtmlList {
+                children: Vec::new(),
+                key: None,
+            }
+            .into_token_stream(),
             HtmlTree::Component(comp) => comp.into_token_stream(),
             HtmlTree::Tag(tag) => tag.into_token_stream(),
             HtmlTree::List(list) => list.into_token_stream(),

crates/macro/tests/macro/html-component-pass.rs

@@ -302,8 +302,8 @@ fn compile_pass() {
 
     let variants = || -> Vec<ChildrenVariants> {
         vec![
-            ChildrenVariants::Child(VChild::new(ChildProperties::default(), NodeRef::default())),
-            ChildrenVariants::AltChild(VChild::new((), NodeRef::default())),
+            ChildrenVariants::Child(VChild::new(ChildProperties::default(), NodeRef::default(), None)),
+            ChildrenVariants::AltChild(VChild::new((), NodeRef::default(), None)),
         ]
     };
 

crates/macro/tests/macro/html-list-fail.rs

@@ -8,6 +8,8 @@ fn compile_fail() {
     html! { <><></> };
     html! { <></><></> };
     html! { <>invalid</> };
+    html! { <key=></>}
+    html! { <key="key".to_string()>invalid</key>}
 }
 
 fn main() {}

crates/macro/tests/macro/html-list-fail.stderr

@@ -39,3 +39,17 @@ error: expected valid html element
    |
 10 |     html! { <>invalid</> };
    |               ^^^^^^^
+
+error: unexpected end of input, expected expression
+  --> $DIR/html-list-fail.rs:11:5
+   |
+11 |     html! { <key=></>}
+   |     ^^^^^^^^^^^^^^^^^^
+   |
+   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
+
+error: this open tag has no corresponding close tag
+  --> $DIR/html-list-fail.rs:12:13
+   |
+12 |     html! { <key="key".to_string()>invalid</key>}
+   |             ^^^^^^^^^^^^^^^^^^^^^^^

crates/macro/tests/macro/html-list-pass.rs

@@ -9,6 +9,10 @@ fn compile_pass() {
             <></>
         </>
     };
+    html! {
+        <key="key".to_string()>
+        </>
+    };
 }
 
 fn main() {}

src/html/mod.rs

@@ -330,7 +330,7 @@ where
     T: Clone + Into<VNode>,
 {
     fn render(&self) -> Html {
-        VList::new_with_children(self.iter().map(|c| c.into()).collect()).into()
+        VList::new_with_children(self.iter().map(|c| c.into()).collect(), None).into()
     }
 }