Skip to content

Commit

Permalink
Redo derive(Properties), take 2 (#2729)
Browse files Browse the repository at this point in the history 
* remove props checking, but take builder by mut ref

* allow property name `build`

* add rough state token machinery

* first working impl, readd props checking

* improve error message

* add documentation and last adjustements

* address review
  • Loading branch information
@WorldSEnder
WorldSEnder committed Jun 21, 2022
1 parent 015412e commit 75bb903
Show file tree
Hide file tree
Showing 13 changed files with 531 additions and 438 deletions.
205 changes: 83 additions & 122 deletions packages/yew-macro/src/derive_props/builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,86 +7,44 @@

use proc_macro2::{Ident, Span};
use quote::{format_ident, quote, ToTokens};
use syn::Attribute;
use syn::{parse_quote_spanned, Attribute, GenericParam};

use super::generics::{to_arguments, with_param_bounds, GenericArguments};
use super::{DerivePropsInput, PropField};
use super::generics::to_arguments;
use super::DerivePropsInput;
use crate::derive_props::generics::push_type_param;

pub struct PropsBuilder<'a> {
builder_name: &'a Ident,
step_trait: &'a Ident,
step_names: Vec<Ident>,
props: &'a DerivePropsInput,
wrapper_name: &'a Ident,
check_all_props_name: &'a Ident,
extra_attrs: &'a [Attribute],
}

impl ToTokens for PropsBuilder<'_> {
fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) {
let Self {
builder_name,
step_trait,
step_names,
props,
wrapper_name,
..
} = self;

let DerivePropsInput {
vis,
generics,
props_name,
..
} = props;
let DerivePropsInput { vis, generics, .. } = props;

let build_step = self.build_step();
let impl_steps = self.impl_steps();
let set_fields = self.set_fields();
let assert_all_props = self.impl_assert_props();

let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();
let turbofish_generics = ty_generics.as_turbofish();
let generic_args = to_arguments(generics, build_step.clone());

// Each builder step implements the `BuilderStep` trait and `step_generics` is used to
// enforce that.
let step_generic_param = Ident::new("YEW_PROPS_BUILDER_STEP", Span::mixed_site());
let step_generics =
with_param_bounds(generics, step_generic_param.clone(), (*step_trait).clone());
let (_, ty_generics, where_clause) = generics.split_for_impl();

let builder = quote! {
#(
#[doc(hidden)]
#[allow(non_camel_case_types)]
#vis struct #step_names;
)*

#[doc(hidden)]
#vis trait #step_trait {}

#(
#[automatically_derived]
impl #step_trait for #step_names {}
)*

#[doc(hidden)]
#vis struct #builder_name #step_generics
#vis struct #builder_name #generics
#where_clause
{
wrapped: ::std::boxed::Box<#wrapper_name #ty_generics>,
_marker: ::std::marker::PhantomData<#step_generic_param>,
}

#impl_steps

#[automatically_derived]
impl #impl_generics #builder_name<#generic_args> #where_clause {
#[doc(hidden)]
#vis fn build(self) -> #props_name #ty_generics {
#props_name #turbofish_generics {
#(#set_fields)*
}
}
}
#assert_all_props
};

tokens.extend(builder);
Expand All @@ -96,112 +54,115 @@ impl ToTokens for PropsBuilder<'_> {
impl<'a> PropsBuilder<'_> {
pub fn new(
name: &'a Ident,
step_trait: &'a Ident,
props: &'a DerivePropsInput,
wrapper_name: &'a Ident,
check_all_props_name: &'a Ident,
extra_attrs: &'a [Attribute],
) -> PropsBuilder<'a> {
PropsBuilder {
builder_name: name,
step_trait,
step_names: Self::build_step_names(step_trait, &props.prop_fields),
props,
wrapper_name,
check_all_props_name,
extra_attrs,
}
}
}

impl PropsBuilder<'_> {
pub fn first_step_generic_args(&self) -> GenericArguments {
to_arguments(&self.props.generics, self.first_step().clone())
}

