[pickers] Add support for @js-joda/core

[MMauro94]

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Currently when using a DatePicker in MUI v5 with JsJodaAdapter (from @date-io/js-joda) an uncaught exception is thrown when changing date. The exception is: DateTimeException: Unable to obtain LocalTime TemporalAccessor: 2021-10-06, type LocalDate.

Expected Behavior 🤔

The picker does not crash and the new value is passed to the onChange function.

Steps to Reproduce 🕹

<LocalizationProvider dateAdapter={JsJodaAdapter}>
  <DatePicker
    value={LocalDate.of(2021, 10, 6)}
    onChange={(d) => console.log(d)}
    renderInput={(p) => <TextField {...p} />}
  />
</LocalizationProvider>

Sandbox link

Context 🔦

The curlpit seems to be on this line, when calling utils.mergeDateAndTime

Your Environment 🌎

`npx @mui/envinfo`

  System:
    OS: Windows 10 10.0.19043
  Binaries:
    Node: 14.17.3 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.10 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 6.14.13 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: Not Found
    Edge: Spartan (44.19041.1023.0), Chromium (94.0.992.38)

Don't know why it doesn't get detected, but I'm using Chrome 94.0.4606.71

[flaviendelangle]

Transfering this issue because it relates to a comment in the code that we should fix.

[cipacda]

Interested in this as well

[jaredkirkley]

I think this util method mergeDateAndTime is being misused throughout this codebase. Often it seems to be being used in an effort to reconcile a previous value with a new value, but from what it is doing it seems to be intended to merge a date value and a time value as the name would imply. I am getting a console error on a similar call in the DesktopTimePicker using AdapterDateFns when the input is cleared. mergeDateAndTime is then called with my previous value and a null new value and it breaks. Maybe this deserves a new bug?

Edit: looks like bumping some versions may have helped. My yarn lock now has changes including a date-fns bump and my issue has disappeared...leaving this here in case others find this later...

[flaviendelangle]

mergeDateAndTime is then called with my previous value and a null new value and it breaks. Maybe this deserves a new bug?

Edit: looks like bumping some versions may have helped. My yarn lock now has changes including a date-fns bump and my issue has disappeared...leaving this here in case others find this later...

Yes the problem here was indeed that we were trying to reconcile a null date with a time.

[joshkel]

If I understand correctly:

js-joda is fairly unique in having separate data types for dates, times, and dates+times. The @date-io project doesn't seem to have any specific APIs to handle this (although they do invite suggestions for new methods).

