Browse Source

Feat: reject invalid keys

Jonathan Kelley 1 year ago
parent
commit
ae352f8958

+ 1 - 1
packages/core/tests/kitchen_sink.rs

@@ -6,7 +6,7 @@ fn basic_syntax_is_a_template() -> Element {
     let var = 123;
 
     rsx! {
-        div { key: "12345", class: "asd", class: "{asd}", class: if true {
+        div { key: "{asd}", class: "asd", class: "{asd}", class: if true {
                 "{asd}"
             }, class: if false {
                 "{asd}"

+ 1 - 0
packages/rsx-rosetta/src/lib.rs

@@ -141,6 +141,7 @@ pub fn collect_svgs(children: &mut [BodyNode], out: &mut Vec<BodyNode>) {
                     fields: vec![],
                     children: vec![],
                     manual_props: None,
+                    key: None,
                     brace: Default::default(),
                 });
 

+ 19 - 19
packages/rsx/src/component.rs

@@ -27,6 +27,7 @@ use syn::{
 pub struct Component {
     pub name: syn::Path,
     pub prop_gen_args: Option<AngleBracketedGenericArguments>,
+    pub key: Option<IfmtInput>,
     pub fields: Vec<ComponentField>,
     pub children: Vec<BodyNode>,
     pub manual_props: Option<Expr>,
@@ -47,6 +48,7 @@ impl Parse for Component {
         let mut fields = Vec::new();
         let mut children = Vec::new();
         let mut manual_props = None;
+        let mut key = None;
 
         while !content.is_empty() {
             // if we splat into a component then we're merging properties
@@ -56,14 +58,26 @@ impl Parse for Component {
             } else if
             // Named fields
             (content.peek(Ident) && content.peek2(Token![:]) && !content.peek3(Token![:]))
-            // shorthand struct initialization
+                // shorthand struct initialization
                 // Not a div {}, mod::Component {}, or web-component {}
                 || (content.peek(Ident)
                     && !content.peek2(Brace)
                     && !content.peek2(Token![:])
                     && !content.peek2(Token![-]))
             {
-                fields.push(content.parse()?);
+                // If it
+                if content.fork().parse::<Ident>()? == "key" {
+                    _ = content.parse::<Ident>()?;
+                    _ = content.parse::<Token![:]>()?;
+
+                    let _key: IfmtInput = content.parse()?;
+                    if _key.is_static() {
+                        invalid_key!(_key);
+                    }
+                    key = Some(_key);
+                } else {
+                    fields.push(content.parse()?);
+                }
             } else {
                 children.push(content.parse()?);
             }
@@ -80,6 +94,7 @@ impl Parse for Component {
             children,
             manual_props,
             brace,
+            key,
         })
     }
 }
@@ -137,15 +152,7 @@ impl Component {
     }
 
     pub fn key(&self) -> Option<&IfmtInput> {
-        match self
-            .fields
-            .iter()
-            .find(|f| f.name == "key")
-            .map(|f| &f.content)
-        {
-            Some(ContentField::Formatted(fmt)) => Some(fmt),
-            _ => None,
-        }
+        self.key.as_ref()
     }
 
     fn collect_manual_props(&self, manual_props: &Expr) -> TokenStream2 {
@@ -169,10 +176,7 @@ impl Component {
             None => quote! { fc_to_builder(#name) },
         };
         for field in &self.fields {
-            match field.name.to_string().as_str() {
-                "key" => {}
-                _ => toks.append_all(quote! {#field}),
-            }
+            toks.append_all(quote! {#field})
         }
         if !self.children.is_empty() {
             let renderer: TemplateRenderer = TemplateRenderer {
@@ -218,10 +222,6 @@ impl ContentField {
             return Ok(ContentField::OnHandlerRaw(input.parse()?));
         }
 
-        if *name == "key" {
-            return Ok(ContentField::Formatted(input.parse()?));
-        }
-
         if input.peek(LitStr) {
             let forked = input.fork();
             let t: LitStr = forked.parse()?;

+ 7 - 1
packages/rsx/src/element.rs

@@ -169,7 +169,13 @@ impl Parse for Element {
                         },
                     }));
                 } else if name_str == "key" {
-                    key = Some(content.parse()?);
+                    let _key: IfmtInput = content.parse()?;
+
+                    if _key.is_static() {
+                        invalid_key!(_key);
+                    }
+
+                    key = Some(_key);
                 } else {
                     let value = content.parse::<ElementAttrValue>()?;
                     attributes.push(attribute::AttributeType::Named(ElementAttrNamed {

+ 10 - 0
packages/rsx/src/errors.rs

@@ -24,3 +24,13 @@ macro_rules! invalid_component_path {
         return Err(Error::new($span, "Invalid component path syntax"));
     };
 }
+
+macro_rules! invalid_key {
+    ($_key:ident) => {
+        let val = $_key.to_static().unwrap();
+        return Err(syn::Error::new(
+            $_key.span(),
+            format!("Element keys must be a dynamic value. Considering using `key: {{{val}}}` instead.\nStatic keys will result in every element using the same key which will cause rendering issues or panics."),
+        ));
+    };
+}

+ 2 - 2
packages/rsx/src/lib.rs

@@ -179,8 +179,8 @@ impl<'a> ToTokens for TemplateRenderer<'a> {
         let mut context = DynamicContext::default();
 
         let key = match self.roots.first() {
-            Some(BodyNode::Element(el)) if self.roots.len() == 1 => el.key.clone(),
-            Some(BodyNode::Component(comp)) if self.roots.len() == 1 => comp.key().cloned(),
+            Some(BodyNode::Element(el)) if self.roots.len() >= 1 => el.key.clone(),
+            Some(BodyNode::Component(comp)) if self.roots.len() >= 1 => comp.key().cloned(),
             _ => None,
         };