-
-
Notifications
You must be signed in to change notification settings - Fork 94
Description
I'm looking at using the js-joda adapters with the MUI date/time pickers and am running into limitations: js-joda provides several date/time classes, but @date-io/js-joda
only supports a few of them. (See here for a good overview of the Java Joda-Time API; js-joda is modeled off of that.)
I'm happy to work on a PR to improve this but wasn't sure about date-io's error-handling philosophy:
@date-io/js-joda
has several functions that fall through and returnundefined
(e.g., 1, 2); I'm more familiar with getting TypeScript compiler errors ("Function lacks ending return statement and return type does not include 'undefined' (ts(2366))") in cases like this and adding explicit returns to address that.- Luxon, Moment, and Day.js apparently handle errors by returning special "invalid" date objects. js-joda instead handles errors by throwing exceptions (which is more what I'm used to). @date-io/js-joda apparently handles this difference by catching some errors and instead return the error object. (In my mind, this addresses that discrepancy but introduces other potential problems - the resulting
Temporal | Error
union isn't captured by the type system, and exceptions that aren't of the expected type cause the code to fall through to returningundefined
, like I mentioned above.)
To pick a concrete example: How should isAfter work if it's asked to compare a js-joda LocalDate
(a date with no time specified) with a LocalTime
(a time of day with no date specified)?
- Explicitly return false, since they aren't comparable, similar to how
NaN !== NaN
? - Fall through to return
undefined
, indicating that there's a potential hole in @date-io/js-joda's abstraction over js-joda? - Throw an exception, since that's what js-joda itself would do if you compared a
LocalDate
to aLocalTime
? - Something I'm not thinking of?
If you can provide some guidance on how to handle these interface mismatches and how errors ought to be handled within date-io, I can try to put together a PR to extend @date-io/js-joda
accordingly. Thanks!