Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Close #109: Add crate_module_path #111

Merged
merged 1 commit into from
Aug 26, 2023

Conversation

idanarye
Copy link
Owner

No description provided.

@idanarye
Copy link
Owner Author

@awesomelemonade Before I merge, please check if this works for your usecase.

@gbj
Copy link

gbj commented Aug 24, 2023

I'm not @awesomelemonade but am maintainer of the Leptos framework that prompted the discussion, and yes, this should work perfectly, as it would allow us to specify the re-exported path to as ::leptos::typed_builder rather than ::typed_builder, so users don't have to add typed_builder to their own Cargo.toml.

Thanks for all your work on this great library.

@gbj
Copy link

gbj commented Aug 25, 2023

It looks like there's an additional direct ::typed_builder reference that might be missed in the PR, in the generics on the impl block.

For example, the following

#[derive(::leptos::typed_builder_macro::TypedBuilder)]
#[builder(crate_module_path = typed_builder)]
struct InfoGridProps<const NUM_ROWS: usize, const NUM_COLS: usize> {
    data: [[u32; NUM_ROWS]; NUM_COLS],
    #[builder(default)]
    optional_prop: bool,
}

expands to code including the following

#[allow(dead_code, non_camel_case_types, missing_docs)]
impl<
    const NUM_ROWS: usize,
    const NUM_COLS: usize,
    // ❌ crate_module_path not used here
    __optional_prop: ::typed_builder::Optional<bool>,
> InfoGridPropsBuilder<
    NUM_ROWS,
    NUM_COLS,
    (([[u32; NUM_ROWS]; NUM_COLS],), __optional_prop),
> {
    #[allow(clippy::default_trait_access)]
    pub fn build(self) -> InfoGridProps<NUM_ROWS, NUM_COLS> {
        let (data, optional_prop) = self.fields;
        let data = data.0;
        // crate_module_path is used here
        let optional_prop = ::leptos::typed_builder::Optional::into_value(
            optional_prop,
            || ::core::default::Default::default(),
        );
        #[allow(deprecated)]
        InfoGridProps {
            data,
            optional_prop,
        }
            .into()
    }
}

@idanarye
Copy link
Owner Author

Yes, this is the problem:

let trait_ref = syn::TraitBound {
paren_token: None,
lifetimes: None,
modifier: syn::TraitBoundModifier::None,
path: syn::Path {
leading_colon: Some(syn::token::PathSep::default()),
segments: [
syn::PathSegment {
ident: Ident::new("typed_builder", Span::call_site()),
arguments: syn::PathArguments::None,
},
syn::PathSegment {
ident: Ident::new("Optional", Span::call_site()),
arguments: syn::PathArguments::AngleBracketed(syn::AngleBracketedGenericArguments {
colon2_token: None,
lt_token: Default::default(),
args: [syn::GenericArgument::Type(field.ty.clone())].into_iter().collect(),
gt_token: Default::default(),
}),
},
]
.into_iter()
.collect(),
},
};

I think I missed it because, for performance reasons, it's not written as ::typed_builder::Optional.

Not 100% sure if I should change it to a syn::parse_quote!...

@idanarye
Copy link
Owner Author

@gbj I pushed a fix. Please try again.

@gbj
Copy link

gbj commented Aug 26, 2023

It works! Thank you so much!

@idanarye idanarye force-pushed the feature/issue-109-setting-crate_module_path branch from e15bd2e to 3f78ca5 Compare August 26, 2023 02:21
@idanarye idanarye merged commit a64cfc5 into master Aug 26, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants