bufbuild / protobuf-es

Protocol Buffers for ECMAScript. The only JavaScript Protobuf library that is fully-compliant with Protobuf conformance tests.
Apache License 2.0
1.15k stars 68 forks source link

Make FieldInfo "sub"-types exported #458

Closed nickwinger closed 1 year ago

nickwinger commented 1 year ago

Hello,

i am having a hardtime to constraint interfaces to only accept FieldInfo_Message (fiRules), because the type FieldInfo is made out of the "sub"-types but the subtypes are not exported. Some subtypes don't have the T property. Now i always get compile errors in typescript that T does not exists, you can workaround that in some places with an if (fieldInfo.kind == 'message') fieldInfo.T then you can access T but there are some places where you cannot do that, and also it's very annoying if you have a component where you know it's only Messages.

So please make that public exported.

e.g.: in File: https://github.com/bufbuild/protobuf-es/blob/main/packages/protobuf/src/field.ts

put something like:

export type FieldInfoScalar = fiRules<fiScalar>;
export type FieldInfoMessage = fiRules<fiMessage>;
...

I can also help out with a PR if you want

jcready commented 1 year ago

FWIW you should be able to get a type like that yourself by using an intersection:

type FieldInfoMessage = FieldInfo & { kind: 'message' };
nickwinger commented 1 year ago

@jcready thanks, i thought maybe typescript can do it, but that trick i haven't seen yet g nice...

However, i still think that these types should be in the source-lib code, however it's good to have it right now, thanks

btw. nice doom avatar ;)

eyalpost commented 1 year ago

I was just about to create the same issue. Would be great if they can be exported so we won't have to declare our own types. Also, we have a case where we want to patch FieldInfo and use declaration merging in order to expose that via typescript so this workaround won't help

eyalpost commented 1 year ago

Specifically we're interested in fiShared and descendants

smaye81 commented 1 year ago

Hi @eyalpost. Unfortunately, I don't see us exporting these anytime soon. We are trying to keep the exported surface area of the library as small as possible.

I'm assuming you would want the fiShared interface and descendants exposed since declaration merging won't work with types. However, this would require us exporting an additional five types that we'd like to stay private in case they need to change. Apologies for any inconvenience.

eyalpost commented 1 year ago

The fact that they are private doesn't really mean you can change them without breaking anything because they are "partially" exported already via FieldInfo. If I use properties of fiShared today (which I can because they are accessible) then any change by you will break my code

smaye81 commented 1 year ago

Not necessarily. Since we are exposing a type that uses Omit, we could theoretically add to the interfaces and then omit that property as part of fiRules. It wouldn't change the exposed type, but would change the interface.

Going to close this since we don't plan on exposing the interfaces, but appreciate the responses.