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

I've been writing a Protobuf plugin for TypeScript #250

Closed
aikoven opened this issue Mar 25, 2021 · 10 comments
Closed

I've been writing a Protobuf plugin for TypeScript #250

aikoven opened this issue Mar 25, 2021 · 10 comments

Comments

@aikoven
Copy link
Collaborator

aikoven commented Mar 25, 2021

I was in the middle of writing by own Protobuf plugin for TypeScript, when I discovered ts-proto, and I find it so awesome, that I'm considering abandoning my project, and instead start contributing to this one.

I'm not sure why I didn't discover it earlier, and I'm surprised how many similar design decisions we have, including:

  • Using just plain object and functions, with no classes, prototypes, getters/setters etc.
  • Making all fields required in generated types, and having a function to initialize objects from partial or deep partial.
  • Keeping the original file/folder structure of input .proto files, instead of outputting a single file, or mimicking protobuf package structure.

I'd like to discuss the differences between our approaches. Some of them are minor, but some are blockers for us to adopt ts-proto, which I'm looking forward to contribute:

  • Nested messages produce nested types, e.g.:

    message Parent {
      message Child {}
    }
    export type Parent = {}
    
    export namespace Parent {
      export function serialize(value: Parent) { /* ... */ }
    
      // ... other functions
    
      export type Child = {}
    
      export namespace Child {
        export function serialize(value: Child) { /* ... */ }
    
        // ... other functions
      }
    }

    This allows to reference nested type as Parent.Child, and its functions as Parent.Child.serialize(...).

  • Enums produce string unions:

    enum Test {
      FIRST = 0,
      SECOND = 1,
    }
    export type Test = 'FIRST' | 'SECOND';
    
    export namespace Test {
      export function toNumber(value: Test): number;
      export function fromNumber(value: number): Test;
    }

    TypeScript string enums are also fine, however, with function fromNumber, we could automatically convert unknown numbers to 0, according to Protobuf rules. Still, with TypeScript enums, we can do it manually using

    Test[num] ?? Test[0]
  • Oneof fields appear at top level:

    message Test {
      string foo = 1;
      oneof test {
        string bar = 2;
        string baz = 3;
      }
    }
    export type Test = {
      foo: string;
    } & Test.TestOneof;
    
    export namespace Test {
      export type TestOneof = 
        | {test: null}
        | {test: 'bar', bar: string}
        | {test: 'baz', baz: string}
    }

    This allows to keep the original structure of fields, and also avoid large changes to the code, when existing field is moved into a oneof. It still works as discriminated union:

    const test: Test;
    
    if (test.test === 'bar') {
      test.bar; // string
    }

    This approach has a downside: it's not easy to assign a different oneof case of existing object, while keeping TypeScript happy. We'd have to treat objects as immutable, and use object spread to alter oneof case:

    const nextTest = Test.fromPartial({
      ...test,
      baz: 'new-value'
    })

    So in the end, I think that your approach to oneofs is more developer-friendly.

  • Getting fully-qualified type names given a message object at runtime. This is needed for implementing helpers for google.protobuf.Any type (see below). We've solved it by adding @type field to generated types:

    package mycompany.myservice;
    
    message Test {
      string foo = 1;
    }
    export type Test = {
      '@type': 'mycompany.myservice.Test';
      foo: string;
    }
  • Resolving messages given fully-qualified type name at runtime. This is also needed for implementing helpers for google.protobuf.Any type (see below). This could be solved using a runtime type registry library, e.g.:

    import {typeRegistry} from 'ts-proto-type-registry';
    
    export type Test = {
      '@type': 'mycompany.myservice.Test';
    }
    
    export const Test = {
      deserialize() {},
      // ... other functions
    }
    
    typeRegistry.register('mycompany.myservice.Test', Test);

    and later using it like:

    typeRegistry.resolve('mycompany.myservice.Test').deserialize(...);

    There's an option to codegen type registry as well, instead of having it as a separate library, and put it into the root of out dir. This would allow to support different shapes of registered objects, since the shape depends on plugin options.

  • Helpers for google.protobuf.Any. This is critical for us. We need a way to have pack and unpack helpers.

  • google.protobuf.Timestamp produce the same code as any other message, but with extra helpers toDate/fromDate:

    type Timestamp = {
      seconds: string;
      nanos: string;
    }
    
    const Timestamp = {
      toDate(value: Timestamp): Date {},
      fromDate(value: Date): Timestamp {},
      // ... other functions
    }

    Having implicit conversion to/from Date is fine for most cases, but breaks for cases where sub-millisecond precision is required.

  • Generic service definitions. I wrote a library for gRPC (nice-grpc), and it currently consumes service definitions generated for grpc-js. I'm currently writing another library for gRPC-Web, which requires service definitions in a different format. My idea was to generate only generic definitions, without generating code for server/client stubs. These stubs can then be generated at type level and at runtime. I'd also like to have service options accessible at runtime, e.g. idempotency_level could be used for automatic retries.

    package mycompany.myservice;
    
    message Request { /* ... */ }
    message Response { /* ... */ }
    
    service Test {
      rpc Foo(Request) returns (Response) {
        option idempotency_level = IDEMPOTENT;
      }
    }
    type Request = { /* ... */ }
    const Request = { /* ... */ }
    
    type Response = { /* ... */ }
    const Response = { /* ... */ }
    
    const Test = {
      name: 'mycompany.myservice.Test',
      methods: {
        foo: {
          name: 'Foo',
          request: Request,
          requestStream: false,
          request: Response,
          requestStream: false,
          options: {
            idempotencyLevel: 'IDEMPOTENT',
          }
        },
      },
    } as const;

    This can later be transformed at type level to server/client type like:

    type Client<ServiceDefinition> = {
      [Method in keyof ServiceDefinition['methods']]: (
        request: InferRequest<ServiceDefinition['methods'][Method]>,
        options?: CallOptions
      ) => Promise<InferResponse<ServiceDefinition['methods'][Method]>>
    }
    
    function createClient<ServiceDefinition>(definition: ServiceDefinition): Client<ServiceDefinition>;
    
    const testClient: Client<typeof Test> = createClient(Test);

