isNullish() doesn't cover types well enough

[threadwaste]

Disclaimer: This might be me simply misunderstanding the intent of isNullish. I'm still deep diving the values implementation.

Currently, isNullish attempts to determine whether a value is a zero-value (in the case of values), or nil (in the case of pointers). This mostly works in its current form, but misses several cases. For example, *time.Time per this playground.

While digging into this, I came to a very sloppy implementation [while hacking this problem out] that involves reflect. To me, this makes more sense. However, despite substantial use of the empty interface, this library often times in cases like this forgoes reflect in favor of type assertions. Is there a technical reason for that? Or, simply an approach that worked initially, but hasn't evolved as the library grew?

At any rate, please let me know if I'm misunderstanding the intent of isNullish. Similarly, if moving this implementation over to one based on reflect is okay for the community, I'm happy to tidy up what I have here and open a pull request.

[threadwaste]

Okay, and now of course after spending a decent amount of time here, I'm realizing after taking a breather that this is just a scalar implementation issue. The type assertions over empty interface are valid because the GraphQL types are of limited scope, making it clean. What I'm missing here is a time scalar that converts this to one of the relevant GraphQL types.

Oof. 🤕

[bbuck]

No worries on that, there is at least one highly known issue with isNullish and that is that "" is considered null when (according to the issue/PR`) is definitely not the desired value.

[chris-ramon]

thanks for closing this one, landed to master via: #220