fn first_step(&self) -> &Ident {
&self.step_names[0]
}

fn build_step(&self) -> &Ident {
&self.step_names[self.step_names.len() - 1]
}

fn build_step_names(prefix: &Ident, prop_fields: &[PropField]) -> Vec<Ident> {
let mut step_names: Vec<Ident> = prop_fields
.iter()
.filter(|pf| pf.is_required())
.map(|pf| pf.to_step_name(prefix))
.collect();
step_names.push(format_ident!(
"{}PropsBuilder",
prefix,
span = prefix.span(),
));
step_names
}

fn set_fields(&self) -> impl Iterator<Item = impl ToTokens + '_> {
self.props.prop_fields.iter().map(|pf| pf.to_field_setter())
}

fn impl_steps(&self) -> proc_macro2::TokenStream {
fn impl_assert_props(&self) -> proc_macro2::TokenStream {
let Self {
builder_name,
props,
step_names,
check_all_props_name,
extra_attrs,
..
} = self;
let DerivePropsInput {
vis,
generics,
props_name,
prop_fields,
..
} = props;
} = self.props;

let (impl_generics, _, where_clause) = generics.split_for_impl();
let mut fields_index = 0;
let mut token_stream = proc_macro2::TokenStream::new();
let set_fields = self.set_fields();
let prop_fns = prop_fields
.iter()
.map(|pf| pf.to_build_step_fn(vis, props_name));

for (step, step_name) in step_names.iter().enumerate() {
let mut optional_fields = Vec::new();
let mut required_field = None;
let (builder_impl_generics, ty_generics, builder_where_clause) = generics.split_for_impl();
let turbofish_generics = ty_generics.as_turbofish();
let generic_args = to_arguments(generics);

if fields_index >= prop_fields.len() {
break;
let mut assert_impl_generics = generics.clone();
let token_arg: GenericParam = parse_quote_spanned! {Span::mixed_site()=>
__YewToken
};
push_type_param(&mut assert_impl_generics, token_arg.clone());
let assert_impl_generics = assert_impl_generics;
let (impl_generics, _, where_clause) = assert_impl_generics.split_for_impl();

let props_mod_name = format_ident!("_{}", props_name, span = Span::mixed_site());
let mut check_impl_generics = assert_impl_generics.clone();
let mut check_args = vec![];
let mut check_props = proc_macro2::TokenStream::new();
let prop_field_decls = prop_fields
.iter()
.map(|pf| pf.to_field_check(props_name, vis, &token_arg))
.collect::<Vec<_>>();
let prop_name_decls = prop_field_decls.iter().map(|pf| pf.to_fake_prop_decl());
for pf in prop_field_decls.iter() {
check_props.extend(pf.to_stream(
&mut check_impl_generics,
&mut check_args,
&props_mod_name,
));
}
let (check_impl_generics, _, check_where_clause) = check_impl_generics.split_for_impl();

quote! {
#[automatically_derived]
#( #extra_attrs )*
impl #builder_impl_generics #builder_name<#generic_args> #builder_where_clause {
#( #prop_fns )*
}

while let Some(pf) = prop_fields.get(fields_index) {
fields_index += 1;
if pf.is_required() {
required_field = Some(pf);
break;
} else {
optional_fields.push(pf);
}
#[doc(hidden)]
#[allow(non_snake_case)]
#vis mod #props_mod_name {
#( #prop_name_decls )*
}
#check_props

#[doc(hidden)]
#vis struct #check_all_props_name<How>(::std::marker::PhantomData<How>);

// Optional properties keep the builder on the current step
let current_step_arguments = to_arguments(generics, step_name.clone());
let optional_prop_fn = optional_fields
.iter()
.map(|pf| pf.to_build_step_fn(builder_name, &current_step_arguments, vis));

// Required properties will advance the builder to the next step
let required_prop_fn = required_field.iter().map(|pf| {
let next_step_name = &step_names[step + 1];
let next_step_arguments = to_arguments(generics, next_step_name.clone());
pf.to_build_step_fn(builder_name, &next_step_arguments, vis)
});

token_stream.extend(quote! {
#[automatically_derived]
#( #extra_attrs )*
impl #impl_generics #builder_name<#current_step_arguments> #where_clause {
#(#optional_prop_fn)*
#(#required_prop_fn)*
#[automatically_derived]
impl<B, P, How> ::yew::html::HasProp<P, &dyn ::yew::html::HasProp<P, How>>
for #check_all_props_name<B>
where B: ::yew::html::HasProp<P, How> {}

#[automatically_derived]
impl #check_impl_generics ::yew::html::HasAllProps<
#props_name #ty_generics ,
( #( #check_args , )* ),
> for #check_all_props_name< #token_arg > #check_where_clause {
}

#[automatically_derived]
impl #impl_generics ::yew::html::Buildable< #token_arg > for #builder_name<#generic_args> #where_clause {
type Output = #props_name #ty_generics;
type WrappedToken = #check_all_props_name< #token_arg >;
fn build(this: Self) -> Self::Output {
#props_name #turbofish_generics {
#(#set_fields)*
}
}
});
}
}
token_stream
}
}

