idanarye / rust-typed-builder

Compile-time type-checked builder derive
https://crates.io/crates/typed-builder
Apache License 2.0
904 stars 52 forks source link

Close #109: Add `crate_module_path` #111

Closed idanarye closed 1 year ago

idanarye commented 1 year ago

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

gbj commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Yes, this is the problem:

https://github.com/idanarye/rust-typed-builder/blob/628786a5959ef7747cbb9f5423df80ebc9b0365f/typed-builder-macro/src/struct_info.rs#L404-L428

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 commented 1 year ago

@gbj I pushed a fix. Please try again.

gbj commented 1 year ago

It works! Thank you so much!