From f3dcdc057d1c4856e2f293336373f36549c1b230 Mon Sep 17 00:00:00 2001 From: Greg Johnston Date: Mon, 16 Sep 2024 20:26:51 -0400 Subject: [PATCH 1/3] fix: sort attributes so `class` and `style` always run before `class:` and `style:` (closes #2982) --- leptos_macro/src/lib.rs | 4 +- leptos_macro/src/view/component_builder.rs | 32 +++++++----- leptos_macro/src/view/mod.rs | 59 +++++++++++++++++----- leptos_macro/src/view/slot_helper.rs | 34 ++++++++----- 4 files changed, 86 insertions(+), 43 deletions(-) diff --git a/leptos_macro/src/lib.rs b/leptos_macro/src/lib.rs index a3f31f38a..15d755658 100644 --- a/leptos_macro/src/lib.rs +++ b/leptos_macro/src/lib.rs @@ -302,10 +302,10 @@ pub fn view(tokens: TokenStream) -> TokenStream { }; let config = rstml::ParserConfig::default().recover_block(true); let parser = rstml::Parser::new(config); - let (nodes, errors) = parser.parse_recoverable(tokens).split_vec(); + let (mut nodes, errors) = parser.parse_recoverable(tokens).split_vec(); let errors = errors.into_iter().map(|e| e.emit_as_expr_tokens()); let nodes_output = view::render_view( - &nodes, + &mut nodes, global_class.as_ref(), normalized_call_site(proc_macro::Span::call_site()), ); diff --git a/leptos_macro/src/view/component_builder.rs b/leptos_macro/src/view/component_builder.rs index 57236221f..9ee0c8ddc 100644 --- a/leptos_macro/src/view/component_builder.rs +++ b/leptos_macro/src/view/component_builder.rs @@ -10,11 +10,9 @@ use std::collections::HashMap; use syn::{spanned::Spanned, Expr, ExprPath, ExprRange, RangeLimits, Stmt}; pub(crate) fn component_to_tokens( - node: &NodeElement, + node: &mut NodeElement, global_class: Option<&TokenTree>, ) -> TokenStream { - let name = node.name(); - #[allow(unused)] // TODO this is used by hot-reloading #[cfg(debug_assertions)] let component_name = super::ident_from_tag_name(node.name()); @@ -45,16 +43,21 @@ pub(crate) fn component_to_tokens( }) .unwrap_or_else(|| node.attributes().len()); - let attrs = node.attributes().iter().filter_map(|node| { - if let NodeAttribute::Attribute(node) = node { - Some(node) - } else { - None - } - }); + let attrs = node + .attributes() + .iter() + .filter_map(|node| { + if let NodeAttribute::Attribute(node) = node { + Some(node) + } else { + None + } + }) + .cloned() + .collect::>(); let props = attrs - .clone() + .iter() .enumerate() .filter(|(idx, attr)| { idx < &spread_marker && { @@ -85,7 +88,7 @@ pub(crate) fn component_to_tokens( }); let items_to_bind = attrs - .clone() + .iter() .filter_map(|attr| { if !is_attr_let(&attr.key) { return None; @@ -107,7 +110,7 @@ pub(crate) fn component_to_tokens( .collect::>(); let items_to_clone = attrs - .clone() + .iter() .filter_map(|attr| { attr.key .to_string() @@ -183,7 +186,7 @@ pub(crate) fn component_to_tokens( quote! {} } else { let children = fragment_to_tokens( - &node.children, + &mut node.children, TagType::Unknown, Some(&mut slots), global_class, @@ -261,6 +264,7 @@ pub(crate) fn component_to_tokens( quote! {} }; + let name = node.name(); #[allow(unused_mut)] // used in debug let mut component = quote! { { diff --git a/leptos_macro/src/view/mod.rs b/leptos_macro/src/view/mod.rs index 8c68acaff..11902abb0 100644 --- a/leptos_macro/src/view/mod.rs +++ b/leptos_macro/src/view/mod.rs @@ -13,7 +13,10 @@ use rstml::node::{ CustomNode, KVAttributeValue, KeyedAttribute, Node, NodeAttribute, NodeBlock, NodeElement, NodeName, NodeNameFragment, }; -use std::collections::{HashMap, HashSet}; +use std::{ + cmp::Ordering, + collections::{HashMap, HashSet}, +}; use syn::{ spanned::Spanned, Expr, Expr::Tuple, ExprLit, ExprRange, Lit, LitStr, RangeLimits, Stmt, @@ -28,7 +31,7 @@ pub(crate) enum TagType { } pub fn render_view( - nodes: &[Node], + nodes: &mut [Node], global_class: Option<&TokenTree>, view_marker: Option, ) -> Option { @@ -44,7 +47,7 @@ pub fn render_view( } 1 => ( node_to_tokens( - &nodes[0], + &mut nodes[0], TagType::Unknown, None, global_class, @@ -89,7 +92,7 @@ pub fn render_view( } fn element_children_to_tokens( - nodes: &[Node], + nodes: &mut [Node], parent_type: TagType, parent_slots: Option<&mut HashMap>>, global_class: Option<&TokenTree>, @@ -137,7 +140,7 @@ fn element_children_to_tokens( } fn fragment_to_tokens( - nodes: &[Node], + nodes: &mut [Node], parent_type: TagType, parent_slots: Option<&mut HashMap>>, global_class: Option<&TokenTree>, @@ -175,7 +178,7 @@ fn fragment_to_tokens( } fn children_to_tokens( - nodes: &[Node], + nodes: &mut [Node], parent_type: TagType, parent_slots: Option<&mut HashMap>>, global_class: Option<&TokenTree>, @@ -183,7 +186,7 @@ fn children_to_tokens( ) -> Vec { if nodes.len() == 1 { match node_to_tokens( - &nodes[0], + &mut nodes[0], parent_type, parent_slots, global_class, @@ -195,7 +198,7 @@ fn children_to_tokens( } else { let mut slots = HashMap::new(); let nodes = nodes - .iter() + .iter_mut() .filter_map(|node| { node_to_tokens( node, @@ -219,7 +222,7 @@ fn children_to_tokens( } fn node_to_tokens( - node: &Node, + node: &mut Node, parent_type: TagType, parent_slots: Option<&mut HashMap>>, global_class: Option<&TokenTree>, @@ -232,7 +235,7 @@ fn node_to_tokens( Some(quote! { ::leptos::tachys::html::doctype(#value) }) } Node::Fragment(fragment) => fragment_to_tokens( - &fragment.children, + &mut fragment.children, parent_type, parent_slots, global_class, @@ -270,12 +273,41 @@ fn text_to_tokens(text: &LitStr) -> TokenStream { } pub(crate) fn element_to_tokens( - node: &NodeElement, + node: &mut NodeElement, mut parent_type: TagType, parent_slots: Option<&mut HashMap>>, global_class: Option<&TokenTree>, view_marker: Option<&str>, ) -> Option { + // the `class` and `style` attributes overwrite individual `class:` and `style:` attributes + // when they are set. as a result, we're going to sort the attributes so that `class` and + // `style` always come before all other attributes. + node.attributes_mut().sort_by(|a, b| { + let key_a = match a { + NodeAttribute::Attribute(attr) => match &attr.key { + NodeName::Path(attr) => { + attr.path.segments.first().map(|n| n.ident.to_string()) + } + _ => None, + }, + _ => None, + }; + let key_b = match b { + NodeAttribute::Attribute(attr) => match &attr.key { + NodeName::Path(attr) => { + attr.path.segments.first().map(|n| n.ident.to_string()) + } + _ => None, + }, + _ => None, + }; + match (key_a.as_deref(), key_b.as_deref()) { + (Some("class"), _) | (Some("style"), _) => Ordering::Less, + (_, Some("class")) | (_, Some("style")) => Ordering::Greater, + _ => Ordering::Equal, + } + }); + // check for duplicate attribute names and emit an error for all subsequent ones let mut names = HashSet::new(); for attr in node.attributes() { @@ -299,7 +331,8 @@ pub(crate) fn element_to_tokens( let name = node.name(); if is_component_node(node) { if let Some(slot) = get_slot(node) { - slot_to_tokens(node, slot, parent_slots, global_class); + let slot = slot.clone(); + slot_to_tokens(node, &slot, parent_slots, global_class); None } else { Some(component_to_tokens(node, global_class)) @@ -414,7 +447,7 @@ pub(crate) fn element_to_tokens( let self_closing = is_self_closing(node); let children = if !self_closing { element_children_to_tokens( - &node.children, + &mut node.children, parent_type, parent_slots, global_class, diff --git a/leptos_macro/src/view/slot_helper.rs b/leptos_macro/src/view/slot_helper.rs index 561e092ba..7c570e301 100644 --- a/leptos_macro/src/view/slot_helper.rs +++ b/leptos_macro/src/view/slot_helper.rs @@ -7,7 +7,7 @@ use std::collections::HashMap; use syn::spanned::Spanned; pub(crate) fn slot_to_tokens( - node: &NodeElement, + node: &mut NodeElement, slot: &KeyedAttribute, parent_slots: Option<&mut HashMap>>, global_class: Option<&TokenTree>, @@ -30,20 +30,25 @@ pub(crate) fn slot_to_tokens( return; }; - let attrs = node.attributes().iter().filter_map(|node| { - if let NodeAttribute::Attribute(node) = node { - if is_slot(node) { - None + let attrs = node + .attributes() + .iter() + .filter_map(|node| { + if let NodeAttribute::Attribute(node) = node { + if is_slot(node) { + None + } else { + Some(node) + } } else { - Some(node) + None } - } else { - None - } - }); + }) + .cloned() + .collect::>(); let props = attrs - .clone() + .iter() .filter(|attr| { !attr.key.to_string().starts_with("let:") && !attr.key.to_string().starts_with("clone:") @@ -65,7 +70,7 @@ pub(crate) fn slot_to_tokens( }); let items_to_bind = attrs - .clone() + .iter() .filter_map(|attr| { attr.key .to_string() @@ -75,7 +80,7 @@ pub(crate) fn slot_to_tokens( .collect::>(); let items_to_clone = attrs - .clone() + .iter() .filter_map(|attr| { attr.key .to_string() @@ -85,6 +90,7 @@ pub(crate) fn slot_to_tokens( .collect::>(); let dyn_attrs = attrs + .iter() .filter(|attr| attr.key.to_string().starts_with("attr:")) .filter_map(|attr| { let name = &attr.key.to_string(); @@ -107,7 +113,7 @@ pub(crate) fn slot_to_tokens( quote! {} } else { let children = fragment_to_tokens( - &node.children, + &mut node.children, TagType::Unknown, Some(&mut slots), global_class, From 8dc600ca02d3ef08a200a3f60bab3b5474c85345 Mon Sep 17 00:00:00 2001 From: Greg Johnston Date: Mon, 16 Sep 2024 21:33:46 -0400 Subject: [PATCH 2/3] fix: do not sort class and style that are after spread marker --- leptos_macro/src/view/mod.rs | 63 ++++++++++++++++++++++++------------ 1 file changed, 43 insertions(+), 20 deletions(-) diff --git a/leptos_macro/src/view/mod.rs b/leptos_macro/src/view/mod.rs index 11902abb0..1aa022d23 100644 --- a/leptos_macro/src/view/mod.rs +++ b/leptos_macro/src/view/mod.rs @@ -279,10 +279,25 @@ pub(crate) fn element_to_tokens( global_class: Option<&TokenTree>, view_marker: Option<&str>, ) -> Option { + // attribute sorting: + // // the `class` and `style` attributes overwrite individual `class:` and `style:` attributes // when they are set. as a result, we're going to sort the attributes so that `class` and // `style` always come before all other attributes. - node.attributes_mut().sort_by(|a, b| { + + // if there's a spread marker, we don't want to move `class` or `style` before it + // so let's only sort attributes that come *before* a spread marker + let spread_position = node + .attributes() + .iter() + .position(|n| match n { + NodeAttribute::Block(node) => as_spread_attr(node).is_some(), + _ => false, + }) + .unwrap_or_else(|| node.attributes().len()); + + // now, sort the attributes + node.attributes_mut()[0..spread_position].sort_by(|a, b| { let key_a = match a { NodeAttribute::Attribute(attr) => match &attr.key { NodeName::Path(attr) => { @@ -496,6 +511,25 @@ fn is_spread_marker(node: &NodeElement) -> bool { } } +fn as_spread_attr(node: &NodeBlock) -> Option> { + if let NodeBlock::ValidBlock(block) = node { + match block.stmts.first() { + Some(Stmt::Expr( + Expr::Range(ExprRange { + start: None, + limits: RangeLimits::HalfOpen(_), + end, + .. + }), + _, + )) => Some(end.as_deref()), + _ => None, + } + } else { + None + } +} + fn attribute_to_tokens( tag_type: TagType, node: &NodeAttribute, @@ -503,29 +537,18 @@ fn attribute_to_tokens( is_custom: bool, ) -> TokenStream { match node { - NodeAttribute::Block(node) => { - let dotted = if let NodeBlock::ValidBlock(block) = node { - match block.stmts.first() { - Some(Stmt::Expr( - Expr::Range(ExprRange { - start: None, - limits: RangeLimits::HalfOpen(_), - end: Some(end), - .. - }), - _, - )) => Some(quote! { .add_any_attr(#end) }), - _ => None, + NodeAttribute::Block(node) => as_spread_attr(node) + .flatten() + .map(|end| { + quote! { + .add_any_attr(#end) } - } else { - None - }; - dotted.unwrap_or_else(|| { + }) + .unwrap_or_else(|| { quote! { .add_any_attr(#[allow(unused_braces)] { #node }) } - }) - } + }), NodeAttribute::Attribute(node) => { let name = node.key.to_string(); if name == "node_ref" { From 9ca36d4763580ffcf4fb02427bfa7e42c1f08230 Mon Sep 17 00:00:00 2001 From: Greg Johnston Date: Mon, 16 Sep 2024 21:34:04 -0400 Subject: [PATCH 3/3] chore: remove deprecated function in doctest example --- meta/src/body.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meta/src/body.rs b/meta/src/body.rs index 8cc3552bf..1030a5075 100644 --- a/meta/src/body.rs +++ b/meta/src/body.rs @@ -29,7 +29,7 @@ use leptos::{ /// #[component] /// fn MyApp() -> impl IntoView { /// provide_meta_context(); -/// let (prefers_dark, set_prefers_dark) = create_signal(false); +/// let (prefers_dark, set_prefers_dark) = signal(false); /// let body_class = move || { /// if prefers_dark.get() { /// "dark".to_string()