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

feat: typed deserialization #75

Closed
wants to merge 2 commits into from
Closed

Conversation

gagdiez
Copy link
Contributor

@gagdiez gagdiez commented Nov 21, 2023

Currently the outter deserialization returns a DecodeType that we expect users to cast to their own type T. This change modifies the interface from deserialize(...): DecodeType to an automatic casting deserialize<T>(...): T.

For JS users this makes no difference, since there are no types involved anyway.

For TS users this is a breaking change, since deserialize will now return an unknown type instead of a DecodeType by default if no T is given. However, we would expect users to always be want to cast to T.

Note: This does not change the internal logic of the library, which keeps using DecodeType to maintain consistency.

Before:

const decoded_A = deserialize(...)                     // decoded type is borsh.DecodedType
const decoded_B = <MyClass> deserialize(...) // decoded type is MyClass

Now

const decoded_A =  deserialize(...)                    // decoded type is unknown
const decoded_B =  deserialize<MyClass>(...) // decoded type is MyClass

Currently the outter deserialization returns a `DecodeType` that we
expect users to cast to their own type `T`. This change modifies the
interface from `deserialize(...): DecodeType` to an automatic casting
`deserialize<T>(...): T`.

For JS users this makes no difference, since there are no types involved
anyway.

For TS users this is a **breaking** change, since `deserialize` will now
return an `unknown` type instead of a `DecodeType` by default if no `T`
is given. However, we would expect users to always be want to cast to `T`.
@gagdiez gagdiez requested a review from ailisp as a code owner November 21, 2023 12:03
@gagdiez gagdiez marked this pull request as draft November 23, 2023 14:39
@gagdiez gagdiez closed this Nov 23, 2023
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.

1 participant