I'd like to hear your thoughts on this. The most critical parts for us are google.protobuf.Any helpers and service definitions. If that sounds right to you, I can create separate proposal issues and later work on PRs once we settle with solutions.

Btw, your ts-poet library is neat! Nice work.

@stephenh
Copy link
Owner

Hey! Thanks for reaching out!

Yep, would be great to have you collaborate on ts-proto. A lot of what you want makes sense, and especially Anys is just features I personally haven't used, so didn't have the itch / 100% sure best way to design/handle things. Same with grpc, still kinda a newb on that stuff. So would be great to have those flushed out by real-world use cases & usage.

All of your asks/suggestions makes sense, and I agree splitting them out into per-issue/per-PR proposals makes sense.

Just glancing, I think the only thing that sticks out is that I originally had namespace-using features in ts-proto but then took them out b/c babel doesn't like it, but that said we could look into (why not!) having yet another config flag :-) to drive things like the nested vs. prefixed inner types or what not.

I'm in a rush, but will do a 1st-pass-thoughts on each of ^ later today/tonight.

@stephenh
Copy link
Owner

Initial thoughts:

  • Nested messages produce nested types -- per previous response, yep makes sense to have a config flag, b/c I agree namespaced is better, just that wrinkle with babel (which afaiu is mostly for browser support) means it should be opt-out-able
  • Enums are unions -- good idea!
  • @type -- makes sense, others have asked for similar, dunno why I've been hesitant
  • better Any support -- yep, would be great
  • Timestamp -- not against the toDate/fromDate although curious if the ts-proto default behavior of "just use Date directly" would be fine/preferable? I know some users prefer Timestamp or need it for things like NestJS support so there's an option for it. I.e. maybe you need to keep Timestamp for grpc-js support.
  • grpc-js / nice-grpc support -- sounds great. Fwiw we have grpc-web support already via improbable-eng, which needed its own service definition format, so sounds pretty similar...out of curiosity, is that what you need to support or something else? Either way, the more lib support the better imo.
    • Using mapped types + proxies for the actual stub impls (on top of generic service metadata) doesn't sound like a bad idea, although I worry a little bit that each framework is going to have nitty-gritty little details that would complicate the mapped types vs. "meh just code-gen it". But it would enable things like supporting multiple frameworks from the same generated code, which would be cool, although in general I assume most people are picking a single library and staying pretty coupled to it. I.e. server/client typically have to agree anyway/etc so you kinda get locked in.

As a summary, I'm not -1 on anything you want to contribute, so feel free to start some PRs. :-)

@aikoven
Copy link
Collaborator Author

aikoven commented Mar 26, 2021

curious if the ts-proto default behavior of "just use Date directly" would be fine/preferable?

Yep that's fine for 99% cases, though I can imagine cases when we need to preserve micro and nanoseconds. Unfortunately, I don't know of any JS lib that allows to work with date/time with sub-millisecond precision. Anyways, this is minor for now.

Fwiw we have grpc-web support already via improbable-eng, which needed its own service definition format, so sounds pretty similar...out of curiosity, is that what you need to support or something else?

Ideally, I'd like to have the same codegen that would work with both nice-grpc and future nice-grpc-web.

I worry a little bit that each framework is going to have nitty-gritty little details that would complicate the mapped types vs. "meh just code-gen it"

Yep I feel you, though currently to build a new framework, you have to first do the codegen stuff. With generic service definitions, you can start right away. You can also make a breaking change to the framework's server/client signatures without changing the code generator. That said, I agree that having the complete server/client types generated is good for users, because they can see the signatures of all methods in generated code; while with mapped types, they need an IDE to see final types.

So my proposal is to have an option to output generic definitions, and then (sometime in the future) another option to also output server/client types for nice-grpc.

@stephenh
Copy link
Owner

when we need to preserve micro and nanoseconds

Ah sure.

So my proposal is to have an option to output generic definitions

Cool, I'd down to try it and see how it goes. Would be great to make it easier to onboard new frameworks, as you said, without bespoke codegen for each.

@aikoven
Copy link
Collaborator Author

aikoven commented Mar 29, 2021

I've decided to first add support for grpc-js, since my lib already works with their service definition format. This unlocks using ts-proto for myself, and would also help others, who want to use grpc-js (#245).

PR: #252

@aikoven
Copy link
Collaborator Author

aikoven commented Mar 31, 2021

PR for type registry and $type field: #254

Decided to go with $type and not @type, since the dollar sign is already used for special field $case.

@eknowles
Copy link

This thread makes me happy, thank you @stephenh and @aikoven for your work!

@aikoven
Copy link
Collaborator Author

aikoven commented Jun 9, 2021

PR for generic service definitions: #316

@stephenh
Copy link
Owner

@aikoven I was just going through issues today; do you think this one can be closed out?

@aikoven
Copy link
Collaborator Author

aikoven commented Nov 27, 2021

Yeah, let's close it, thanks for discussion!

@aikoven aikoven closed this as completed Nov 27, 2021
zfy0701 added a commit to sentioxyz/ts-proto that referenced this issue Jan 5, 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

No branches or pull requests

3 participants