3 comments on commit 75bb903

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yew master branch benchmarks (Lower is better)

Benchmark suite Current: 75bb903 Previous: 015412e Ratio
yew-struct-keyed 01_run1k 197.445 173.1965 1.14
yew-struct-keyed 02_replace1k 210.9265 185.558 1.14
yew-struct-keyed 03_update10th1k_x16 381.963 433.353 0.88
yew-struct-keyed 04_select1k 67.2205 94.892 0.71
yew-struct-keyed 05_swap1k 95.391 119.07 0.80
yew-struct-keyed 06_remove-one-1k 34.159000000000006 38.1445 0.90
yew-struct-keyed 07_create10k 3472.1585 3255.804 1.07
yew-struct-keyed 08_create1k-after1k_x2 462.481 434.6715 1.06
yew-struct-keyed 09_clear1k_x8 202.455 178.334 1.14
yew-struct-keyed 21_ready-memory 1.4694786071777344 1.4694786071777344 1
yew-struct-keyed 22_run-memory 1.6719017028808594 1.6629371643066406 1.01
yew-struct-keyed 23_update5-memory 1.6990013122558594 1.7175788879394531 0.99
yew-struct-keyed 24_run5-memory 1.717529296875 1.710590362548828 1.00
yew-struct-keyed 25_run-clear-memory 1.328399658203125 1.3284416198730469 1.00
yew-struct-keyed 31_startup-ci 1736.908 1881.3 0.92
yew-struct-keyed 32_startup-bt 34.092 44.44399999999999 0.77
yew-struct-keyed 33_startup-mainthreadcost 256.5160000000001 228.1040000000001 1.12
yew-struct-keyed 34_startup-totalbytes 332.2724609375 332.2763671875 1.00

This comment was automatically generated by workflow using github-action-benchmark.

@maurerdietmar
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit break my code. I use Properties with generics, for example:

#[derive(Properties, Clone)]
pub struct GridPicker<T>
where
    T: 'static,
{
    #[prop_or_default]
    pub items: Rc<Vec<T>>,
}

impl<T> GridPicker<T> {

    pub fn new() -> Self {
        yew::props!(GridPicker<T> {})
    }
}

This results in the following error:

yew::props!(GridPicker<T> {})
   |                                ^ use of generic parameter from outer function

Also, I often use code like:

impl XXX {
  pub fn new() -> Self {
         yew::props!(Self {})
  }
}

This results in a similar error:

yew::props!(Self {})
   |                     ^^^^
   |                     |
   |                     use of generic parameter from outer function
   |                     use a type here instead

@WorldSEnder
Copy link
Member Author

@WorldSEnder WorldSEnder commented on 75bb903 Jun 22, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maurerdietmar Sorry for that, it seems the test cases need expansion, I can reproduce the issue. Expect a follow up soon and you can pin to the commit prior to this in the meanwhile.

Please sign in to comment.