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

Enum Codecs #208

Closed
josepot opened this issue Nov 28, 2023 · 3 comments
Closed

Enum Codecs #208

josepot opened this issue Nov 28, 2023 · 3 comments

Comments

@josepot
Copy link
Member

josepot commented Nov 28, 2023

As I've been working more extensively with the top-level client, I've encountered a fundamental limitation in the current typing system, especially evident in the use of Enum higher-order codecs of scale-ts. This experience has led me to believe that the assumption that a Codec should have identical types for both its encoder and decoder is not always ideal.

Current Challenge:

The decoder's functionality, particularly its ability to return discriminated unions, is exceptional and provides a great developer experience. However, the requirement to use the same object structures for encoding that are returned by the decoder is not only inconvenient but also counterintuitive in many scenarios.

Current Codec Typing:

type Encoder<T> = (value: T) => Uint8Array
type Decoder<T> = (value: Uint8Array | ArrayBuffer | string) => T

type Codec<T> = [Encoder<T>, Decoder<T>] & {
  enc: Encoder<T>
  dec: Decoder<T>
}

In this structure, both encoding and decoding operations are expected to work with the same type T, which limits flexibility.

Proposed Typing Enhancement:

type Encoder<E> = (value: E) => Uint8Array
type Decoder<D> = (value: Uint8Array | ArrayBuffer | string) => D

type Codec<E, D = E> = [Encoder<E>, Decoder<D>] & {
  enc: Encoder<E>
  dec: Decoder<D>
}

This modification introduces separate generic types for encoders (E) and decoders (D). This change would allow for more nuanced and ergonomic handling of different structures for encoding and decoding operations.

Practical Example:

Consider the following Enum definition:

const event = Enum({
    _void,
    one: str,
    many: Vector(str),
    allOrNothing: bool,
})

With the proposed change, the inferred types could be:

type Event = Codec<
  | { _void: null }
  | { one: string }
  | { many: string[] }
  | { allOrNothing: boolean },

  | { tag: '_void' }
  | { tag: 'one'; value: string }
  | { tag: 'many'; value: string[] }
  | { tag: 'allOrNothing'; value: boolean }
>

This approach allows us to encode an event more naturally, like event.enc({ one: 'thing' }), instead of the current less ergonomic method event.enc({ tag: 'one', value: 'thing' }).

Conclusion:

I think that this breaking change would significantly enhance the developer experience by providing more flexibility and intuitiveness in handling different data structures for encoding and decoding. This change seems particularly crucial for effective use of Enum types and could potentially benefit other use cases within scale-ts.

I'm wondering if this could have some undesired effects that I can't foresee right now 🤔

@josepot
Copy link
Member Author

josepot commented Nov 28, 2023

I'm having second thoughts on this. I think that what I'm suggesting in here is wrong. The whole point of having a Codec type is that the output of the decoder is the same type as the input of the encoder... If that's not the case, then it's just something else, but not a Codec.

That being said, we could solve this in our end by using enhanced encoders wherever necessary... 🤔

@josepot
Copy link
Member Author

josepot commented Jan 5, 2024

The more I think about this, the more I realize that we should make this breaking change ASAP. I keep finding more and more cases where the encoder and the decoder should have different types... We should tackle this before it's too late, the longer we wait the more difficult it's going to be to make this change in the future.

@josepot
Copy link
Member Author

josepot commented Jan 6, 2024

Ok, so after spiking this task and I ended up realizing that this is a really bad idea.

The output of the decoder should always match with the input of the encoder.

That being said, I have come up with a very nice solution to the problem that I described above. However, I will be explaining that solution in a separate issue.

@josepot josepot closed this as completed Jan 6, 2024
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

1 participant