0.3.6-rc3 regression: missing '?' in message constructor (for repeated and map fields) about protoc-gen-ts HOT 10 CLOSED

Fields marked with 'repeated' or 'map' are missing the optional indicator; '?' in the Message constructor signature.

I am not sure whether they were correct in the past. right now the constructor takes field presence into consideration when generating the constructor. back then the idea was to make everything optional so that they can be filled later with setters but with the introduction of one-of-fields and checks we have to take field absence into consideration.

Hence, I am opinion of preserving the current behavior as it is stricter than the previous behavior and warn you upfront but this can be reverted if it creates so much overhead of changing the written code.

I would rather listen to users rather than taking a strong opinion on this. what do you think about how this should be?

from protoc-gen-ts.

Pretty sure that all fields are/should-be optional.
I posted comment about "oneof" on your "things to do next" thread.
"oneof" tracks internal field presence in its own ways... so should not be interact with the constructor form.
and: 'oneof' means: at MOST one of the fields is present, not a guarantee "one of the fields" is present.
It is totally legitimate to have a one-of field be empty.

from protoc-gen-ts.

I'm not sure what issue there is with 'map'...
But, fwiw, I noticed this in the spec, in case it is relevant: Map Field Equivalence

from protoc-gen-ts.

Pretty sure that all fields are/should-be optional.
I posted comment about "oneof" on your "things to do next" thread.
"oneof" tracks internal field presence in its own ways... so should not be interact with the constructor form.
and: 'oneof' means: at MOST one of the fields is present, not a guarantee "one of the fields" is present.
It is totally legitimate to have a one-of field be empty.

this is definitely the expected behavior when the data is given via constructor. I must have broken it while refactoring the codebase. so by this logic and since one-of fields are mutually exclusive the generated code should look like below regardless of syntax or field presence.

{field?: string} & {oneoffield1?: string, oneoffield2?: never} & {oneoffield1?: never, oneoffield2?: string} 

from protoc-gen-ts.

the reason we generate union types for one-of-fields is to warn the user through the compile-time errors hence the user knows which properties can be mutually present.

from protoc-gen-ts.

I'm not sure what issue there is with 'map'...
But, fwiw, I noticed this in the spec, in case it is relevant: Map Field Equivalence

I have other plans of using Map for map<k, v> which will be in memory until the serialize is called.

from protoc-gen-ts.

"the reason we generate union types for one-of-fields"

Hmm, that relates to the gist of my "Things to do" comment.
I don't see a union type being created.
Especially, I don't see a: get value(): ValueUnionType { return ... the value variant currently set ...}

I have synthesized the type and accesor for my simple TypedMsg* by patching:

TypedMsg.ts.patch.txt

syntax="proto3";
message TypedMsg {
  oneof value {
    bool boolValue = 3;
    int32 intValue = 4;
    string strValue = 5;
  }
}

from protoc-gen-ts.

is the generated message supposed to have a field called value alongside the one-of-fields?

from protoc-gen-ts.

I think so... the accessor 'value' returns the one-of {boolValue, intValue or strValue} that is actually set.

I've seen two implementation styles:
a) maintain 3 individual fields, and an indicator for which is 'set', and 'value' returns the field that is set.
and unset a field when the one-of the others is set (complex but builds on the existing field structure)
b) have 1 field for 'value' and the other 3 field accessors return value or undefined, based on the indicator.
(somewhat simpler, because there is only one field that can be set) [sets 'hasField' ... to make it clear]
Either way, you need to track the last one-of the fields to be set...

[.js implementation seems to be based on (a); Java is based on (b) ]

Take a look at the link into the "Dart" API... (in the other thread)
See if i got it right.

from protoc-gen-ts.

Hey @jackpunt.

I have landed a fix for this along with the support for one-of-fields and ECMAScript maps.

please try version 0.3.6-rc5 it should be working now.

feel free to reopen. if there is a problem

from protoc-gen-ts.