From 6580ed454ac76688c2c848ea1f8b949f2b3c9458 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Sat, 15 Mar 2025 14:40:06 -0500 Subject: [PATCH 1/2] allow spread attributes to be set directly --- packages/core-macro/src/props/mod.rs | 127 ++++++++----------- packages/core-macro/tests/forward_spreads.rs | 40 ++++++ packages/core/src/nodes.rs | 7 +- packages/html-internal-macro/src/lib.rs | 2 +- 4 files changed, 102 insertions(+), 74 deletions(-) create mode 100644 packages/core-macro/tests/forward_spreads.rs diff --git a/packages/core-macro/src/props/mod.rs b/packages/core-macro/src/props/mod.rs index 2a52d1c5c3..a0b5f6b60a 100644 --- a/packages/core-macro/src/props/mod.rs +++ b/packages/core-macro/src/props/mod.rs @@ -33,7 +33,9 @@ pub fn impl_my_derive(ast: &syn::DeriveInput) -> Result { let fields = quote!(#(#fields)*).into_iter(); let required_fields = struct_info .included_fields() - .filter(|f| f.builder_attr.default.is_none()) + .filter(|f| { + f.builder_attr.default.is_none() && f.builder_attr.extends.is_empty() + }) .map(|f| struct_info.required_field_impl(f)) .collect::, _>>()?; let build_method = struct_info.build_method_impl(); @@ -566,9 +568,7 @@ mod struct_info { impl<'a> StructInfo<'a> { pub fn included_fields(&self) -> impl Iterator> { - self.fields - .iter() - .filter(|f| !f.builder_attr.skip && f.builder_attr.extends.is_empty()) + self.fields.iter().filter(|f| !f.builder_attr.skip) } pub fn extend_fields(&self) -> impl Iterator> { @@ -670,7 +670,6 @@ mod struct_info { let regular_fields: Vec<_> = self .included_fields() - .chain(self.extend_fields()) .filter(|f| !looks_like_signal_type(f.ty) && !looks_like_callback_type(f.ty)) .map(|f| { let name = f.name; @@ -837,24 +836,10 @@ Finally, call `.build()` to create the instance of `{name}`. let memoize = self.memoize_impl()?; - let global_fields = self - .extend_fields() - .map(|f| { - let name = f.name; - let ty = f.ty; - quote!(#name: #ty) - }) - .chain(self.has_child_owned_fields().then(|| quote!(owner: Owner))); + let global_fields = self.has_child_owned_fields().then(|| quote!(owner: Owner,)); let global_fields_value = self - .extend_fields() - .map(|f| { - let name = f.name; - quote!(#name: Vec::new()) - }) - .chain( - self.has_child_owned_fields() - .then(|| quote!(owner: Owner::default())), - ); + .has_child_owned_fields() + .then(|| quote!(owner: Owner::default(),)); Ok(quote! { impl #impl_generics #name #ty_generics #where_clause { @@ -862,7 +847,7 @@ Finally, call `.build()` to create the instance of `{name}`. #[allow(dead_code, clippy::type_complexity)] #vis fn builder() -> #builder_name #generics_with_empty { #builder_name { - #(#global_fields_value,)* + #global_fields_value fields: #empties_tuple, _phantom: ::core::default::Default::default(), } @@ -873,7 +858,7 @@ Finally, call `.build()` to create the instance of `{name}`. #builder_type_doc #[allow(dead_code, non_camel_case_types, non_snake_case)] #vis struct #builder_name #b_generics { - #(#global_fields,)* + #global_fields fields: #all_fields_param, _phantom: (#( #phantom_generics ),*), } @@ -925,12 +910,8 @@ Finally, call `.build()` to create the instance of `{name}`. let field_name = field.name; let descructuring = self.included_fields().map(|f| { - if f.ordinal == field.ordinal { - quote!(_) - } else { - let name = f.name; - quote!(#name) - } + let name = f.name; + quote!(#name) }); let reconstructing = self.included_fields().map(|f| f.name); @@ -962,7 +943,12 @@ Finally, call `.build()` to create the instance of `{name}`. .count(); for f in self.included_fields() { if f.ordinal == field.ordinal { - ty_generics_tuple.elems.push_value(empty_type()); + g.params.insert( + index_after_lifetime_in_generics, + syn::GenericParam::Type(self.generic_builder_param(f)), + ); + let generic_argument: syn::Type = f.type_ident(); + ty_generics_tuple.elems.push_value(generic_argument.clone()); target_generics_tuple .elems .push_value(f.tuplized_type_ty_param()); @@ -992,11 +978,6 @@ Finally, call `.build()` to create the instance of `{name}`. ); let (impl_generics, _, where_clause) = generics.split_for_impl(); - let forward_extended_fields = self.extend_fields().map(|f| { - let name = f.name; - quote!(#name: self.#name) - }); - let forward_owner = self .has_child_owned_fields() .then(|| quote!(owner: self.owner)) @@ -1015,18 +996,23 @@ Finally, call `.build()` to create the instance of `{name}`. } }); + let helper_trait_name = &self.conversion_helper_trait_name; + Ok(quote! { #[allow(dead_code, non_camel_case_types, missing_docs)] impl #impl_generics dioxus_core::prelude::HasAttributes for #builder_name < #( #ty_generics ),* > #where_clause { + type Output = #builder_name < #( #target_generics ),* >; + fn push_attribute( mut self, ____name: &'static str, ____ns: Option<&'static str>, ____attr: impl dioxus_core::prelude::IntoAttributeValue, ____volatile: bool - ) -> Self { + ) -> Self::Output { let ( #(#descructuring,)* ) = self.fields; - self.#field_name.push( + let mut #field_name = #helper_trait_name::into_value(#field_name, || ::core::default::Default::default()); + #field_name.push( dioxus_core::Attribute::new( ____name, { @@ -1037,8 +1023,8 @@ Finally, call `.build()` to create the instance of `{name}`. ____volatile, ) ); + let #field_name = (#field_name,); #builder_name { - #(#forward_extended_fields,)* #(#forward_owner,)* fields: ( #(#reconstructing,)* ), _phantom: self._phantom, @@ -1174,15 +1160,8 @@ Finally, call `.build()` to create the instance of `{name}`. let repeated_fields_error_message = format!("Repeated field {field_name}"); let forward_fields = self - .extend_fields() - .map(|f| { - let name = f.name; - quote!(#name: self.#name) - }) - .chain( - self.has_child_owned_fields() - .then(|| quote!(owner: self.owner)), - ); + .has_child_owned_fields() + .then(|| quote!(owner: self.owner,)); Ok(quote! { #[allow(dead_code, non_camel_case_types, missing_docs)] @@ -1193,7 +1172,7 @@ Finally, call `.build()` to create the instance of `{name}`. let #field_name = (#arg_expr,); let ( #(#descructuring,)* ) = self.fields; #builder_name { - #(#forward_fields,)* + #forward_fields fields: ( #(#reconstructing,)* ), _phantom: self._phantom, } @@ -1323,6 +1302,31 @@ Finally, call `.build()` to create the instance of `{name}`. }) } + fn generic_builder_param(&self, field: &FieldInfo) -> syn::TypeParam { + let trait_ref = syn::TraitBound { + paren_token: None, + lifetimes: None, + modifier: syn::TraitBoundModifier::None, + path: syn::PathSegment { + ident: self.conversion_helper_trait_name.clone(), + arguments: syn::PathArguments::AngleBracketed( + syn::AngleBracketedGenericArguments { + colon2_token: None, + lt_token: Default::default(), + args: make_punctuated_single(syn::GenericArgument::Type( + field.ty.clone(), + )), + gt_token: Default::default(), + }, + ), + } + .into(), + }; + let mut generic_param: syn::TypeParam = field.generic_ident.clone().into(); + generic_param.bounds.push(trait_ref.into()); + generic_param + } + pub fn build_method_impl(&self) -> TokenStream { let StructInfo { ref name, @@ -1338,27 +1342,7 @@ Finally, call `.build()` to create the instance of `{name}`. .count(); for field in self.included_fields() { if field.builder_attr.default.is_some() { - let trait_ref = syn::TraitBound { - paren_token: None, - lifetimes: None, - modifier: syn::TraitBoundModifier::None, - path: syn::PathSegment { - ident: self.conversion_helper_trait_name.clone(), - arguments: syn::PathArguments::AngleBracketed( - syn::AngleBracketedGenericArguments { - colon2_token: None, - lt_token: Default::default(), - args: make_punctuated_single(syn::GenericArgument::Type( - field.ty.clone(), - )), - gt_token: Default::default(), - }, - ), - } - .into(), - }; - let mut generic_param: syn::TypeParam = field.generic_ident.clone().into(); - generic_param.bounds.push(trait_ref.into()); + let generic_param = self.generic_builder_param(field); g.params .insert(index_after_lifetime_in_generics, generic_param.into()); } @@ -1396,9 +1380,8 @@ Finally, call `.build()` to create the instance of `{name}`. let assignments = self.fields.iter().map(|field| { let name = &field.name; if !field.builder_attr.extends.is_empty() { - quote!(let #name = self.#name;) + quote!(let #name = #helper_trait_name::into_value(#name, || ::core::default::Default::default());) } else if let Some(ref default) = field.builder_attr.default { - // If field has `into`, apply it to the default value. // Ignore any blank defaults as it causes type inference errors. let is_default = *default == parse_quote!(::core::default::Default::default()); diff --git a/packages/core-macro/tests/forward_spreads.rs b/packages/core-macro/tests/forward_spreads.rs new file mode 100644 index 0000000000..227ca30d8e --- /dev/null +++ b/packages/core-macro/tests/forward_spreads.rs @@ -0,0 +1,40 @@ +use dioxus::prelude::*; +use dioxus_core::ElementId; +use std::{any::Any, rc::Rc}; + +// Regression test for https://github.com/DioxusLabs/dioxus/issues/3844 +#[test] +fn forward_spreads() { + #[derive(Props, Clone, PartialEq)] + struct Comp1Props { + #[props(extends = GlobalAttributes)] + attributes: Vec, + } + + #[component] + fn Comp1(props: Comp1Props) -> Element { + rsx! { + Comp2 { + attributes: props.attributes, + height: "100%", + } + } + } + + #[derive(Props, Clone, PartialEq)] + struct CompProps2 { + #[props(extends = GlobalAttributes)] + attributes: Vec, + } + + #[component] + fn Comp2(props: CompProps2) -> Element { + rsx! {} + } + + rsx! { + Comp1 { + width: "100%" + } + }; +} diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index bc50d9bc9c..a3fb84ad19 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -1222,6 +1222,11 @@ where /// A trait for anything that has a dynamic list of attributes pub trait HasAttributes { + /// The output type after the attribute has been added. This + /// can be used by the properties macro to prevent duplicate + /// attributes + type Output; + /// Push an attribute onto the list of attributes fn push_attribute( self, @@ -1229,5 +1234,5 @@ pub trait HasAttributes { ns: Option<&'static str>, attr: impl IntoAttributeValue, volatile: bool, - ) -> Self; + ) -> Self::Output; } diff --git a/packages/html-internal-macro/src/lib.rs b/packages/html-internal-macro/src/lib.rs index 88d857d8af..73a1254f60 100644 --- a/packages/html-internal-macro/src/lib.rs +++ b/packages/html-internal-macro/src/lib.rs @@ -43,7 +43,7 @@ impl ToTokens for ImplExtensionAttributes { let impls = self.attrs.iter().map(|ident| { let d = quote! { #name::#ident }; quote! { - fn #ident(self, value: impl IntoAttributeValue) -> Self { + fn #ident(self, value: impl IntoAttributeValue) -> Self::Output { let d = #d; self.push_attribute(d.0, d.1, value, d.2) } From 3920dbfd12cef54f55c8b2e10819ffe058c2a78d Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Thu, 20 Mar 2025 08:26:56 -0500 Subject: [PATCH 2/2] Remove ordering requirement --- packages/core-macro/src/props/mod.rs | 71 +++++++++++++------ packages/core/src/nodes.rs | 7 +- packages/document/src/elements/link.rs | 8 +++ packages/html-internal-macro/src/lib.rs | 2 +- .../tests/forward_spreads.rs | 29 ++++++-- 5 files changed, 83 insertions(+), 34 deletions(-) rename packages/{core-macro => ssr}/tests/forward_spreads.rs (51%) diff --git a/packages/core-macro/src/props/mod.rs b/packages/core-macro/src/props/mod.rs index a0b5f6b60a..7cde3ee5af 100644 --- a/packages/core-macro/src/props/mod.rs +++ b/packages/core-macro/src/props/mod.rs @@ -174,7 +174,7 @@ mod util { mod field_info { use crate::props::type_from_inside_option; use proc_macro2::TokenStream; - use quote::quote; + use quote::{format_ident, quote}; use syn::spanned::Spanned; use syn::{parse::Error, punctuated::Punctuated}; use syn::{parse_quote, Expr, Path}; @@ -271,6 +271,13 @@ mod field_info { } .into() } + + pub fn extends_vec_ident(&self) -> Option { + (!self.builder_attr.extends.is_empty()).then(|| { + let ident = &self.name; + format_ident!("__{ident}_attributes") + }) + } } #[derive(Debug, Default, Clone)] @@ -836,10 +843,24 @@ Finally, call `.build()` to create the instance of `{name}`. let memoize = self.memoize_impl()?; - let global_fields = self.has_child_owned_fields().then(|| quote!(owner: Owner,)); + let global_fields = self + .extend_fields() + .map(|f| { + let name = f.extends_vec_ident(); + let ty = f.ty; + quote!(#name: #ty) + }) + .chain(self.has_child_owned_fields().then(|| quote!(owner: Owner))); let global_fields_value = self - .has_child_owned_fields() - .then(|| quote!(owner: Owner::default(),)); + .extend_fields() + .map(|f| { + let name = f.extends_vec_ident(); + quote!(#name: Vec::new()) + }) + .chain( + self.has_child_owned_fields() + .then(|| quote!(owner: Owner::default())), + ); Ok(quote! { impl #impl_generics #name #ty_generics #where_clause { @@ -847,7 +868,7 @@ Finally, call `.build()` to create the instance of `{name}`. #[allow(dead_code, clippy::type_complexity)] #vis fn builder() -> #builder_name #generics_with_empty { #builder_name { - #global_fields_value + #(#global_fields_value,)* fields: #empties_tuple, _phantom: ::core::default::Default::default(), } @@ -858,7 +879,7 @@ Finally, call `.build()` to create the instance of `{name}`. #builder_type_doc #[allow(dead_code, non_camel_case_types, non_snake_case)] #vis struct #builder_name #b_generics { - #global_fields + #(#global_fields,)* fields: #all_fields_param, _phantom: (#( #phantom_generics ),*), } @@ -907,7 +928,7 @@ Finally, call `.build()` to create the instance of `{name}`. ref builder_name, .. } = *self; - let field_name = field.name; + let field_name = field.extends_vec_ident().unwrap(); let descructuring = self.included_fields().map(|f| { let name = f.name; @@ -978,6 +999,11 @@ Finally, call `.build()` to create the instance of `{name}`. ); let (impl_generics, _, where_clause) = generics.split_for_impl(); + let forward_extended_fields = self.extend_fields().map(|f| { + let name = f.extends_vec_ident(); + quote!(#name: self.#name) + }); + let forward_owner = self .has_child_owned_fields() .then(|| quote!(owner: self.owner)) @@ -996,23 +1022,18 @@ Finally, call `.build()` to create the instance of `{name}`. } }); - let helper_trait_name = &self.conversion_helper_trait_name; - Ok(quote! { #[allow(dead_code, non_camel_case_types, missing_docs)] impl #impl_generics dioxus_core::prelude::HasAttributes for #builder_name < #( #ty_generics ),* > #where_clause { - type Output = #builder_name < #( #target_generics ),* >; - fn push_attribute( mut self, ____name: &'static str, ____ns: Option<&'static str>, ____attr: impl dioxus_core::prelude::IntoAttributeValue, ____volatile: bool - ) -> Self::Output { + ) -> Self { let ( #(#descructuring,)* ) = self.fields; - let mut #field_name = #helper_trait_name::into_value(#field_name, || ::core::default::Default::default()); - #field_name.push( + self.#field_name.push( dioxus_core::Attribute::new( ____name, { @@ -1023,8 +1044,8 @@ Finally, call `.build()` to create the instance of `{name}`. ____volatile, ) ); - let #field_name = (#field_name,); #builder_name { + #(#forward_extended_fields,)* #(#forward_owner,)* fields: ( #(#reconstructing,)* ), _phantom: self._phantom, @@ -1160,8 +1181,15 @@ Finally, call `.build()` to create the instance of `{name}`. let repeated_fields_error_message = format!("Repeated field {field_name}"); let forward_fields = self - .has_child_owned_fields() - .then(|| quote!(owner: self.owner,)); + .extend_fields() + .map(|f| { + let name = f.extends_vec_ident(); + quote!(#name: self.#name) + }) + .chain( + self.has_child_owned_fields() + .then(|| quote!(owner: self.owner)), + ); Ok(quote! { #[allow(dead_code, non_camel_case_types, missing_docs)] @@ -1172,7 +1200,7 @@ Finally, call `.build()` to create the instance of `{name}`. let #field_name = (#arg_expr,); let ( #(#descructuring,)* ) = self.fields; #builder_name { - #forward_fields + #(#forward_fields,)* fields: ( #(#reconstructing,)* ), _phantom: self._phantom, } @@ -1379,8 +1407,11 @@ Finally, call `.build()` to create the instance of `{name}`. // reordering based on that, but for now this much simpler thing is a reasonable approach. let assignments = self.fields.iter().map(|field| { let name = &field.name; - if !field.builder_attr.extends.is_empty() { - quote!(let #name = #helper_trait_name::into_value(#name, || ::core::default::Default::default());) + if let Some(extends_vec) = field.extends_vec_ident() { + quote!{ + let mut #name = #helper_trait_name::into_value(#name, || ::core::default::Default::default()); + #name.extend(self.#extends_vec); + } } else if let Some(ref default) = field.builder_attr.default { // If field has `into`, apply it to the default value. // Ignore any blank defaults as it causes type inference errors. diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index a3fb84ad19..bc50d9bc9c 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -1222,11 +1222,6 @@ where /// A trait for anything that has a dynamic list of attributes pub trait HasAttributes { - /// The output type after the attribute has been added. This - /// can be used by the properties macro to prevent duplicate - /// attributes - type Output; - /// Push an attribute onto the list of attributes fn push_attribute( self, @@ -1234,5 +1229,5 @@ pub trait HasAttributes { ns: Option<&'static str>, attr: impl IntoAttributeValue, volatile: bool, - ) -> Self::Output; + ) -> Self; } diff --git a/packages/document/src/elements/link.rs b/packages/document/src/elements/link.rs index 215bd8a8a4..cbfa3dcfa7 100644 --- a/packages/document/src/elements/link.rs +++ b/packages/document/src/elements/link.rs @@ -2,6 +2,14 @@ use super::*; use crate::document; use dioxus_html as dioxus_elements; +#[non_exhaustive] +#[derive(Clone, Props, PartialEq)] +pub struct OtherLinkProps { + pub rel: String, + #[props(extends = link, extends = GlobalAttributes)] + pub additional_attributes: Vec, +} + #[non_exhaustive] #[derive(Clone, Props, PartialEq)] pub struct LinkProps { diff --git a/packages/html-internal-macro/src/lib.rs b/packages/html-internal-macro/src/lib.rs index 73a1254f60..88d857d8af 100644 --- a/packages/html-internal-macro/src/lib.rs +++ b/packages/html-internal-macro/src/lib.rs @@ -43,7 +43,7 @@ impl ToTokens for ImplExtensionAttributes { let impls = self.attrs.iter().map(|ident| { let d = quote! { #name::#ident }; quote! { - fn #ident(self, value: impl IntoAttributeValue) -> Self::Output { + fn #ident(self, value: impl IntoAttributeValue) -> Self { let d = #d; self.push_attribute(d.0, d.1, value, d.2) } diff --git a/packages/core-macro/tests/forward_spreads.rs b/packages/ssr/tests/forward_spreads.rs similarity index 51% rename from packages/core-macro/tests/forward_spreads.rs rename to packages/ssr/tests/forward_spreads.rs index 227ca30d8e..0cf03845c1 100644 --- a/packages/core-macro/tests/forward_spreads.rs +++ b/packages/ssr/tests/forward_spreads.rs @@ -1,6 +1,4 @@ use dioxus::prelude::*; -use dioxus_core::ElementId; -use std::{any::Any, rc::Rc}; // Regression test for https://github.com/DioxusLabs/dioxus/issues/3844 #[test] @@ -15,9 +13,13 @@ fn forward_spreads() { fn Comp1(props: Comp1Props) -> Element { rsx! { Comp2 { - attributes: props.attributes, + attributes: props.attributes.clone(), height: "100%", } + Comp2 { + height: "100%", + attributes: props.attributes.clone(), + } } } @@ -29,12 +31,25 @@ fn forward_spreads() { #[component] fn Comp2(props: CompProps2) -> Element { - rsx! {} + let attributes = props.attributes; + rsx! { + div { + ..attributes + } + } } - rsx! { - Comp1 { - width: "100%" + let merged = || { + rsx! { + Comp1 { + width: "100%" + } } }; + let dom = VirtualDom::prebuilt(merged); + let html = dioxus_ssr::render(&dom); + assert_eq!( + html, + r#"
"# + ); }