Based on the comment in the attached issue, it's unclear what the use-case for knowing the time is null is that a pointer wouldn't solve.

Is this related to the TypeScript generation?

I think this is a cleaner approach from a usability perspective in Go code. That is to say, the benefit is not in the frontend talking to the backend, it's using these values in the backend or cli.

IMO it'd be nice for us to consistently useNullTime over*time.Time in the API. Things like not being able to take a pointer of a function call&time.Now() are not an issue when you can writecodersdk.NewNullTime(time.Now(), true). Likewise when using the type, there's no need for theif t != nil check.

There are work-arounds for making pointer use a bit more convenient, like usingcoderd/util/ptr, but it's an extra step to using them vs accessing the concrete type directly.

One last point in favor ofNullTime, it's safer to use. There's no risk of doing a nil pointer dereference.

FWIW working with pointers makes me sad; I much prefer theNullXXX approach.
Unless we start going down the lines of having a genericOptional[T] type?

I'm hesitant to stray from such strong language primitives. Do we imagine having aNullXXX for every type that could be nullable? This seems odd to me because embedded structs would likely be pointers withomitempty rather thanNull<StructName>. Are we promoting thisonly for built-in types?

I understand why you may be hesitant, but I don't think it will be a problem. If we strive to make the zero value useful in most cases, the need for nullable values will be pretty low. We can also look atcodersdk (today), there we only have a couple of primitives (int, string) and time.

      7 *int64      5 *time.Time      3 *string      2 *uuid.UUID

Theuuid package already hasuuid.NullUUID, so it would make sense to use that one instead. In fact, the occurrence ofuuid.NullUUID is much larger in our codebase than that of*uuid.UUID (14 vs 3), yet we have both.

I also foresee that the need for a nullable struct will be very rare (we have none incodersdk, currently), and in those cases it would likely be a custom type for which we can simply implement a custom json marshaler/unmarshaler. It's unfortunate Go never got around to addingomitzero/omitempty to thejson package, because many times that would solve the use-case for using pointer-structs (i.e. often the reason is not to detect omission, it's to create a specific type of output).

I appreciate the explanation! I agree thatNullUUID is classically used, but that's typically been used for database operations, not the API.

I'll defer here though. You've spent a lot more time thinking about this than I, so I approve!

Leaving this here for future reference, and perhaps an alternative avenue for achieving the same in a generic way. (Did some prototyping on this with@johnstcn some days ago)

typeOptional[Tany]struct {ValueTValidbool}// Of returns a new Optional of the given value.funcOf[Tany](oT)Optional[T] {returnOptional[T]{Value:o,Valid:true,}}// MarshalJSON implements json.Marshaler.func (oOptional[T])MarshalJSON() ([]byte,error) {// omitted for brevity}// UnmarshalJSON implements json.Unmarshaler.func (o*Optional[T])UnmarshalJSON(data []byte)error {// omitted for brevity}typeAPIRequeststruct {TimeOptional[time.Time]`json:"time"`}// other packagefuncsample() {r:= codersdk.APIRequest{}r.Time=codersdk.Of(time.Now())ifr.Time.Valid {// if we care about null._=r.Time.Value}// zero value is ok without null check too_=r.Time.Value.IsZero()}

The namingsOptional,Of, etc are all up in the air, they could beNull andNewFrom instead.