MUI-X uses mergeDateAndTime to, e.g., let TimePicker modify just the time portion of a date+time, or let CalendarPicker modify just the date portion of a date+time. This works for other date/time libraries but fails with js-joda if the user is using just a date or just a time (because there's no "date+X" or "X+time" to use).

I'm experimenting with the following modified adapter. I've done very little, but it seems to work so far.

import AdapterJoda from '@date-io/js-joda';
import { Temporal, TemporalQueries } from '@js-joda/core';

export class MuiAdapterJoda extends AdapterJoda {
  mergeDateAndTime(date: Temporal, time: Temporal): Temporal {
    const datePart = date.query(TemporalQueries.localDate());
    const timePart = time.query(TemporalQueries.localTime());
    if (datePart && timePart) {
      // This gives the same result as @date-io's implementation's
      // "LocalDate.from(date).atTime(LocalTime.from(time))"
      return datePart.atTime(timePart);
    } else if (!datePart) {
      // E.g., if MUI-X's TimePicker wants to preserve the date part, but the
      // user just gave a LocalTime.
      return time;
    } else if (!timePart) {
      // E.g., if MUI-X's CalendarPicker wants to preserve the time part, but
      // the user just gave a LocalDate:
      return date;
    } else {
      // Fall back to @date-io's implementation, which will throw due to null
      // date and time.
      return super.mergeDateAndTime(date, time);
    }
  }
}

const App = () => <LocalizationProvider dateAdapter={MuiAdapterJoda}><Root /></LocalizationProvider>;

[flaviendelangle]

Thanks for your experimentation
If the problem is on the @date-io packages, feel free to open a PR there and we will review it 👍
I struggle to evaluate the consequences of this quite unique format.

[joshkel]

Thanks, @flaviendelangle. As I understand it:

js-joda offers several date/time options that other libraries don't (the ability to explicitly work with dates without times, or times without dates; the ability to work in local dates/times, since UTC is not a silver bullet; etc.).

This is (almost?) unique among JavaScript date/time libraries, which complicates the task of an abstraction library like @date-io. E.g., to what degree should @date-io offer APIs that allow access to js-joda's more specialized functionality, vs. targeting the lowest common denominator? If @date-io does decide to offer access to more specialized functionality, then what's the best way to ensure that its API is sufficiently generic and abstract, given that js-joda is the only one of its kind and there doesn't seem to be strong demand for accessing this more specialized functionality? Etc. And so I start to think it would be better to not modify @date-io's API until there are broader and more concrete use cases.

So, for mergeDateAndTime in particular: @date-io's mergeDateAndTime for js-joda currently has the behavior of, "Given a valid date and time, return a valid date + time, and throw an error if that can't be done." That seems like a reasonable API, based on the method name and the behavior of other @date-io adapters. However, as used by mui-x, mergeDateAndTime's purpose is different: "Given either a valid date plus optional time, or an optional date plus valid time, preserve the optional value if possible."

Since that is an alternate, more specialized API, and since I thought it would be better to limit changes to @date-io's own API, I thought it might be better to extend the @date-io adapter within the mui-x package (similar to what I did here), or to at least discuss here before opening a PR in @date-io.

If you'd like me to submit this as for discussion within @date-io, I'm happy to do so.

Thanks.

[oliviertassinari]

Considering https://npm-stat.com/charts.html?package=%40js-joda%2Fcore&package=dayjs&package=luxon&package=moment&package=date-fns&from=2022-07-22&to=2023-07-22

would it make more sense to not have a built-in adapter for js-joda but to use this issue to 1. collect upvotes 2. have a JavaScript file for developers to copy to make the integration work?

[flaviendelangle]

Right now, we are not advertising for people to create their own adapter, mainly because it would widen the scope of what is a "breaking change".

We currently allow ourselves to add methods to adapters and use them on the pickers on a minor release.
But if people use their own adapter, it's a breaking change.
This would have made it very hard to support timezones in v6 for example.

[oliviertassinari]

@flaviendelangle Fair point. Then maybe we could do two things:

1. wait for upvotes, no official support until it's clear it's worth the opportunity cost
2. welcome the community who create unstable unofficial adapters for new data libraries

[flaviendelangle]

I think we could improve the doc to say that we are willing to help anyone wanting to build an adapter and host it on our repo.
That way we can test and update the adapter on new features to it will always be up to date.

There is a cost to maintaining adapters, but I doubt dozens of people will create adapters for an unknown date library.
And if that's the case, we could reconsider.

That's basically your 2.

[oliviertassinari]

But even with Joda, it has 1.3% (=3.4/(71+71+24+88+3.4)) of the market share. It feels like we would be better off by spending time on something else, e.g. to build an upload component than efforts to support it.

[joshkel]

I'm biased, because I'm one of the folks hoping for js-joda support. But a few potential considerations:

• I'm not sure that it's valid to translate from download count to market share and write off js-joda as only 1.3%, because some projects may use multiple libraries. My current project has Day.js (transitive dependency, and I'm using it directly because one of my dependencies doesn't support js-joda), date-fns (transitive dependency), Luxon (transitive dependency, because I'm still using a date-io version of @mui/x-date-pickers), and js-joda, even though I'm using js-joda everywhere I can. Intuitively, I'd expect the more popular libraries to be directly used less than their download counts suggest (because they're more likely to be transitive dependencies), while less popular libraries are more likely to be consciously chosen and directly used. (On the other hand, packages like the date-io version of @mui/x-date-pickers, which pull in every date library, could skew results the other way.)
• The upcoming JS Temporal API standard has a lot of similarities to js-joda. (js-joda is a port of Java's Joda, which became the standard java.time API, and which was also ported to .NET as NodaTime; if I understand correctly, the Temporal designers drew inspiration from Java and .NET as part of looking at prior art.) It seems likely that @mui/x-date-pickers will want to support Temporal, which means that much of the work to support js-joda would need to be done one way or another. (Of course, waiting until Temporal is standardized and focusing efforts there, instead of worrying about js-joda now, is always an option.)
• At the risk of sounding like a fanboy, and with the caveat that I'm far from an expert on dates/times/calendars: a js-joda-style API is really, really nice. It has an actual Date type (instead of hand-waving dates as "whatever timestamp happens to correspond to midnight on a given date") and Time type. (I need a Date type and haven't found it elsewhere in JS; I'm happy to be corrected here.) It has full timezone support. It supports durations as a data type. (24 hours may not be the same as 1 calendar day.) It understands the difference between an instant on a timeline (e.g., "2023-07-26T9:43:12Z"), how that instant is represented as a date + time in a specific calendar and time zone (e.g., that timestamp in the Gregorian calendar and US Eastern Daylight Saving Time), and a date + time in an unspecified time zone (e.g., a user selected "July 26 2023 9:43am" - UTC is not a silver bullet). It's the result of deep, sustained effort by people with a deep understanding of the issues involved. I bring this up in case there's any difference in discussions and tradeoffs between "here are five vaguely equivalent libraries, and one is a lot less popular" and "here's an admittedly less popular library, but it's uniquely powerful, so maybe there are good reasons for supporting it." 🙂

I'm happy to build and submit an adapter (as I tried with #8438 - I had failed to realize that @mui/x-date-pickers had finished its migration away from date-io, and I haven't had time to revisit), if that's what the project wants. I also understand if the project decides to not support js-joda directly - I'll likely end up creating my own unstable + unsupported interface in that case (which is more or less what I've been doing with the code that #8438 is based off of). I'll share a link to that here if I end up going that route.

Thank you.

[Brokemia]

Is there a current solution or workaround for using js-joda with DatePickers? It sounds like I am forced to use one of the other adapters, and then convert manually, is that correct?

[flaviendelangle]

It is correct 👍

[joshkel]

I've pushed an updated version of my js-joda adapter for MUI at #8438. See comments there for current status.

[LukasTy]

Thank you @joshkel. I've put this issue back into "Needs grooming" so the team could re-evaluate the potential of adding this adapter into the codebase. 😉