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

Add support for byte translation #230

Open
wants to merge 26 commits into
base: main
Choose a base branch
from

Conversation

MOmarMiraj
Copy link
Contributor

@MOmarMiraj MOmarMiraj commented Feb 18, 2025

This PR adds support for translating Vec<u8> to the appropriate byte translation for Go, Python, and Typescript.

To maintain backwards compatibility, decided to opt against adding a Bytes in SpecialRustType as there could be some devs translating Vec<u8> into a non byte translation and will be a breaking change.

There was two options I was considering when implementing this feature:

  1. Adding it as a decorator so the user could do something like this:
#[typeshare]
#[serde(rename_all = "camelCase")]
pub struct Foo {
    #[typeshare(go,python,typescript(binary_data))]
    pub bytes: Vec<u8>,
}

This option would allow us to use the existing decorators functionality to parse each specific field to determine whether or not we can translate it to its appropriate byte translation.

The issue with this implementation is it would cause a pretty big refactor as the format_special_type function does not have access to decorators and allowing the function to have access whether passing it as an argument or attaching it to each language struct as a field would be overly complex for achieving not the greatest result.

The second option is what this PR does, which is take adavantage of the type_mappings field which allows us to use the typeshare.toml to translate the Vec<u8> to its appropriate translation in each target language. This way is the least complex and is backwards compatible as the user would have to specify inside the typeshare.toml if they want the byte translation otherwise it would keep the current implementation intact.

I added some tests in Go, Python, and Typescript to test this out.

With typescript, as there is no native support for Uint8Array for JSON serialization/deserialization, we have to add these reviver and replacer functions to not lose context of the Uint8Array during serialization/deserialization and the same thing goes for the Python implementation.

@MOmarMiraj MOmarMiraj requested a review from hculea February 18, 2025 20:15
@MOmarMiraj MOmarMiraj self-assigned this Feb 18, 2025
@MOmarMiraj MOmarMiraj marked this pull request as ready for review February 19, 2025 16:06
Comment on lines +207 to +211
SpecialRustType::Vec(rtype) => {
// TODO: https://github.com/1Password/typeshare/issues/231
self.add_import("typing".to_string(), "List".to_string());
Ok(format!("List[{}]", self.format_type(rtype, generic_types)?))
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This diff is now no longer needed right? It can have the same translation as Array and Slice

Comment on lines +94 to +96
/// Whether or not to include the serialization/deserialzation functions for bytes.
/// This by default should be false as unless the user expclitly wants to translate to its bytes
/// representation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is no longer accurate

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