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

Why are outputted protos not classes #780

Closed
Ekrekr opened this issue Feb 17, 2023 · 3 comments · Fixed by #786
Closed

Why are outputted protos not classes #780

Ekrekr opened this issue Feb 17, 2023 · 3 comments · Fixed by #786

Comments

@Ekrekr
Copy link
Contributor

Ekrekr commented Feb 17, 2023

I'm trying out migrating to this from a different proto library because in general this looks much better! However we have hit a bit of a roadblock. We have variables that are of types of multiple kinds of protos. For example, we have a function that looks like:

function(a: proto1 | proto2 | proto3) {
  if (a instanceof proto1) {
    ...
  }
}

Because ts-proto generates an output of:

interface proto1 {...}
const proto1 = {...}
...

rather than classes, there doesn't seem to be a way of validating the proto type in this scenario - instanceof doesn't work with interfaces. Is there an expected way of accomplishing this?

See https://github.com/dataform-co/dataform/pull/1462/files#diff-8ac9e2a1b048a5e2289c9d7b9fcf1955150ce19191b86fc76c0e6cd7a643148a for full WIP.

@stephenh
Copy link
Owner

Avoiding classes was just one of the original "idiomatic TS / just POJOs" design goals for ts-proto, and I think that has generally worked out + is what most users prefer / like about the library.

It is not instanceof, but there is an option to add a $type field to every message that would let you do similar checks but more like ADTs instead of classes:

--ts_proto_opt=outputTypeRegistry=true

I kinda thought we had an option to output $type but not literally the whole type registry infra...but maybe not.

@Ekrekr
Copy link
Contributor Author

Ekrekr commented Feb 17, 2023

Thanks, this is what I was looking for!

Not outputting the whole type registry infra would be super useful - getting it to play nicely with Bazel at the moment is difficult.

@Ekrekr Ekrekr closed this as completed Feb 17, 2023
@stephenh
Copy link
Owner

Good! Fwiw I think adding a outputTypeFlag or something that did "just $type" from the type registry code would be a pretty simple PR; if you wanted to take a crack at it, that'd be great. Thanks!

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 a pull request may close this issue.

2 participants