~emersion/gqlclient#2: 
Correctly handle nullable time.Time

Status
REPORTED
Submitter
~xenrox
Assigned to
No-one
Submitted
3 months ago
Updated
3 months ago
Labels
No labels applied.

~rockorager 3 months ago

Does this only apply to the referenced patch? It seems like the issue in that patch can be traced to this value.

It seems to me like gqlclient shouldn’t have to handle nullable structs, but that inputs should marshal to json properly on their own. In this case, changing time.Time to *time.Time should fix SubmitTicketInput.

I haven’t looked much deeper than that though so I recognize this might be shortsighted.

~emersion 3 months ago

Indeed, we already handle nullable structs. We special-case time.Time because it has a natural "no value" representation: the zero time. Using a nil *time.Time isn't very idiomatic.

~xenrox 3 months ago

The time documentation recommends that it should typically not be passed as a pointer and that IsZero can be used to determine uninitialized times. But if there are indeed legitimate use-cases where you would use the zero value, then time should probably be removed from this case. Otherwise the backend has to check with IsZero.

~emersion 3 months ago

The ideal solution is to use the time.Time zero value to represent a GQL null value, and marshal it as "null" on the wire. It's more difficult to implement.

Register here or Log in to comment, or comment via email.