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

Consider [de]serialization without explicit pbjson-types #75

Open
rnarubin opened this issue Sep 30, 2022 · 1 comment
Open

Consider [de]serialization without explicit pbjson-types #75

rnarubin opened this issue Sep 30, 2022 · 1 comment

Comments

@rnarubin
Copy link
Contributor

Thanks for your work on these crates, they're super useful.

I have a design question that I'm wondering if you all have given any thought: could the serialization/deseralization code-gen be written in such a way to convert the well-known types in prost-types directly, without substituting pbjson-types?

For example, the code gen could detect whether a field has a known type, and then invoke a free-standing [de]serialize function to convert a prost-type, instead of using the trait-based type serialization. This would be similar to the #[serde(with = ...)] attribute that substitutes in functions, applied within the generated trait code

The motivation is to have a single "canonical" source for the well-known types, to make things like From impls simpler for user types. For example, suppose I have an internal timestamp type, with impl From<MyTimestamp> for prost_types::Timestamp. Adding another impl for pbjson::Timestamp is easy enough, but it's more code and exposes the json support in the types rather than only an implementation detail.

I'm wondering if this approach is possible, and if it's something you'd consider. It would be behind some flag on the builder of course, to give users the option to opt-in or to continue with the current type-based approach

@tustvold
Copy link
Contributor

tustvold commented Oct 4, 2022

Apologies for the slow reply, I think this is possible, although likely rather fiddly to implement, and I would be happy to review a PR that does this.

Perhaps you might like to try it out for a simple type like Duration, or Timestamp and file a draft PR and we can go from there? The key part will be ensuring all the various different ways that such a type can appear, in a map, repeated field, oneof, etc... are correctly handled

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

No branches or pull requests

2 participants