Closed awesomelemonade closed 1 year ago
Maybe it's the real issue of https://github.com/idanarye/rust-typed-builder/issues/101?
This makes no sense. #101 was using a reexported TypedBuilder
which messed up the crate paths, but this does not seem to be the case with Leptos. This is the only place I found where it uses TypedBuilder
: https://github.com/leptos-rs/leptos/blob/3a98bdb3c2aee80db02bcb606e1e2bbf84cf080c/leptos_config/src/lib.rs#L26
It uses TypedBuilder
directly from this crate - not reexported from elsewhere. So the code gets generated inside leptos_config/src/lib.rs
, which has access to the typed builder crate. Shouldn't any crate that uses it just see a LeptosOptions
type, that yes - has some (related) impl
s that use ::typed_builder::Optional
, but how is that any different than other Rust crates that use stuff from other crates without reexporting them or asking their users to add them?
What am I missing here?
Reverting "Add Optional" does fix leptos. See https://github.com/leptos-rs/leptos/pull/1496#issuecomment-1671822322. We could merge this patch in and re-unify Optional at a later time. Not sure what you think @idanarye
I don't want to revert b9ca28737dd9309ea294df828583982f17a543d8. The real source is #31 - Optional
is just the only thing that uses it right now, but the whole split is supposed to open the path for other features, like #95, which will also break Leptos in the exact same way once merged.
Also note that if someone else creates a macro that calls the Leptos macro, Leptos will have the exact same problem. Of course, it is much less likely that someone will do that with a web framework than with a coding sugar utility, but I never considered someone may use Typed Builder like this - so things may happen.
May I suggest a different approach? What if the TypedBuilder
derive would accept the path to the typed_builder
module? Usually it'd just be ::typed_builder
, but:
#[derive(::leptos::typed_builder::TypedBuilder)]
#[builder(doc, crate_module_path=::leptos::typed_builder)]
#vis struct #props_name #impl_generics #where_clause {
Yes, it's more work, but only the crates that create macros will need to do that work, and these crates should alredy be prepared to do more work. Plus, if Leptos ever finds itself in the same boat, it'd add its own crate_module_path
parameter (which will default to ::leptos
) and do:
#[derive(#crate_module_path::typed_builder::TypedBuilder)]
#[builder(doc, crate_module_path=#crate_module_path::typed_builder)]
#vis struct #props_name #impl_generics #where_clause {
So this solution is transitive.
Yup your approach seems reasonable to me. serde and strum seem to do the same. Reverting was just a proof of concept/quickfix and obviously did not seem like a long term solution, which is why I never created a PR. I will likely be using my patch for now, but when I get some more free time (and it hasn't been fixed by then), I will make a PR adding crate_module_path similar to this
See https://github.com/leptos-rs/leptos/pull/1496. If Leptos updates to 0.15, it forces all leptos apps to add typed-builder to its dependencies in the Cargo.toml which is not ideal.
The easiest way to fix this is to revert this commit: https://github.com/idanarye/rust-typed-builder/commit/b9ca28737dd9309ea294df828583982f17a543d8, which would mean each type would now have its own Optional again. This would yield more generated code.
Had a note here, but I guess it was forgotten about: https://github.com/idanarye/rust-typed-builder/commit/b9ca28737dd9309ea294df828583982f17a543d8#diff-f4519feb64908d4106578124d2bb7d0fbc21b38c3ae011d89aa95b2320feb7a8L170
Ideally, this should be resolved with https://github.com/rust-lang/rust/issues/54363. There are potential workarounds here: https://github.com/frozenlib/parse-display/issues/28, but that may require more effort to investigate and implement.