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

Support re-export of derive_builder #274

Closed
ryo33 opened this issue Nov 18, 2022 · 6 comments · Fixed by #275
Closed

Support re-export of derive_builder #274

ryo33 opened this issue Nov 18, 2022 · 6 comments · Fixed by #275

Comments

@ryo33
Copy link
Contributor

ryo33 commented Nov 18, 2022

Background

parol generates code that has several external crates, including derive_builder. And, parol_runtime re-exports the crates for ease of use.

Problem

I know derive_builder doesn't support crate renaming, but it also seems not to support re-exporting.
So we need a user-confusing workaround in lib.rs or main.rs as below:

use parol_runtime::derive_builder;

Solution

Replace `::derive_builder" with "derive_builder" in generated code.

It may break projects in Rust 2015 edition, and we may need a feature toggle for this behavior.

@TedDriggs
Copy link
Collaborator

I think I'm reading that parol is generating structs that are then trying to derive a builder, without requiring the user of parol to directly depend on derive_builder; does that accurately describe the issue?

@ryo33
Copy link
Contributor Author

ryo33 commented Nov 22, 2022

Yes, it does.

@TedDriggs
Copy link
Collaborator

It seems that the common solution for this is to expose a crate attribute, e.g.

#[derive(Builder)]
#[builder(crate = "parol_runtime::derive_builder")]

That's how serde does it, and that seems like that should not have any backwards compatibility risks. Would parol_runtime be able to use such a solution?

@ryo33
Copy link
Contributor Author

ryo33 commented Nov 22, 2022

Yeah, it seems nice.

@TedDriggs
Copy link
Collaborator

Okay, that seems doable though some help on tests will be appreciated.

TedDriggs added a commit that referenced this issue Nov 22, 2022
This allows setting the crate root for re-export scenarios.

All hardcoded usage of `::derive_builder` in generated code has now
been replaced by a `crate_root` variable, which contains the path to use.

For backwards compatibility, this defaults to `::derive_builder`.

Fixes #274
TedDriggs added a commit that referenced this issue Nov 22, 2022
This allows setting the crate root for re-export scenarios.

All hardcoded usage of `::derive_builder` in generated code has now
been replaced by a `crate_root` variable, which contains the path to use.

For backwards compatibility, this defaults to `::derive_builder`.

Fixes #274
TedDriggs added a commit that referenced this issue Nov 22, 2022
This allows setting the crate root for re-export scenarios.

All hardcoded usage of `::derive_builder` in generated code has now
been replaced by a `crate_root` variable, which contains the path to use.

For backwards compatibility, this defaults to `::derive_builder`.

Fixes #274
TedDriggs added a commit that referenced this issue Nov 28, 2022
This allows setting the crate root for re-export scenarios.

All hardcoded usage of `::derive_builder` in generated code has now
been replaced by a `crate_root` variable, which contains the path to use.

For backwards compatibility, this defaults to `::derive_builder`.

Fixes #274
@TedDriggs
Copy link
Collaborator

This is published as part of 0.12.0

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 a pull request may close this issue.

2 participants