From 745d492d5b05e7fef521179ea2dd9f3bc9316cbe Mon Sep 17 00:00:00 2001 From: Manuel Cillero Date: Mon, 17 Aug 2026 00:02:20 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20(html):=20Evita=20duplicar=20atribu?= =?UTF-8?q?tos=20de=20Props?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `html!` recopila los atributos literales de cada elemento y evita que `Props` los duplique al renderizar. --- helpers/pagetop-macros/src/maud/generate.rs | 30 ++++- src/html.rs | 4 +- src/html/maud.rs | 125 +++++++++++++++++--- src/html/props.rs | 105 +++++++++++++--- tests/html_props.rs | 52 +++++++- 5 files changed, 274 insertions(+), 42 deletions(-) diff --git a/helpers/pagetop-macros/src/maud/generate.rs b/helpers/pagetop-macros/src/maud/generate.rs index ed2fa214..6e4649ba 100644 --- a/helpers/pagetop-macros/src/maud/generate.rs +++ b/helpers/pagetop-macros/src/maud/generate.rs @@ -1,6 +1,6 @@ use proc_macro2::{Ident, Span, TokenStream}; use quote::{ToTokens, quote}; -use syn::{Expr, Local, parse_quote, token::Brace}; +use syn::{Expr, LitStr, Local, parse_quote, token::Brace}; use crate::maud::{ast::*, escape}; @@ -71,6 +71,17 @@ impl Generator { ); } + fn splice_attrs(&self, expr: Expr, exclude: &[LitStr], build: &mut Builder) { + let output_ident = &self.output_ident; + build.push_tokens(quote!( + pagetop::html::html_private::render_attrs_to!( + &(#expr), + &[#(#exclude),*], + &mut #output_ident + ); + )); + } + fn element(&self, element: Element, build: &mut Builder) { let element_name = element.name.clone().unwrap_or_else(|| parse_quote!(div)); build.push_str("<"); @@ -141,6 +152,21 @@ impl Generator { fn attrs(&self, attrs: Vec, build: &mut Builder) { let (classes, id, named_attrs, spliced) = split_attrs(attrs); + // Must run before `classes`/`id`/`named_attrs` are consumed below. + let literal_attr_names: Vec = { + let mut names = Vec::new(); + if !classes.is_empty() { + names.push(LitStr::new("class", Span::call_site())); + } + if id.is_some() { + names.push(LitStr::new("id", Span::call_site())); + } + for (name, _) in &named_attrs { + names.push(LitStr::new(&name.to_string(), Span::call_site())); + } + names + }; + if !classes.is_empty() { let mut toggle_class_exprs = vec![]; @@ -185,7 +211,7 @@ impl Generator { self.attr(name, attr_type, build); } for expr in spliced { - self.splice(expr, build); + self.splice_attrs(expr, &literal_attr_names, build); } } diff --git a/src/html.rs b/src/html.rs index 1f8e346e..a2d4d2d0 100644 --- a/src/html.rs +++ b/src/html.rs @@ -1,7 +1,9 @@ //! HTML en código. pub(crate) mod maud; -pub use maud::{DOCTYPE, Escaper, Markup, PreEscaped, Render, display, html, html_private}; +pub use maud::DOCTYPE; +pub use maud::{Escaper, Markup, PreEscaped, Render, RenderAttrs}; +pub use maud::{display, html, html_private}; mod route_path; pub use route_path::RoutePath; diff --git a/src/html/maud.rs b/src/html/maud.rs index cfc35105..858ed25a 100644 --- a/src/html/maud.rs +++ b/src/html/maud.rs @@ -2,8 +2,8 @@ //! A macro for writing HTML templates. //! -//! This documentation only describes the runtime API. For a general -//! guide, check out the [book] instead. +//! This documentation only describes the runtime API. For a general guide, check out the [book] +//! instead. //! //! [book]: https://maud.lambda.xyz/ @@ -29,8 +29,7 @@ mod escape; /// /// All other characters are passed through unchanged. /// -/// **Note:** In versions prior to 0.13, the single quote (`'`) was -/// escaped as well. +/// **Note:** In versions prior to 0.13, the single quote (`'`) was escaped as well. /// /// # Example /// @@ -59,15 +58,14 @@ impl fmt::Write for Escaper<'_> { /// Representa un tipo que puede renderizarse como HTML. /// -/// To implement this for your own type, override either the `.render()` -/// or `.render_to()` methods; since each is defined in terms of the -/// other, you only need to implement one of them. See the example below. +/// To implement this for your own type, override either the `.render()` or `.render_to()` methods; +/// since each is defined in terms of the other, you only need to implement one of them. See the +/// example below. /// /// # Minimal implementation /// -/// An implementation of this trait must override at least one of -/// `.render()` or `.render_to()`. Since the default definitions of -/// these methods call each other, not doing this will result in +/// An implementation of this trait must override at least one of `.render()` or `.render_to()`. +/// Since the default definitions of these methods call each other, not doing this will result in /// infinite recursion. pub trait Render { /// Renders `self` as a block of `Markup`. @@ -79,13 +77,12 @@ pub trait Render { /// Appends a representation of `self` to the given buffer. /// - /// Its default implementation just calls `.render()`, but you may - /// override it with something more efficient. + /// Its default implementation just calls `.render()`, but you may override it with something + /// more efficient. /// - /// Note that no further escaping is performed on data written to - /// the buffer. If you override this method, you must make sure that - /// any data written is properly escaped, whether by hand or using - /// the [`Escaper`](struct.Escaper.html) wrapper struct. + /// Note that no further escaping is performed on data written to the buffer. If you override + /// this method, you must make sure that any data written is properly escaped, whether by hand + /// or using the [`Escaper`](struct.Escaper.html) wrapper struct. fn render_to(&self, buffer: &mut String) { buffer.push_str(&self.render().into_string()); } @@ -139,6 +136,27 @@ impl Render for Arc { } } +/// Representa un tipo que puede renderizarse como los atributos de un elemento HTML. +/// +/// Exists so that a single value "spliced" into the attribute position of an element can avoid +/// duplicating an attribute the element already writes literally. The [`html!`](crate::html::html) +/// macro automatically computes the names of the element's literal attributes and passes them here; +/// no action is required from the programmer. +/// +/// [`Props`](crate::html::Props) is the only implementation in PageTop. +pub trait RenderAttrs { + /// Same as [`Render::render_to()`], but omitting any attribute whose name is in `exclude`. + #[track_caller] + fn render_attrs_to(&self, buffer: &mut String, exclude: &[&str]); +} + +impl RenderAttrs for &T { + #[track_caller] + fn render_attrs_to(&self, buffer: &mut String, exclude: &[&str]) { + T::render_attrs_to(self, buffer, exclude); + } +} + macro_rules! impl_render_with_display { ($($ty:ty)*) => { $( @@ -286,7 +304,7 @@ mod axum_support { pub mod html_private { extern crate alloc; - use super::{Render, display}; + use super::{Render, RenderAttrs, display}; use alloc::string::String; use core::fmt::Display; @@ -333,4 +351,77 @@ pub mod html_private { display(value).render_to(buffer); } } + + #[doc(hidden)] + #[macro_export] + macro_rules! render_attrs_to { + ($x:expr, $exclude:expr, $buffer:expr) => {{ + use $crate::html::html_private::*; + match ChooseAttrsRenderOrDisplay($x) { + x => (&&&x) + .implements_attrs_render_or_display() + .render_to(x.0, $exclude, $buffer), + } + }}; + } + + pub use render_attrs_to; + + pub struct ChooseAttrsRenderOrDisplay(pub T); + + pub struct ViaAttrsTag; + pub struct ViaAttrsRenderTag; + pub struct ViaAttrsDisplayTag; + + pub trait ViaAttrs { + fn implements_attrs_render_or_display(&self) -> ViaAttrsTag { + ViaAttrsTag + } + } + pub trait ViaAttrsRender { + fn implements_attrs_render_or_display(&self) -> ViaAttrsRenderTag { + ViaAttrsRenderTag + } + } + pub trait ViaAttrsDisplay { + fn implements_attrs_render_or_display(&self) -> ViaAttrsDisplayTag { + ViaAttrsDisplayTag + } + } + + impl ViaAttrs for &&ChooseAttrsRenderOrDisplay {} + impl ViaAttrsRender for &ChooseAttrsRenderOrDisplay {} + impl ViaAttrsDisplay for ChooseAttrsRenderOrDisplay {} + + impl ViaAttrsTag { + #[track_caller] + pub fn render_to( + self, + value: &T, + exclude: &[&str], + buffer: &mut String, + ) { + value.render_attrs_to(buffer, exclude); + } + } + impl ViaAttrsRenderTag { + pub fn render_to( + self, + value: &T, + _exclude: &[&str], + buffer: &mut String, + ) { + value.render_to(buffer); + } + } + impl ViaAttrsDisplayTag { + pub fn render_to( + self, + value: &T, + _exclude: &[&str], + buffer: &mut String, + ) { + display(value).render_to(buffer); + } + } } diff --git a/src/html/props.rs b/src/html/props.rs index 616b3cf7..04c35fe7 100644 --- a/src/html/props.rs +++ b/src/html/props.rs @@ -1,5 +1,5 @@ use crate::core::TypeInfo; -use crate::html::maud::{Escaper, Render}; +use crate::html::maud::{Escaper, RenderAttrs}; use crate::{AutoDefault, CowStr, builder_fn, trace, util}; use thiserror::Error; @@ -7,6 +7,7 @@ use thiserror::Error; use std::any::Any; use std::collections::HashMap; use std::fmt::{self, Write}; +use std::panic::Location; use std::sync::Arc; // **< PropsExtra >********************************************************************************* @@ -350,6 +351,24 @@ impl PropsOp { /// assert_eq!(markup.into_string(), r#""#); /// ``` /// +/// # Atributos duplicados junto a `Props` +/// +/// Cuando el componente combina `(self.props())` con un atributo literal del mismo nombre en el +/// mismo elemento (una clase, un `#id`, o `nombre=valor`), la macro [`html!`](crate::html::html) +/// evita automáticamente la duplicación. Recopila en tiempo de compilación los nombres de los +/// atributos del elemento y al renderizar se omiten los duplicados en tiempo de ejecución. No +/// depende del orden en que se escriban ni requiere ninguna acción del desarrollador. +/// +/// ```rust +/// # use pagetop::prelude::*; +/// let props = Props::default().with_prop(PropsOp::set("title", "de Props")); +/// +/// let markup = html! { span title="literal" (props) { "OK" } }; +/// +/// // El atributo literal prevalece; `Props` omite su propio "title" en vez de duplicarlo. +/// assert_eq!(markup.into_string(), r#"OK"#); +/// ``` +/// /// # Valores extra /// /// Las variantes [`SetExtra`](PropsOp::SetExtra) y [`RemoveExtra`](PropsOp::RemoveExtra), usando @@ -881,32 +900,82 @@ impl Props { } #[doc(hidden)] -impl Render for Props { - fn render_to(&self, w: &mut String) { +impl RenderAttrs for Props { + // Omite cualquier atributo que esté en `exclude` (recopilados por `html!` a partir de los + // atributos literales del elemento). Registra un `trace::debug!` por cada atributo duplicado, + // con la posición exacta del `html!` que lo produjo (propagado gracias a `#[track_caller]`) + // para facilitar la localización del problema. + #[track_caller] + fn render_attrs_to(&self, w: &mut String, exclude: &[&str]) { if let Some(id) = self.id.as_deref() { - w.push_str(" id=\""); - let _ = write!(Escaper::new(w), "{}", id); - w.push('"'); + if exclude.contains(&"id") { + trace::debug!( + caller = %Location::caller(), + attribute = "id", + discarded = %id, + "Ignoring Props attribute already set as a literal on the same element" + ); + } else { + w.push_str(" id=\""); + let _ = write!(Escaper::new(w), "{}", id); + w.push('"'); + } } if let Some((first, rest)) = self.classes.split_first() { - w.push_str(" class=\""); - let _ = write!(Escaper::new(w), "{}", first); - for class in rest { - w.push(' '); - let _ = write!(Escaper::new(w), "{}", class); + if exclude.contains(&"class") { + trace::debug!( + caller = %Location::caller(), + attribute = "class", + discarded = %self.classes.join(" "), + id = %self.id.as_deref().unwrap_or(""), + "Ignoring Props attribute already set as a literal on the same element" + ); + } else { + w.push_str(" class=\""); + let _ = write!(Escaper::new(w), "{}", first); + for class in rest { + w.push(' '); + let _ = write!(Escaper::new(w), "{}", class); + } + w.push('"'); } - w.push('"'); } if let Some((first, rest)) = self.styles.split_first() { - w.push_str(" style=\""); - let _ = write!(Escaper::new(w), "{}: {}", first.0, first.1); - for (property, value) in rest { - w.push_str("; "); - let _ = write!(Escaper::new(w), "{}: {}", property, value); + if exclude.contains(&"style") { + let discarded = self + .styles + .iter() + .map(|(property, value)| format!("{property}: {value}")) + .collect::>() + .join("; "); + trace::debug!( + caller = %Location::caller(), + attribute = "style", + discarded = %discarded, + id = %self.id.as_deref().unwrap_or(""), + "Ignoring Props attribute already set as a literal on the same element" + ); + } else { + w.push_str(" style=\""); + let _ = write!(Escaper::new(w), "{}: {}", first.0, first.1); + for (property, value) in rest { + w.push_str("; "); + let _ = write!(Escaper::new(w), "{}: {}", property, value); + } + w.push('"'); } - w.push('"'); } for (name, value) in &self.attrs { + if exclude.contains(&name.as_ref()) { + trace::debug!( + caller = %Location::caller(), + attribute = %name, + discarded = %value, + id = %self.id.as_deref().unwrap_or(""), + "Ignoring Props attribute already set as a literal on the same element" + ); + continue; + } w.push(' '); let _ = write!(Escaper::new(w), "{}", name); w.push_str("=\""); diff --git a/tests/html_props.rs b/tests/html_props.rs index 6b5f751e..afcfe929 100644 --- a/tests/html_props.rs +++ b/tests/html_props.rs @@ -157,11 +157,13 @@ async fn props_alongside_named_attr_renders_after_it() { } #[pagetop::test] -async fn props_multiple_splices_in_same_element() { - let p1 = Props::new("hx-get", "/api"); - let p2 = Props::new("hx-swap", "outerHTML"); +async fn props_combined_via_chaining_instead_of_multiple_splices() { + // An element accepts only a single attribute splice (a second `(props)` on the same element + // is a compile error); values from separate sources are combined by chaining `with_prop()` + // on one `Props`, not by splicing two of them. + let p = Props::new("hx-get", "/api").with_prop(PropsOp::set("hx-swap", "outerHTML")); assert_eq!( - html! { button (p1) (p2) {} }.into_string(), + html! { button (p) {} }.into_string(), r#""# ); } @@ -193,6 +195,48 @@ async fn props_splice_empty_string_emits_nothing() { assert_eq!(html! { span ("") { "x" } }.into_string(), "x"); } +// **< RenderAttrs: literal attribute collisions >************************************************** + +#[pagetop::test] +async fn props_id_collision_with_literal_omits_props_id() { + // A literal `#id` on the element takes precedence; `Props`'s own id is silently omitted instead + // of producing a duplicate `id` attribute. + let p = Props::default().with_id("from-props"); + assert_eq!( + html! { div #fixed (p) {} }.into_string(), + r#"
"# + ); +} + +#[pagetop::test] +async fn props_class_collision_with_literal_omits_props_classes() { + let p = Props::classes("from-props-a from-props-b"); + assert_eq!( + html! { div.fixed (p) {} }.into_string(), + r#"
"# + ); +} + +#[pagetop::test] +async fn props_style_collision_with_literal_omits_props_styles() { + let p = Props::default() + .with_prop(PropsOp::add_style("color", "red")) + .with_prop(PropsOp::add_style("font-weight", "bold")); + assert_eq!( + html! { div style="color: blue" (p) {} }.into_string(), + r#"
"# + ); +} + +#[pagetop::test] +async fn props_named_attr_collision_with_literal_omits_props_value() { + let p = Props::default().with_prop(PropsOp::set("title", "from-props")); + assert_eq!( + html! { span title="literal" (p) {} }.into_string(), + r#""# + ); +} + // **< is_attrs_empty / is_empty >****************************************************************** #[pagetop::test]