Add arguments to Typescript generated message constructor. about ts-protoc-gen HOT 12 OPEN

In theory we should be able to do better than typing the constructor as any[] by providing named constructor arguments with the corresponding types for each field when generating the .d.ts file.

Personally I would not suggest this construction syntax to consumers owing to concerns around readability and future maintainability of this approach, however this point is rendered somewhat moot by support being provided in the underlying JavaScript.

Update 1
Just reread your sample code and saw that the constructor expects an Array where each index is sequentially assigned to a field. If that's the case then I don't think we can provide any type hints which makes me even less compelled to suggest we support this method of instantiation as it negates the benefits of adding typesaftey.

Update 2
Derp, we can define a Tuple type in the .d.ts file and assign that to the constructor argument.

from ts-protoc-gen.

A Tuple would do the job.

type UserArgs = (
  [/* name */ string] | 
  [/* name */ string, /* email */ string]
);

export class User extends jspb.Message {
  constructor(
    args?:  UserArgs
  );

  getName(): string;
  setName(value: string): void;

  getEmail(): string;
  setEmail(value: string): void;

  // ... definition
}

Edit: google-protobuf message constructor accepts an Array of any size up to maximum field number as arguments. I adjusted UserArgs to reflect this.

from ts-protoc-gen.

Hope this is still on the timeline. Very interested in seeing this implemented, would solve most of my remaining problems.

from ts-protoc-gen.

from ts-protoc-gen.

@samirbr Would you like to submit a PR for this?

from ts-protoc-gen.

Note that I have a fairly heft refactor to the way the protoc definitions are constructed in the following branch: https://github.com/improbable-eng/ts-protoc-gen/tree/feature/refactor-proto-codegen -- needs a fair amount of work before it's ready to raise :(

from ts-protoc-gen.

Ah, my bad. Thanks for letting us know!

from ts-protoc-gen.

Nothing to apologies for! :) Just it's probably easier to base it off of that refactor as it's non-trivial to add in-line to the existing logic.

from ts-protoc-gen.

Is anyone from the community interested in contributing an implementation of this? @samirbr?

from ts-protoc-gen.

Sure @gunn4r and I are planning to look into it within the next few weeks. Is the refactor that @jonnyreeves mentioned still being worked on? Or are we safe to start now?

from ts-protoc-gen.

So since this does not seem to be in progress I will add my two cents. For our case we need any[] for performance. Directly setting the constructor as an untyped array corresponding to the protobuf vs setting a message "the proper way" reduced API response time from about 200ms to 40ms due to the nature of the message/data (its huge).

That being said I was able to update the code to create a constructor with a type of any[] | {TupleType} fairly easily by converting Printer.ts output to a string[] and getOutput to a .join("") and then collecting the exportType's in printMessage to build the tuple and add it to the constructor. Probably less than 20 lines of code added or changed.

The main problem with this approach is the any[] portion as that does not work for everyone. I am not super familiar with the protoc plugins but does anyone know if there is a way to pass in say a "strict mode" flag that could tell this plugin to not put in any[] in that condition? If that is possible then I might have a PR I can submit in the near future once I do some cleanups and testing.

from ts-protoc-gen.

any progress on this topic?

from ts-protoc-gen.