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

Status
RESOLVED CLOSED
Submitter
~xenrox
Assigned to
No-one
Submitted
1 year, 1 month ago
Updated
7 months ago
Labels
No labels applied.

~rockorager 1 year, 1 month 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 1 year, 1 month 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 1 year, 1 month 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 1 year, 1 month 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.

~xenrox 7 months ago

This was implemented in ccaba8ec.

~emersion REPORTED CLOSED 7 months ago

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